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

Publish transport-netty4 module to Central Repository #4054

Merged
merged 1 commit into from
Aug 1, 2022

Conversation

dbwiddis
Copy link
Member

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

Description

The extension framework opensearch-sdk requires the transport-netty4 module as a dependency.

Issues Resolved

Fixes #3118

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.

@dbwiddis dbwiddis requested review from a team and reta as code owners July 31, 2022 01:37
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

Codecov Report

Merging #4054 (c81bb49) into main (740f75d) will increase coverage by 0.09%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main    #4054      +/-   ##
============================================
+ Coverage     70.50%   70.60%   +0.09%     
- Complexity    56848    56924      +76     
============================================
  Files          4583     4583              
  Lines        273931   273931              
  Branches      40158    40158              
============================================
+ Hits         193146   193404     +258     
+ Misses        64561    64323     -238     
+ Partials      16224    16204      -20     
Impacted Files Coverage Δ
...luster/routing/allocation/RoutingExplanations.java 41.37% <0.00%> (-58.63%) ⬇️
...ava/org/opensearch/action/NoSuchNodeException.java 0.00% <0.00%> (-50.00%) ⬇️
.../admin/cluster/reroute/ClusterRerouteResponse.java 55.00% <0.00%> (-45.00%) ⬇️
...adcast/BroadcastShardOperationFailedException.java 55.55% <0.00%> (-44.45%) ⬇️
...nsearch/rest/action/cat/RestCatRecoveryAction.java 43.61% <0.00%> (-42.56%) ⬇️
...cluster/routing/allocation/RerouteExplanation.java 65.00% <0.00%> (-35.00%) ⬇️
...arch/search/aggregations/pipeline/LinearModel.java 23.07% <0.00%> (-34.62%) ⬇️
...min/cluster/snapshots/get/GetSnapshotsRequest.java 52.63% <0.00%> (-31.58%) ⬇️
...rc/main/java/org/opensearch/ingest/IngestInfo.java 51.72% <0.00%> (-27.59%) ⬇️
...org/opensearch/test/engine/MockInternalEngine.java 56.52% <0.00%> (-26.09%) ⬇️
... and 438 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

@@ -38,6 +38,9 @@ apply plugin: 'opensearch.yaml-rest-test'
apply plugin: 'opensearch.java-rest-test'
apply plugin: 'opensearch.internal-cluster-test'

// The transport-netty4 plugin is published to maven
apply plugin: 'opensearch.publish'
Copy link
Member

@dreamer-89 dreamer-89 Jul 31, 2022

Choose a reason for hiding this comment

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

@dbwiddis : Thanks for this PR. I have few questions around this change for my own understanding.

  1. How is opensearch-sdk consuming this plugin ?
  2. Why do we want to have plugin in core but consumed in opensearch-sdk ?

Copy link
Member Author

Choose a reason for hiding this comment

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

How is opensearch-sdk consuming this plugin ?

We are using the TransportService wrapping Netty4Transport as our means of having the extensions communicate with OpenSearch itself. See here for example.

Presently we have simply copied all the classes over into a package in the sdk during development. We'd like to move this to a dependency published to Maven.

Why do we want to have plugin in core but consumed in opensearch-sdk ?

It will also be consumed in core for remote communication with the extensions. That has not yet been added as we are temporarily using plugin features. See here for where the TransportService is used and will eventually also consume this dependency.

@owaiskazi19 please clarify/correct anything I may have misstated.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you @dbwiddis for the details.

@saratvemulapalli saratvemulapalli added v3.0.0 Issues and PRs related to version 3.0.0 v2.2.0 backport 2.x Backport to 2.x branch labels Aug 1, 2022
@saratvemulapalli saratvemulapalli merged commit 707ca13 into opensearch-project:main Aug 1, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 1, 2022
Signed-off-by: Daniel Widdis <[email protected]>
(cherry picked from commit 707ca13)
pranikum pushed a commit to pranikum/OpenSearch that referenced this pull request Aug 2, 2022
saratvemulapalli pushed a commit that referenced this pull request Aug 2, 2022
Signed-off-by: Daniel Widdis <[email protected]>
(cherry picked from commit 707ca13)

Co-authored-by: Daniel Widdis <[email protected]>
@dbwiddis dbwiddis deleted the publish-netty4 branch August 3, 2022 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch v2.2.0 v3.0.0 Issues and PRs related to version 3.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Publish Netty module as a maven artifact
4 participants