Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "[Manual Backport 2.x][MD] Support SigV4 as a new auth type of datasource" #3464

Merged
merged 1 commit into from
Feb 20, 2023

Conversation

joshuarrrr
Copy link
Member

Bumping @opensearch-project/opensearch from ^1.1.0 to ^2.1.0 is a breaking change for plugins and CI, and should not have been backported to 2.x

Fixes opensearch-project/opensearch-build#3210

@codecov-commenter
Copy link

codecov-commenter commented Feb 20, 2023

Codecov Report

Merging #3464 (32944dd) into 2.x (448c755) will increase coverage by 0.12%.
The diff coverage is 76.74%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##              2.x    #3464      +/-   ##
==========================================
+ Coverage   66.42%   66.54%   +0.12%     
==========================================
  Files        3205     3203       -2     
  Lines       61551    61397     -154     
  Branches     9492     9453      -39     
==========================================
- Hits        40884    40859      -25     
+ Misses      18399    18284     -115     
+ Partials     2268     2254      -14     
Flag Coverage Δ
Linux 66.49% <76.74%> (+0.12%) ⬆️
Windows 66.49% <76.74%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...omponents/validation/datasource_form_validation.ts 100.00% <ø> (+24.13%) ⬆️
src/plugins/data_source_management/public/types.ts 100.00% <ø> (ø)
...gins/data_source/server/client/configure_client.ts 66.03% <66.66%> (+19.37%) ⬆️
...c/plugins/data_source/server/client/client_pool.ts 58.82% <71.42%> (+12.15%) ⬆️
...ta_source/server/legacy/configure_legacy_client.ts 70.83% <77.77%> (+11.21%) ⬆️
...rce/components/edit_form/edit_data_source_form.tsx 90.76% <86.66%> (+20.02%) ⬆️
.../plugins/data_source/server/data_source_service.ts 71.42% <100.00%> (ø)
...components/create_form/create_data_source_form.tsx 96.96% <100.00%> (+21.70%) ⬆️
...ic/application/models/sense_editor/sense_editor.ts 65.77% <0.00%> (+0.88%) ⬆️
packages/osd-optimizer/src/node/cache.ts 53.94% <0.00%> (+2.63%) ⬆️
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@kavilla
Copy link
Member

kavilla commented Feb 20, 2023

I think we can remove the 2.6 label here as well right: #3465?

Copy link
Member

@kavilla kavilla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with reverting this. I don't think the call was to bump to a major version. I think if sigv4 support was a minor change for 2.x it can also be a minor change for 1.x and request can be made to support.

There is no indication that the 1.x version of OpenSearch JS is deprecated: https://www.npmjs.com/package/@opensearch-project/opensearch/v/1.1.0.

We should request if this: opensearch-project/opensearch-js#279 can be backported to 1.x. So that we can get the new feature without causing a breaking change.

cc: @ananzh, @zhongnansu, @abbyhu2000

@ananzh
Copy link
Member

ananzh commented Feb 20, 2023

I think 2.6 is too rushy. We should revert it. Then we should discuss whether we could treat client as a special package and relax the restrict version match.

@seraphjiang
Copy link
Member

Not clear on what specific functionality break changes have occurred that justify the revert

We should have a retrospective to revisit and align with the community on the definition break change perf SemVer to improve the process. Otherwise, it will decrease productivity and erode the community's trust.

@seanneumann @dblock @CEHENKLE @bbarani

@joshuarrrr joshuarrrr merged commit 17ca299 into 2.x Feb 20, 2023
@joshuarrrr joshuarrrr deleted the revert-3458-2.x-md-sigv4 branch February 20, 2023 22:56
@huyaboo huyaboo mentioned this pull request Aug 29, 2024
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants