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

Abstracting outbound side of transport #13293

Merged
merged 11 commits into from
May 13, 2024

Conversation

VachaShah
Copy link
Collaborator

@VachaShah VachaShah commented Apr 18, 2024

Description

Coming from #13178 (comment) and as a next step to #12967, abstracting and adding support for multiple protocols on outbound side of transport.

Related Issues

Resolves #[Issue number to be closed when this PR is merged]
Part of #10684

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)
  • Public documentation issue/PR created

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.

Copy link
Contributor

❕ Gradle check result for 0a9c4d8: UNSTABLE

  • TEST FAILURES:
      1 org.opensearch.http.SearchRestCancellationIT.testAutomaticCancellationDuringQueryPhase

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

Copy link

codecov bot commented Apr 18, 2024

Codecov Report

Attention: Patch coverage is 95.31250% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 71.52%. Comparing base (b15cb0c) to head (cc43b4b).
Report is 273 commits behind head on main.

Files Patch % Lines
...ransport/nativeprotocol/NativeOutboundHandler.java 95.65% 1 Missing and 1 partial ⚠️
...ransport/nativeprotocol/NativeOutboundMessage.java 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #13293      +/-   ##
============================================
+ Coverage     71.42%   71.52%   +0.10%     
- Complexity    59978    61039    +1061     
============================================
  Files          4985     5054      +69     
  Lines        282275   287150    +4875     
  Branches      40946    41607     +661     
============================================
+ Hits         201603   205373    +3770     
- Misses        63999    64813     +814     
- Partials      16673    16964     +291     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@VachaShah
Copy link
Collaborator Author

@reta @dblock Let me know what you guys think.

@VachaShah VachaShah force-pushed the abstract-outbound-transport branch 2 times, most recently from 9e629ae to 0ab2aec Compare April 29, 2024 21:23
Copy link
Contributor

❌ Gradle check result for 9e629ae: FAILURE

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?

Copy link
Contributor

❌ Gradle check result for 0ab2aec: FAILURE

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?

Copy link
Contributor

github-actions bot commented May 7, 2024

❌ Gradle check result for c904eda: FAILURE

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?

Signed-off-by: Vacha Shah <[email protected]>
@VachaShah VachaShah requested a review from reta May 8, 2024 19:18
Copy link
Contributor

github-actions bot commented May 8, 2024

❌ Gradle check result for 0a7c0a3: FAILURE

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?

Copy link
Contributor

github-actions bot commented May 9, 2024

✅ Gradle check result for 5048253: SUCCESS

@VachaShah VachaShah requested a review from reta May 9, 2024 17:44
Copy link
Collaborator

@reta reta left a comment

Choose a reason for hiding this comment

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

@VachaShah pefrect, thank you! @dblock mind taking a look please, thank you?

Copy link
Contributor

github-actions bot commented May 9, 2024

✅ Gradle check result for cc43b4b: SUCCESS

@dblock
Copy link
Member

dblock commented May 13, 2024

Looks great, let's merge it.

@dblock dblock merged commit 14f1c43 into opensearch-project:main May 13, 2024
31 of 32 checks passed
@opensearch-trigger-bot
Copy link
Contributor

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:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-2.x
# Create a new branch
git switch --create backport/backport-13293-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 14f1c43c108f378b13d109ade364216c082fb858
# Push it to GitHub
git push --set-upstream origin backport/backport-13293-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-2.x

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

@dblock
Copy link
Member

dblock commented May 13, 2024

@VachaShah manual backport pls :(

VachaShah added a commit to VachaShah/OpenSearch that referenced this pull request May 13, 2024
* Abstracting outbound side of transport

Signed-off-by: Vacha Shah <[email protected]>

* Making outbound handler protocol dependent via inbound handler

Signed-off-by: Vacha Shah <[email protected]>

* Fixing precommit

Signed-off-by: Vacha Shah <[email protected]>

* Addressing comments

Signed-off-by: Vacha Shah <[email protected]>

* Fixing precommit

Signed-off-by: Vacha Shah <[email protected]>

* Cleaning up code

Signed-off-by: Vacha Shah <[email protected]>

* Addressing comments

Signed-off-by: Vacha Shah <[email protected]>

* Cleaning up

Signed-off-by: Vacha Shah <[email protected]>

* Addressing comments

Signed-off-by: Vacha Shah <[email protected]>

* Abstracting InboundHandlerTests

Signed-off-by: Vacha Shah <[email protected]>

* Abstracting TransportLoggerTests

Signed-off-by: Vacha Shah <[email protected]>

---------

Signed-off-by: Vacha Shah <[email protected]>
@VachaShah
Copy link
Collaborator Author

@VachaShah manual backport pls :(

Thank you @dblock! Added the backport in #13656.

reta pushed a commit that referenced this pull request May 14, 2024
* Abstracting outbound side of transport



* Making outbound handler protocol dependent via inbound handler



* Fixing precommit



* Addressing comments



* Fixing precommit



* Cleaning up code



* Addressing comments



* Cleaning up



* Addressing comments



* Abstracting InboundHandlerTests



* Abstracting TransportLoggerTests



---------

Signed-off-by: Vacha Shah <[email protected]>
deshsidd pushed a commit to deshsidd/OpenSearch that referenced this pull request May 17, 2024
* Abstracting outbound side of transport

Signed-off-by: Vacha Shah <[email protected]>

* Making outbound handler protocol dependent via inbound handler

Signed-off-by: Vacha Shah <[email protected]>

* Fixing precommit

Signed-off-by: Vacha Shah <[email protected]>

* Addressing comments

Signed-off-by: Vacha Shah <[email protected]>

* Fixing precommit

Signed-off-by: Vacha Shah <[email protected]>

* Cleaning up code

Signed-off-by: Vacha Shah <[email protected]>

* Addressing comments

Signed-off-by: Vacha Shah <[email protected]>

* Cleaning up

Signed-off-by: Vacha Shah <[email protected]>

* Addressing comments

Signed-off-by: Vacha Shah <[email protected]>

* Abstracting InboundHandlerTests

Signed-off-by: Vacha Shah <[email protected]>

* Abstracting TransportLoggerTests

Signed-off-by: Vacha Shah <[email protected]>

---------

Signed-off-by: Vacha Shah <[email protected]>
parv0201 pushed a commit to parv0201/OpenSearch that referenced this pull request Jun 10, 2024
* Abstracting outbound side of transport

Signed-off-by: Vacha Shah <[email protected]>

* Making outbound handler protocol dependent via inbound handler

Signed-off-by: Vacha Shah <[email protected]>

* Fixing precommit

Signed-off-by: Vacha Shah <[email protected]>

* Addressing comments

Signed-off-by: Vacha Shah <[email protected]>

* Fixing precommit

Signed-off-by: Vacha Shah <[email protected]>

* Cleaning up code

Signed-off-by: Vacha Shah <[email protected]>

* Addressing comments

Signed-off-by: Vacha Shah <[email protected]>

* Cleaning up

Signed-off-by: Vacha Shah <[email protected]>

* Addressing comments

Signed-off-by: Vacha Shah <[email protected]>

* Abstracting InboundHandlerTests

Signed-off-by: Vacha Shah <[email protected]>

* Abstracting TransportLoggerTests

Signed-off-by: Vacha Shah <[email protected]>

---------

Signed-off-by: Vacha Shah <[email protected]>
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 backport-failed v2.15.0 Issues and PRs related to version 2.15.0
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants