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

[Feature/extensions] Integrated CreateComponent extensionPoint #3265

Merged
merged 7 commits into from
Jun 27, 2022
Merged

[Feature/extensions] Integrated CreateComponent extensionPoint #3265

merged 7 commits into from
Jun 27, 2022

Conversation

owaiskazi19
Copy link
Member

@owaiskazi19 owaiskazi19 commented May 9, 2022 •
edited
Loading

Signed-off-by: Owais Kazi owaiskazi19@gmail.com

Description

As a part of #2981, working on integrating createComponent extension point with SDK. Integrated ClusterState, LocalNode and PluginSettings.

Issues Resolved

[List any issues this PR will resolve]

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

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.

@owaiskazi19 owaiskazi19 requested review from a team and reta as code owners May 9, 2022 22:52
@owaiskazi19 owaiskazi19 marked this pull request as draft May 9, 2022 22:53
@owaiskazi19 owaiskazi19 force-pushed the draft-createComponent branch from ed22a7f to ba6c57f Compare May 9, 2022 22:55
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure ed22a7f3c48e125bcc70115a8a24bdea804061ae
Log 5183

Reports 5183

@saratvemulapalli saratvemulapalli changed the title Integrated CreateComponent extensionPoint [Feature/extensions] Integrated CreateComponent extensionPoint May 9, 2022
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure ba6c57fd07caa60be34aee34f93e1c9551779ba2
Log 5184

Reports 5184

@owaiskazi19 owaiskazi19 mentioned this pull request May 10, 2022
Closed
1 task
nknize
nknize requested changes May 10, 2022
View reviewed changes
Copy link
Collaborator

@nknize nknize left a comment

Choose a reason for hiding this comment

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

I know this is a PR to a feature branch but since we hope to feature flag this soon (so as to not hold on to long running feature branches) please start adding javadocs so we can slowly begin enabling the missingJavadoc precommit check. The predecessor has done a terrible / non-existent job adding javadocs and opensearch needs to quickly remedy this.

server/src/main/java/org/opensearch/cluster/ClusterServiceRequest.java Outdated Show resolved Hide resolved
server/src/main/java/org/opensearch/cluster/CreateComponentResponse.java Outdated Show resolved Hide resolved
server/src/main/java/org/opensearch/cluster/ClusterServiceRequest.java Outdated Show resolved Hide resolved
@owaiskazi19 owaiskazi19 force-pushed the draft-createComponent branch 3 times, most recently from 564efd2 to 0f6f739 Compare May 31, 2022 19:11
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 87ca0b4fca7a8298c7b05144e7fc315f466b5f21
Log 5687

Reports 5687

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 564efd21b07e9ad08d4d55cd2f7598bd19b3860b
Log 5688

Reports 5688

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 0f6f739fd2e7b942efea6dd55e530a6aee7f8108
Log 5689

Reports 5689

@owaiskazi19 owaiskazi19 marked this pull request as ready for review May 31, 2022 21:48
@joshpalis joshpalis self-requested a review June 1, 2022 17:49
@owaiskazi19 owaiskazi19 self-assigned this Jun 1, 2022
saratvemulapalli
saratvemulapalli reviewed Jun 6, 2022
View reviewed changes
Copy link
Member

@saratvemulapalli saratvemulapalli left a comment

Choose a reason for hiding this comment

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

Could you add some unit tests as well?
Hopefully @ryanbogan's PR gets merged and you can leverage the framework.

server/src/main/java/org/opensearch/cluster/ClusterServiceRequest.java Outdated Show resolved Hide resolved
server/src/main/java/org/opensearch/cluster/CreateComponentResponse.java Outdated Show resolved Hide resolved
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 22f964c0d3d994d4662b5d21d0140821e7530aa7
Log 5900

Reports 5900

@owaiskazi19 owaiskazi19 requested a review from nknize June 15, 2022 00:14
@owaiskazi19 owaiskazi19 mentioned this pull request Jun 17, 2022
Merged
@owaiskazi19
Copy link
Member Author

Created opensearch-project/opensearch-sdk-java#25 for unit tests as #3449 is yet to be merged which has the testing framework. If this PR looks good we can get it in.

@joshpalis joshpalis self-requested a review June 22, 2022 22:56
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 3bca134651bdc5501ac763f7a77ea95f66b51089
Log 6261

Reports 6261

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure b0cf2677c6fc88ad9bc0bad6e3ea61544bb9b3fa
Log 6277

Reports 6277

@owaiskazi19 owaiskazi19 force-pushed the draft-createComponent branch from b0cf267 to 560052a Compare June 24, 2022 20:59
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 560052aff23525cd8bb13781297d96b3528b5bf7
Log 6312

Reports 6312

@owaiskazi19
Draft createComponent extensionPoint
28c311f 
Signed-off-by: Owais Kazi <owaiskazi19@gmail.com>
@owaiskazi19
Integrated cluster state for createComponent
82bccc4 
Signed-off-by: Owais Kazi <owaiskazi19@gmail.com>
@owaiskazi19
Decoupled extension points design
54dd714 
Signed-off-by: Owais Kazi <owaiskazi19@gmail.com>
@owaiskazi19
Changed ClusterServiceRequest to generic ExtensionRequest
1ba2e89 
Signed-off-by: Owais Kazi <owaiskazi19@gmail.com>
@owaiskazi19
PR comments
deb6ed9 
Signed-off-by: Owais Kazi <owaiskazi19@gmail.com>
@owaiskazi19
Using ClusterStateResponse
b93ee83 
Signed-off-by: Owais Kazi <owaiskazi19@gmail.com>
@saratvemulapalli
Copy link
Member

@owaiskazi19 looks like there are conflicts can you take care of it. Thanks!

@owaiskazi19
Rebased
82a7d37 
Signed-off-by: Owais Kazi <owaiskazi19@gmail.com>
@owaiskazi19 owaiskazi19 force-pushed the draft-createComponent branch from 560052a to 82a7d37 Compare June 27, 2022 19:14
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 82a7d37
Log 6378

Reports 6378

@owaiskazi19 owaiskazi19 merged commit 76bbc9f into opensearch-project:feature/extensions Jun 27, 2022
@ryanbogan ryanbogan mentioned this pull request Nov 17, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshpalis joshpalis joshpalis approved these changes

@saratvemulapalli saratvemulapalli saratvemulapalli approved these changes

@ryanbogan ryanbogan ryanbogan approved these changes

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

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

Assignees

@owaiskazi19 owaiskazi19

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@owaiskazi19 @opensearch-ci-bot @saratvemulapalli @nknize @ryanbogan @joshpalis