Skip to content

[Feature/extensions] Adding support for registering Transport Actions for extensions#4326

Merged
saratvemulapalli merged 5 commits intoopensearch-project:feature/extensionsfrom
saratvemulapalli:get-actions-api
Sep 1, 2022
Merged

[Feature/extensions] Adding support for registering Transport Actions for extensions#4326
saratvemulapalli merged 5 commits intoopensearch-project:feature/extensionsfrom
saratvemulapalli:get-actions-api

Conversation

@saratvemulapalli
Copy link
Copy Markdown
Member

Signed-off-by: Sarat Vemulapalli vemulapallisarat@gmail.com

Description

Adds support for registering Transport Actions for extensions.

Issues Resolved

Closes opensearch-project/opensearch-sdk-java#105

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: Sarat Vemulapalli <vemulapallisarat@gmail.com>
@saratvemulapalli saratvemulapalli requested review from a team and reta as code owners August 29, 2022 17:40
@github-actions
Copy link
Copy Markdown
Contributor

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Sarat Vemulapalli <vemulapallisarat@gmail.com>
@github-actions
Copy link
Copy Markdown
Contributor

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Sarat Vemulapalli <vemulapallisarat@gmail.com>
@github-actions
Copy link
Copy Markdown
Contributor

Gradle Check (Jenkins) Run Completed with:

public static final String REQUEST_EXTENSION_LOCAL_NODE = "internal:discovery/localnode";
public static final String REQUEST_EXTENSION_CLUSTER_SETTINGS = "internal:discovery/clustersettings";
public static final String REQUEST_EXTENSION_REGISTER_REST_ACTIONS = "internal:discovery/registerrestactions";
public static final String REQUEST_EXTENSION_REGISTER_TRANSPORT_ACTIONS = "internal:discovery/registertransportactions";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are we having separate action for extensions?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There is an open issue: #4334. Lets do a sweep all together in a dedicated PR.

Signed-off-by: Sarat Vemulapalli <vemulapallisarat@gmail.com>
@github-actions
Copy link
Copy Markdown
Contributor

Gradle Check (Jenkins) Run Completed with:

Copy link
Copy Markdown
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. One optional comment inline


BytesStreamOutput output = new BytesStreamOutput();
originalRequest.writeTo(output);
StreamInput input = output.bytes().streamInput();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(Comment) I realize that close() is a noop here so these don't need to be closed, but some IDE linters will complain that a resource isn't closed. Personal preference of mine. :)

Signed-off-by: Sarat Vemulapalli <vemulapallisarat@gmail.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Sep 1, 2022

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Merging #4326 (1970358) into feature/extensions (50f2d9e) will decrease coverage by 0.01%.
The diff coverage is 81.25%.

@@                   Coverage Diff                    @@
##             feature/extensions    #4326      +/-   ##
========================================================
- Coverage                 70.61%   70.60%   -0.02%     
- Complexity                57271    57332      +61     
========================================================
  Files                      4633     4634       +1     
  Lines                    275817   275849      +32     
  Branches                  40337    40340       +3     
========================================================
- Hits                     194763   194752      -11     
- Misses                    64722    64798      +76     
+ Partials                  16332    16299      -33     
Impacted Files Coverage Δ
.../opensearch/extensions/ExtensionsOrchestrator.java 66.45% <33.33%> (-0.63%) ⬇️
...ch/extensions/RegisterTransportActionsRequest.java 86.20% <86.20%> (ø)
...java/org/opensearch/client/indices/DataStream.java 0.00% <0.00%> (-76.09%) ⬇️
...n/indices/forcemerge/ForceMergeRequestBuilder.java 0.00% <0.00%> (-75.00%) ⬇️
...pensearch/client/cluster/RemoteConnectionInfo.java 0.00% <0.00%> (-68.30%) ⬇️
...adcast/BroadcastShardOperationFailedException.java 33.33% <0.00%> (-66.67%) ⬇️
...a/org/opensearch/client/cluster/SniffModeInfo.java 0.00% <0.00%> (-58.83%) ⬇️
.../indices/forcemerge/TransportForceMergeAction.java 25.00% <0.00%> (-58.34%) ⬇️
...a/org/opensearch/tasks/TaskCancelledException.java 50.00% <0.00%> (-50.00%) ⬇️
...search/aggregations/pipeline/HoltWintersModel.java 21.47% <0.00%> (-42.96%) ⬇️
... and 513 more

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

@saratvemulapalli
Copy link
Copy Markdown
Member Author

@owaiskazi19 Waiting for the 2nd review.
I'll follow up with opensearch-project/opensearch-sdk-java#106 after this set of PRs are merged.

@saratvemulapalli saratvemulapalli merged commit 0e6ff2c into opensearch-project:feature/extensions Sep 1, 2022
@saratvemulapalli saratvemulapalli deleted the get-actions-api branch September 1, 2022 16:53
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.

5 participants