Added extension Points, initial REST implementation and registering Transport Actions for extensions #5518

ryanbogan · 2022-12-12T07:48:44Z

Signed-off-by: Ryan Bogan rbogan@amazon.com

Description

This PR merges the second batch of feature/extensions into main and will then be backported to 2.x. It adds extension points, initial REST implementation, and registering Transport Actions for extensions.

Issues Resolved

Issue with links to PR's included in this merge: #5271
Meta issue for entire feature/extensions merge: #5263

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
Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

Signed-off-by: Ryan Bogan <rbogan@amazon.com>

github-actions · 2022-12-12T08:21:05Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/7914/
CommitID: 60ea641

github-actions · 2022-12-12T08:22:26Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/7915/
CommitID: 8c60329

ryanbogan · 2022-12-13T17:24:19Z

@dblock @reta Can you take a look at this when you get a chance?

dblock

Explain in the PR what it actually brings in in much more level of detail, please. "Second batch" is not very clear :)

Are we going to backport this to 2.x? Should we be adding the CHANGELOG in the 2.x section?

CHANGELOG.md

github-actions · 2022-12-13T23:10:51Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/8058/
CommitID: 0cdb48e
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

ryanbogan · 2022-12-13T23:44:20Z

Are we going to backport this to 2.x? Should we be adding the CHANGELOG in the 2.x section?

@dblock What is the difference between the Unreleased 2.x section on main branch and the Added section on the CHANGELOG file for 2.x branch?

Signed-off-by: Ryan Bogan <rbogan@amazon.com>

github-actions · 2022-12-14T00:51:51Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/8075/
CommitID: 331b654

saratvemulapalli · 2022-12-14T22:06:25Z

@reta would love your thoughts as well on this PR.

server/src/main/java/org/opensearch/common/io/stream/NamedWriteableRegistryParseRequest.java

dblock · 2022-12-15T18:51:52Z

Are we going to backport this to 2.x? Should we be adding the CHANGELOG in the 2.x section?

@dblock What is the difference between the Unreleased 2.x section on main branch and the Added section on the CHANGELOG file for 2.x branch?

There shouldn't be AFAIK, but now I am confused too @kotwanikunal?

…O comments Signed-off-by: Ryan Bogan <rbogan@amazon.com>

Signed-off-by: Ryan Bogan <rbogan@amazon.com>

ryanbogan · 2022-12-15T22:02:03Z

@reta I addressed your requested changes by removing the NamedWriteableRegistry extension implementation, which we will design differently in the future, but do not need right now. The PR is now ready for your review again.

github-actions · 2022-12-15T22:19:47Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/8164/
CommitID: d221711
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

github-actions · 2022-12-15T22:33:48Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/8165/
CommitID: e5ed171

saratvemulapalli · 2022-12-16T00:39:04Z

@reta I addressed your requested changes by removing the NamedWriteableRegistry extension implementation, which we will design differently in the future, but do not need right now. The PR is now ready for your review again.

@ryanbogan if we think its not the right implementation, lets go open up an issue and also help us understand why this is not right?

reta · 2022-12-16T16:00:15Z

server/src/main/java/org/opensearch/extensions/ExtensionBooleanResponse.java

