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

[Feature/extensions] Modified Environment settings support #179

Merged
merged 5 commits into from
Oct 12, 2022
Merged

[Feature/extensions] Modified Environment settings support #179

merged 5 commits into from
Oct 12, 2022

Conversation

joshpalis
Copy link
Member

Description

Environment Settings support for extensibility will now pass the entire Settings object from OpenSearch node environment to the SDK.

Companion PR : opensearch-project/OpenSearch#4731

Issues Resolved

#78

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.

…ceptionHandling

Signed-off-by: Joshua Palis <jpalis@amazon.com>
…sponse handler syncronously

Signed-off-by: Joshua Palis <jpalis@amazon.com>
Copy link
Member

@dbwiddis dbwiddis left a comment

Choose a reason for hiding this comment

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

Need better handling of non-happy-path.

Signed-off-by: Joshua Palis <jpalis@amazon.com>
…andler

Signed-off-by: Joshua Palis <jpalis@amazon.com>
@dbwiddis dbwiddis force-pushed the main branch 3 times, most recently from f2399fb to 6eab1a5 Compare October 12, 2022 01:09
dbwiddis
dbwiddis previously approved these changes Oct 12, 2022
Copy link
Member

@dbwiddis dbwiddis left a comment

Choose a reason for hiding this comment

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

LGTM!

ryanbogan
ryanbogan previously approved these changes Oct 12, 2022
owaiskazi19
owaiskazi19 previously approved these changes Oct 12, 2022
Signed-off-by: Joshua Palis <jpalis@amazon.com>
@joshpalis joshpalis merged commit 0ddc9aa into opensearch-project:main Oct 12, 2022
@dbwiddis
Copy link
Member

Hey @joshpalis I just noticed the following in some gradle checks:

/home/runner/work/opensearch-sdk-java/opensearch-sdk-java/src/main/java/org/opensearch/sdk/handlers/EnvironmentSettingsResponseHandler.java:70: warning: no @throws for java.lang.InterruptedException

Can you please add that @throws in one of your next PRs?

@joshpalis
Copy link
Member Author

@dbwiddis Sure thanks for catching that, I'll make sure to add this

kokibas pushed a commit to kokibas/opensearch-sdk-java that referenced this pull request Mar 17, 2023
…h-project#179)

* modified environment settings request to use sendGenericRequestWithExceptionHandling

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* modified sendEnvironmentRequest to return the Settings object from response handler syncronously

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* addressing PR comments

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* addressing PR comments, moving countdown latch creation to response handler

Signed-off-by: Joshua Palis <jpalis@amazon.com>

* adding missing javadocs

Signed-off-by: Joshua Palis <jpalis@amazon.com>

Signed-off-by: Joshua Palis <jpalis@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants