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

Implement Action dependency injection #466

Merged
merged 9 commits into from
Feb 20, 2023

Conversation

dbwiddis
Copy link
Member

Description

Fully implements getActions() extension point

  • Injects the actions in Guice module
  • Enables migration of client.execute(action, request, listener) syntax via SDKClient
  • Populates the TransportActions object on the extensions runner using the expected map types
  • Adds tests

Issues Resolved

Fixes #368
Fixes #283

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.

@codecov-commenter
Copy link

codecov-commenter commented Feb 19, 2023

Codecov Report

Merging #466 (319e48e) into main (1133514) will increase coverage by 3.76%.
The diff coverage is 97.26%.

📣 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     #466      +/-   ##
============================================
+ Coverage     62.16%   65.92%   +3.76%     
- Complexity      158      180      +22     
============================================
  Files            30       35       +5     
  Lines           740      807      +67     
  Branches         21       24       +3     
============================================
+ Hits            460      532      +72     
+ Misses          270      264       -6     
- Partials         10       11       +1     
Impacted Files Coverage Δ
src/main/java/org/opensearch/sdk/Extension.java 100.00% <ø> (ø)
...main/java/org/opensearch/sdk/ExtensionsRunner.java 70.05% <93.75%> (+1.67%) ⬆️
...ava/org/opensearch/sdk/action/SDKActionModule.java 95.45% <95.45%> (ø)
src/main/java/org/opensearch/sdk/SDKClient.java 90.66% <100.00%> (+0.66%) ⬆️
...rch/sdk/sample/helloworld/HelloWorldExtension.java 66.66% <100.00%> (+6.66%) ⬆️
.../sdk/sample/helloworld/transport/SampleAction.java 100.00% <100.00%> (ø)
...sdk/sample/helloworld/transport/SampleRequest.java 100.00% <100.00%> (ø)
...dk/sample/helloworld/transport/SampleResponse.java 100.00% <100.00%> (ø)
...le/helloworld/transport/SampleTransportAction.java 100.00% <100.00%> (ø)
... and 1 more

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

Copy link
Member

@joshpalis joshpalis left a comment

Choose a reason for hiding this comment

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

LGTM thanks Dan

@dbwiddis dbwiddis merged commit 7883fba into opensearch-project:main Feb 20, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Feb 20, 2023
* ActionModule with getActions method

Signed-off-by: Daniel Widdis <[email protected]>

* Implement getActionFilters extension point

Signed-off-by: Daniel Widdis <[email protected]>

* Inject getActions and populate TransportActions

Signed-off-by: Daniel Widdis <[email protected]>

* Add Sample Actions and fix broken tests

Signed-off-by: Daniel Widdis <[email protected]>

* Add tests

Signed-off-by: Daniel Widdis <[email protected]>

* Undo whitespace changes to minimize diff

Signed-off-by: Daniel Widdis <[email protected]>

* Refactor TransportActions initialization

Signed-off-by: Daniel Widdis <[email protected]>

* Add test coverage for unregistered action

Signed-off-by: Daniel Widdis <[email protected]>

* Update migration guide

Signed-off-by: Daniel Widdis <[email protected]>

---------

Signed-off-by: Daniel Widdis <[email protected]>
(cherry picked from commit 7883fba)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Copy link
Member

@owaiskazi19 owaiskazi19 left a comment

Choose a reason for hiding this comment

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

Overall LGTM! I have a small question on the design though. The way we are registering RestHandlers of SDK(extension) in OpenSearch to let OS know about the new rest handlers. In case I missed, we aren't doing the same for actions. WDYT?

dbwiddis pushed a commit that referenced this pull request Feb 20, 2023
* ActionModule with getActions method



* Implement getActionFilters extension point



* Inject getActions and populate TransportActions



* Add Sample Actions and fix broken tests



* Add tests



* Undo whitespace changes to minimize diff



* Refactor TransportActions initialization



* Add test coverage for unregistered action



* Update migration guide



---------


(cherry picked from commit 7883fba)

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>
@vibrantvarun
Copy link
Member

Reviewed the PR. Great Job Dan @dbwiddis .

@dbwiddis dbwiddis deleted the get-action-inject branch February 20, 2023 23:32
kokibas pushed a commit to kokibas/opensearch-sdk-java that referenced this pull request Mar 17, 2023
* ActionModule with getActions method

Signed-off-by: Daniel Widdis <[email protected]>

* Implement getActionFilters extension point

Signed-off-by: Daniel Widdis <[email protected]>

* Inject getActions and populate TransportActions

Signed-off-by: Daniel Widdis <[email protected]>

* Add Sample Actions and fix broken tests

Signed-off-by: Daniel Widdis <[email protected]>

* Add tests

Signed-off-by: Daniel Widdis <[email protected]>

* Undo whitespace changes to minimize diff

Signed-off-by: Daniel Widdis <[email protected]>

* Refactor TransportActions initialization

Signed-off-by: Daniel Widdis <[email protected]>

* Add test coverage for unregistered action

Signed-off-by: Daniel Widdis <[email protected]>

* Update migration guide

Signed-off-by: Daniel Widdis <[email protected]>

---------

Signed-off-by: Daniel Widdis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Action dependency injection [META] Create Component Integration
6 participants