Skip to content

Move gRPC transitive dependencies to expected version #18672

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

Merged
merged 4 commits into from
Jul 9, 2025

Conversation

finnegancarroll
Copy link
Contributor

@finnegancarroll finnegancarroll commented Jul 2, 2025

Description

A couple of dependencies in the gRPC transport plugin expect slightly different transitive dependency versions than what we explicitly force in build.gradle.

grpc-netty-shaded:1.68.2 expects perfmark-api:0.27.0 but we use 0.26.0.
guava:33.2.1-jre expects failureaccess:1.0.2 but we use 1.0.1.

As core explicitly declares a version for these transitive dependencies gradle does not see a conflict. However when a plugin wants to include org.opensearch.plugin:transport-grpc:<version>-SNAPSHOT as a dependency gradle sees both the original expected transitive dependency and the forced version included in the snapshot and cannot immediately resolve this conflict.

   > Could not resolve com.google.guava:failureaccess:1.0.1.
      > Conflict found for module 'com.google.guava:failureaccess': between versions 1.0.3 and 1.0.1
   > Could not resolve io.perfmark:perfmark-api:0.27.0.
      > Conflict found for module 'io.perfmark:perfmark-api': between versions 0.27.0 and 0.26.0

To avoid requiring plugins to force dependency versions or include a resolution strategy we should match versions exactly.

Related Issues

N/A

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

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.

@finnegancarroll finnegancarroll marked this pull request as ready for review July 2, 2025 23:03
@finnegancarroll finnegancarroll requested a review from a team as a code owner July 2, 2025 23:03
@kotwanikunal
Copy link
Member

@finnegancarroll Do you mind adding the version bump to the changelog?

Copy link
Contributor

github-actions bot commented Jul 3, 2025

❌ Gradle check result for 70e1a6c: 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 Jul 3, 2025

✅ Gradle check result for d4d927a: SUCCESS

Copy link

codecov bot commented Jul 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.61%. Comparing base (871fe29) to head (404acd2).
Report is 10 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #18672      +/-   ##
============================================
- Coverage     72.68%   72.61%   -0.08%     
+ Complexity    68420    68404      -16     
============================================
  Files          5569     5571       +2     
  Lines        314467   314710     +243     
  Branches      45614    45676      +62     
============================================
- Hits         228580   228531      -49     
- Misses        67347    67605     +258     
- Partials      18540    18574      +34     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cwperks
Copy link
Member

cwperks commented Jul 4, 2025

@finnegancarroll do we need to declare these transitive dependencies explicitly?

@finnegancarroll
Copy link
Contributor Author

@finnegancarroll do we need to declare these transitive dependencies explicitly?

I believe we enforce explicitly listing all transitive dependencies. I'm not sure exactly where this is enforced but do see java.lang.NoClassDefFoundError on removing these dependencies.

Copy link
Contributor

github-actions bot commented Jul 7, 2025

❌ Gradle check result for 017881e: null

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 Jul 7, 2025

❌ Gradle check result for 017881e: 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 Jul 7, 2025

❌ Gradle check result for 017881e: 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 Jul 7, 2025

❌ Gradle check result for 017881e: 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 Jul 7, 2025

❌ Gradle check result for 017881e:

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 Jul 8, 2025

❌ Gradle check result for 404acd2: 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 Jul 8, 2025

❌ Gradle check result for 404acd2: 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 Jul 8, 2025

❌ Gradle check result for 404acd2:

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 Jul 8, 2025

❌ Gradle check result for 404acd2: 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 Jul 9, 2025

❌ Gradle check result for 404acd2: 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 Jul 9, 2025

❌ Gradle check result for 404acd2: 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 Jul 9, 2025

❌ Gradle check result for 404acd2: 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 Jul 9, 2025

✅ Gradle check result for 404acd2: SUCCESS

@mch2 mch2 merged commit b609a50 into opensearch-project:main Jul 9, 2025
247 of 263 checks passed
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.

4 participants