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.

Sign up for GitHub

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

Jump to bottom

Revert "Draft: Add Err() function to Response for detailed errors (#2… #322

Merged
merged 5 commits into from
May 22, 2023
Merged

Revert "Draft: Add Err() function to Response for detailed errors (#2… #322

merged 5 commits into from
May 22, 2023

Conversation

VachaShah
Copy link
Collaborator

…46)"

This reverts commit 8d3cf4d.

Description

Reverting the breaking change in 2.3 branch so that a new 2.x version can be released. The main branch tracks the 3.0 changes with this breaking change.

Issues Resolved

List any issues this PR will resolve, e.g. Closes [...].

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@VachaShah
Revert "Draft: Add Err() function to Response for detailed errors (op…
ffed0df 
…ensearch-project#246)"

This reverts commit 8d3cf4d.

Signed-off-by: Vacha Shah <vachshah@amazon.com>
VachaShah added 2 commits May 18, 2023 07:01
@VachaShah
Fixing PIT APIs introduced in PR opensearch-project#253 since they us…
733f709 
…e response.Err()

Signed-off-by: Vacha Shah <vachshah@amazon.com>
@VachaShah
Removing opensearchapi.Error usage from snapshot tests introduced in PR
e3079ee 
opensearch-project#237

Signed-off-by: Vacha Shah <vachshah@amazon.com>
VijayanB
VijayanB previously approved these changes May 18, 2023
View reviewed changes
@dblock
Copy link
Member

dblock commented May 18, 2023

Resolve changelog?

@dblock
Copy link
Member

dblock commented May 18, 2023

Why do we need to release another 2.x? Can we release 3.x and move forward?

@VachaShah
Copy link
Collaborator Author

Why do we need to release another 2.x? Can we release 3.x and move forward?

2.x has some security fixes which need to go out and since it had some new features too, so the next 2.x version. If we want to move to 3.x, we could release this last version and mention in the README that for new features, the user can move to the 3.x line.

WDYT @dblock?

@VachaShah
Copy link
Collaborator Author

Also @Jakob3xD would love to get your eyes on this change since it reverts the Err() related changes from several PRs. Thank you!

@VachaShah
Resolve Changelog conflicts
0449215 
Signed-off-by: Vacha Shah <vachshah@amazon.com>
@VachaShah
Merge branch '2.3' into 2.3
0e1aa65 
Signed-off-by: Vacha Shah <vachshah@amazon.com>
@dblock
Copy link
Member

dblock commented May 21, 2023

Why do we need to release another 2.x? Can we release 3.x and move forward?

2.x has some security fixes which need to go out and since it had some new features too, so the next 2.x version. If we want to move to 3.x, we could release this last version and mention in the README that for new features, the user can move to the 3.x line.

WDYT @dblock?

I'm fine with it.

I guess I don't understand why we need to revert this feature from 2.3. Can we release the next 2.x with it?

@Jakob3xD
Copy link
Collaborator

I guess I don't understand why we need to revert this feature from 2.3. Can we release the next 2.x with it?

IIRC we did not want to make a 2.x release with it because it is a braking changes.
Maybe we want to wait with the 3.x release for the restructure of this lib, but this will take some time as I am currently busy with managing opensearch clusters.

The MR LGTM.

@dblock dblock merged commit 2f58bf6 into opensearch-project:2.3 May 22, 2023
@VachaShah VachaShah deleted the 2.3 branch May 22, 2023 23:14
@VachaShah VachaShah mentioned this pull request May 22, 2023
Closed
@VachaShah
Copy link
Collaborator Author

Thank you @dblock and @Jakob3xD! I added an update for the 2.3.0 version in PR #325 that is described above.

VachaShah added a commit to VachaShah/opensearch-go that referenced this pull request Oct 3, 2023
@VachaShah
Revert "Draft: Add Err() function to Response for detailed errors (#2… (
f7b3fdc 
opensearch-project#322)

* Revert "Draft: Add Err() function to Response for detailed errors (opensearch-project#246)"

This reverts commit 8d3cf4d.

Signed-off-by: Vacha Shah <vachshah@amazon.com>

* Fixing PIT APIs introduced in PR opensearch-project#253 since they use response.Err()

Signed-off-by: Vacha Shah <vachshah@amazon.com>

* Removing opensearchapi.Error usage from snapshot tests introduced in PR opensearch-project#237

Signed-off-by: Vacha Shah <vachshah@amazon.com>

* Resolve Changelog conflicts

Signed-off-by: Vacha Shah <vachshah@amazon.com>

---------

Signed-off-by: Vacha Shah <vachshah@amazon.com>
This was referenced Oct 3, 2023
Closed
Merged
dblock pushed a commit that referenced this pull request Oct 4, 2023
@VachaShah
Revert "Draft: Add Err() function to Response for detailed errors (#2… (
bb74a19 
#322) (#384)

* Revert "Draft: Add Err() function to Response for detailed errors (#246)"

This reverts commit 8d3cf4d.



* Fixing PIT APIs introduced in PR #253 since they use response.Err()



* Removing opensearchapi.Error usage from snapshot tests introduced in PR #237



* Resolve Changelog conflicts



---------

Signed-off-by: Vacha Shah <vachshah@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dblock dblock dblock approved these changes

@VijayanB VijayanB VijayanB left review comments

@Jakob3xD Jakob3xD Awaiting requested review from Jakob3xD Jakob3xD is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@VachaShah @dblock @Jakob3xD @VijayanB