-
Notifications
You must be signed in to change notification settings - Fork 58
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
Execute remote actions on another extension #588
Conversation
4201515
to
5e423d1
Compare
Gradle check is failing based on companion PR classes. |
e15d9d3
to
80578bd
Compare
Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
bee5f3e
to
87803c6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work on Dynamic Actions @dbwiddis 🙌! Didn't get a chance to review the tests
src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java
Show resolved
Hide resolved
src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java
Show resolved
Hide resolved
src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/sdk/handlers/ExtensionActionResponseHandler.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
09b34db
to
79499a0
Compare
src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java
Show resolved
Hide resolved
src/main/java/org/opensearch/sdk/handlers/ExtensionActionRequestHandler.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
@owaiskazi19 @saratvemulapalli addressed all your comments. And as-of now it still works:
|
Gradle check failing due to companion PR. Only thing left is to update the sequence diagram which I'm working on. |
Signed-off-by: Daniel Widdis <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for incorporating all the suggestions!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks @dbwiddis for this.
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #588 +/- ##
=======================================
Coverage ? 63.97%
Complexity ? 244
=======================================
Files ? 49
Lines ? 1088
Branches ? 35
=======================================
Hits ? 696
Misses ? 381
Partials ? 11
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
* Add ProxyAction with TransportAction and handlers Signed-off-by: Daniel Widdis <[email protected]> * Give SDKActionModule a copy of ExtensionsRunner to use with transport Signed-off-by: Daniel Widdis <[email protected]> * Add new ProxyActionRequest Signed-off-by: Daniel Widdis <[email protected]> * Add SDKTransportService wrapper accessible to actions Signed-off-by: Daniel Widdis <[email protected]> * Implement ProxyTransportAction Signed-off-by: Daniel Widdis <[email protected]> * Add test case to HelloWorldExtension Signed-off-by: Daniel Widdis <[email protected]> * Better naming of ExtensionActionResponse and correct action name Signed-off-by: Daniel Widdis <[email protected]> * Refactoring with TransportService and latest OpenSearch PR updates Signed-off-by: Daniel Widdis <[email protected]> * Add ExtensionsActionRequestHandler Signed-off-by: Daniel Widdis <[email protected]> * Instantiate Proxy Action Request Signed-off-by: Daniel Widdis <[email protected]> * Working test case! Signed-off-by: Daniel Widdis <[email protected]> * Properly parse returned byte array into a response Signed-off-by: Daniel Widdis <[email protected]> * Add sequence diagram to DESIGN.md Signed-off-by: Daniel Widdis <[email protected]> * Typoo fix Signed-off-by: Daniel Widdis <[email protected]> * Update with latest changes on companion PR Signed-off-by: Daniel Widdis <[email protected]> * Rename ProxyFoo to RemoteExtensionFoo Signed-off-by: Daniel Widdis <[email protected]> * Better handling of response bytes Signed-off-by: Daniel Widdis <[email protected]> * Handle plugin remote action requests Signed-off-by: Daniel Widdis <[email protected]> * Address code review comments Signed-off-by: Daniel Widdis <[email protected]> * Update sequence diagram Signed-off-by: Daniel Widdis <[email protected]> --------- Signed-off-by: Daniel Widdis <[email protected]> (cherry picked from commit ebc684a) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* Add ProxyAction with TransportAction and handlers * Give SDKActionModule a copy of ExtensionsRunner to use with transport * Add new ProxyActionRequest * Add SDKTransportService wrapper accessible to actions * Implement ProxyTransportAction * Add test case to HelloWorldExtension * Better naming of ExtensionActionResponse and correct action name * Refactoring with TransportService and latest OpenSearch PR updates * Add ExtensionsActionRequestHandler * Instantiate Proxy Action Request * Working test case! * Properly parse returned byte array into a response * Add sequence diagram to DESIGN.md * Typoo fix * Update with latest changes on companion PR * Rename ProxyFoo to RemoteExtensionFoo * Better handling of response bytes * Handle plugin remote action requests * Address code review comments * Update sequence diagram --------- (cherry picked from commit ebc684a) Signed-off-by: Daniel Widdis <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Joshua Palis <[email protected]>
Companion PR: #6734 must be merged first
Description
Allows one extension to execute an action on another extension.
See the Hello World RestRemoteHelloAction for sample implementation.
Other work done to support:
Issues Resolved
Fixes #525
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.