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] Adding support for Extension Create Components [environment settings and add setting update consumers] #138

Merged
merged 20 commits into from
Sep 26, 2022

Conversation

joshpalis
Copy link
Member

@joshpalis joshpalis commented Sep 14, 2022

Description

Adds support for extensions to request environment setting values from opensearch, to add setting update consumers to cluster settings, and to receive update setting requests from Opensearch whenever a setting is updated.

Companion PR : opensearch-project/OpenSearch#4517

Note : Currently in a draft state until #79 is resolved

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.

@joshpalis joshpalis changed the title Adding support for Extension Create Components [environment settings and add setting update consumers] [Feature/extensions] Adding support for Extension Create Components [environment settings and add setting update consumers] Sep 16, 2022
dbwiddis
dbwiddis previously approved these changes Sep 16, 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!

Signed-off-by: Joshua Palis <[email protected]>
@joshpalis joshpalis marked this pull request as ready for review September 19, 2022 23:23
@joshpalis joshpalis requested review from a team and saratvemulapalli September 19, 2022 23:23
@joshpalis joshpalis requested a review from dbwiddis September 20, 2022 18:10
dbwiddis
dbwiddis previously approved these changes Sep 20, 2022
Object data = updateSettingsRequest.getData();

// Setting updater in OpenSearch performs setting change validation, only need to cast the consumer to the corresponding type and
// invoke the consumer
Copy link
Member

Choose a reason for hiding this comment

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

Not a request to change anything in your PR but a note for awareness. The original validator and parser are lost in the existing setup, so if there was some extra validation step (for example, minimum value integer) it might not properly validate on the OpenSearch side. That's fine for now as it will eventually be handled by #154.

…eanResponseHandler, moving handleUpdateSettings method to seperate class

Signed-off-by: Joshua Palis <[email protected]>
@joshpalis joshpalis requested a review from dbwiddis September 23, 2022 21:39
…will only need to provide a map of setting and consumer, and upon sending the addsettingupdateconsumer request, this map will be registered within the updateSettingsRequestHandler

Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
Signed-off-by: Joshua Palis <[email protected]>
@ryanbogan ryanbogan self-requested a review September 26, 2022 17:08
@ryanbogan ryanbogan merged commit 30d6373 into opensearch-project:main Sep 26, 2022
kokibas pushed a commit to kokibas/opensearch-sdk-java that referenced this pull request Mar 17, 2023
…environment settings and add setting update consumers] (opensearch-project#138)

* Added support for enviornment settings requests and unit tests

Signed-off-by: Joshua Palis <[email protected]>

* updated environment settings support for create components

Signed-off-by: Joshua Palis <[email protected]>

* Updating javadocs for environmentsetting request/ response handlers

Signed-off-by: Joshua Palis <[email protected]>

* Adding inital support for AddSettingsUpdateConsumer

Signed-off-by: Joshua Palis <[email protected]>

* Updated implementation in preparation for getSettings support integration

Signed-off-by: Joshua Palis <[email protected]>

* Updated sendAddSettingsUpdateConsumerRequest to pull settings list directly from setting update consumer registry

Signed-off-by: Joshua Palis <[email protected]>

* Integrated writeable settings support

Signed-off-by: Joshua Palis <[email protected]>

* Updating writeable settings integration for setting type

Signed-off-by: Joshua Palis <[email protected]>

* Replacing AddSettingsUpdateConsumerResponseHandler with ExtensionBooleanResponseHandler, moving handleUpdateSettings method to seperate class

Signed-off-by: Joshua Palis <[email protected]>

* refactoring sendAddSettingsUpdateConsumerRequest. Now each component will only need to provide a map of setting and consumer, and upon sending the addsettingupdateconsumer request, this map will be registered within the updateSettingsRequestHandler

Signed-off-by: Joshua Palis <[email protected]>

* simplifying settingUpdateConsumer registration

Signed-off-by: Joshua Palis <[email protected]>

* Fixing javadocs

Signed-off-by: Joshua Palis <[email protected]>

* fixing javadocs

Signed-off-by: Joshua Palis <[email protected]>

* adding missing javadocs

Signed-off-by: Joshua Palis <[email protected]>

* fixing javadocs

Signed-off-by: Joshua Palis <[email protected]>

Signed-off-by: Joshua Palis <[email protected]>
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