+ *
+ * @opensearch.internal
+ */
+public class ExtensionBooleanResponse extends TransportResponse {


It is yet another variation if .opensearch.index.AcknowledgedResponse (I see it is removed), org.opensearch.action.support.master.AcknowledgedResponse ? May be we could (re)use the same pattern? Because ExtensionBooleanResponse is not saying anything about what its usage is. At least with acknowledged it reads as "request was accepted".

ryanbogan · 2022-12-16T17:28:48Z

@reta I addressed your requested changes by removing the NamedWriteableRegistry extension implementation, which we will design differently in the future, but do not need right now. The PR is now ready for your review again.

@ryanbogan if we think its not the right implementation, lets go open up an issue and also help us understand why this is not right?

opensearch-project/opensearch-sdk-java#291

Signed-off-by: Ryan Bogan <rbogan@amazon.com>

ryanbogan · 2022-12-16T17:56:21Z

@reta ready for your review again. Thanks for all the feedback!

github-actions · 2022-12-16T17:59:32Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/8195/
CommitID: 563f760

codecov-commenter · 2022-12-16T18:01:45Z

Codecov Report

Merging #5518 (563f760) into main (cb26035) will decrease coverage by 0.07%.
The diff coverage is 53.35%.

@@             Coverage Diff              @@
##               main    opensearch-project/OpenSearch#5518      +/-   ##
============================================
- Coverage     70.95%   70.88%   -0.08%     
- Complexity    58304    58316      +12     
============================================
  Files          4733     4741       +8     
  Lines        278256   278492     +236     
  Branches      40249    40262      +13     
============================================
- Hits         197441   197407      -34     
- Misses        64628    64973     +345     
+ Partials      16187    16112      -75

Impacted Files	Coverage Δ
...ensearch/discovery/InitializeExtensionRequest.java	`0.00% <0.00%> (ø)`
...nsearch/discovery/InitializeExtensionResponse.java	`0.00% <0.00%> (ø)`
...rg/opensearch/extensions/AcknowledgedResponse.java	`0.00% <0.00%> (ø)`
...a/org/opensearch/extensions/OpenSearchRequest.java	`0.00% <0.00%> (ø)`
...rch/extensions/rest/RestSendToExtensionAction.java	`27.77% <27.77%> (ø)`
...a/org/opensearch/extensions/ExtensionsManager.java	`60.75% <56.89%> (-2.28%)`	⬇️
...extensions/rest/RestExecuteOnExtensionRequest.java	`65.21% <65.21%> (ø)`
...ch/extensions/rest/RegisterRestActionsRequest.java	`70.00% <70.00%> (ø)`
...ch/extensions/RegisterTransportActionsRequest.java	`86.20% <86.20%> (ø)`
...h/extensions/rest/RegisterRestActionsResponse.java	`100.00% <100.00%> (ø)`
... and 488 more

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

reta · 2022-12-16T18:39:09Z

server/src/main/java/org/opensearch/extensions/AcknowledgedResponse.java

+ *
+ * @opensearch.internal
+ */
+public class AcknowledgedResponse extends TransportResponse {


👍 thanks @ryanbogan !

reta · 2022-12-16T18:40:58Z

server/src/main/java/org/opensearch/extensions/ExtensionsManager.java

            );
+            // TODO: make asynchronous


Sorry, I remember we discussed that before, this TODO: make asynchronous for this pull request or there is issue / pull request to follow up?

@reta right here: https://github.com/opensearch-project/OpenSearch/issues/5595

reta

LGTM, thanks @ryanbogan

opensearch-trigger-bot · 2022-12-16T19:07:01Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-5518-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 0f520f6ffe356f51584b8ed9dcec0f0ec24ef989
# Push it to GitHub
git push --set-upstream origin backport/backport-5518-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-5518-to-2.x.

ryanbogan added 2 commits December 12, 2022 07:47

Merge second batch of feature/extensions into main

60ea641

Signed-off-by: Ryan Bogan <rbogan@amazon.com>

Update CHANGELOG

8c60329

Signed-off-by: Ryan Bogan <rbogan@amazon.com>

ryanbogan marked this pull request as ready for review December 12, 2022 08:24

ryanbogan requested review from reta, anasalkouz, andrross, Bukhtawar, CEHENKLE, dblock, setiah, kartg, kotwanikunal, mch2, nknize, owaiskazi19, adnapibar, Rishikesh1159, saratvemulapalli, shwetathareja, dreamer-89, tlfeng, VachaShah and xuezhou25 as code owners December 12, 2022 08:24

Merge branch 'main' into merge_feature_extensions_into_main_2

0cdb48e

dblock approved these changes Dec 13, 2022

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Change logger level from info to bug for one call

331b654

Signed-off-by: Ryan Bogan <rbogan@amazon.com>

reta requested changes Dec 15, 2022

View reviewed changes

server/src/main/java/org/opensearch/common/io/stream/NamedWriteableRegistryParseRequest.java Outdated Show resolved Hide resolved

ryanbogan added 2 commits December 15, 2022 21:55

Removed extension NamedWriteableRegistry implementation and added TOD…

d221711

…O comments Signed-off-by: Ryan Bogan <rbogan@amazon.com>

Merge main into merge_feature_extensions_into_main_2

e5ed171

Signed-off-by: Ryan Bogan <rbogan@amazon.com>

ryanbogan requested a review from reta December 15, 2022 22:00

reta reviewed Dec 16, 2022

View reviewed changes

This was referenced Dec 16, 2022

Removed NamedWriteableRegistry and fixed DiscoveryExtensionNode const… opensearch-project/opensearch-sdk-java#290

Merged

[Feature/extensions] Add getNamedWriteableRegistry() API for extensions opensearch-project/opensearch-sdk-java#291

Closed

Rename ExtensionBooleanResponse to AcknowledgedResponse

563f760

Signed-off-by: Ryan Bogan <rbogan@amazon.com>

ryanbogan requested a review from reta December 16, 2022 17:55

reta reviewed Dec 16, 2022

View reviewed changes

reta approved these changes Dec 16, 2022

View reviewed changes

owaiskazi19 approved these changes Dec 16, 2022

View reviewed changes

owaiskazi19 merged commit 0f520f6 into opensearch-project:main Dec 16, 2022

ryanbogan added the backport 2.x Backport to 2.x branch label Dec 16, 2022

ryanbogan mentioned this pull request Dec 16, 2022

[Backport] [2.x] Added extension Points, initial REST implementation and registering T… #5596

Merged

6 tasks

ryanbogan deleted the merge_feature_extensions_into_main_2 branch December 16, 2022 23:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added extension Points, initial REST implementation and registering Transport Actions for extensions #5518

Added extension Points, initial REST implementation and registering Transport Actions for extensions #5518

ryanbogan commented Dec 12, 2022 •

edited

Loading

github-actions bot commented Dec 12, 2022

github-actions bot commented Dec 12, 2022

ryanbogan commented Dec 13, 2022

dblock left a comment

github-actions bot commented Dec 13, 2022

ryanbogan commented Dec 13, 2022

github-actions bot commented Dec 14, 2022

saratvemulapalli commented Dec 14, 2022

dblock commented Dec 15, 2022

ryanbogan commented Dec 15, 2022

github-actions bot commented Dec 15, 2022

github-actions bot commented Dec 15, 2022

saratvemulapalli commented Dec 16, 2022

reta Dec 16, 2022 •

edited

Loading

ryanbogan commented Dec 16, 2022

ryanbogan commented Dec 16, 2022

github-actions bot commented Dec 16, 2022

codecov-commenter commented Dec 16, 2022

reta Dec 16, 2022

reta Dec 16, 2022

ryanbogan Dec 16, 2022

reta left a comment

opensearch-trigger-bot bot commented Dec 16, 2022

Added extension Points, initial REST implementation and registering Transport Actions for extensions #5518

Added extension Points, initial REST implementation and registering Transport Actions for extensions #5518

Conversation

ryanbogan commented Dec 12, 2022 • edited Loading

Description

Issues Resolved

Check List

github-actions bot commented Dec 12, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Dec 12, 2022

Gradle Check (Jenkins) Run Completed with:

ryanbogan commented Dec 13, 2022

dblock left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 13, 2022

Gradle Check (Jenkins) Run Completed with:

ryanbogan commented Dec 13, 2022

github-actions bot commented Dec 14, 2022

Gradle Check (Jenkins) Run Completed with:

saratvemulapalli commented Dec 14, 2022

dblock commented Dec 15, 2022

ryanbogan commented Dec 15, 2022

github-actions bot commented Dec 15, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Dec 15, 2022

Gradle Check (Jenkins) Run Completed with:

saratvemulapalli commented Dec 16, 2022

reta Dec 16, 2022 • edited Loading

Choose a reason for hiding this comment

ryanbogan commented Dec 16, 2022

ryanbogan commented Dec 16, 2022

github-actions bot commented Dec 16, 2022

Gradle Check (Jenkins) Run Completed with:

codecov-commenter commented Dec 16, 2022

Codecov Report

reta Dec 16, 2022

Choose a reason for hiding this comment

reta Dec 16, 2022

Choose a reason for hiding this comment

ryanbogan Dec 16, 2022

Choose a reason for hiding this comment

reta left a comment

Choose a reason for hiding this comment

opensearch-trigger-bot bot commented Dec 16, 2022

ryanbogan commented Dec 12, 2022 •

edited

Loading

reta Dec 16, 2022 •

edited

Loading