-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Add failureaccess dependency to transport-grpc module #19339
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
Add failureaccess dependency to transport-grpc module #19339
Conversation
- Change failureaccess from compileOnly to runtimeOnly to bundle it with transport-grpc - Add required SHA, LICENSE, and NOTICE files for failureaccess-1.0.2.jar - This fixes NoClassDefFoundError for InternalFutureFailureAccess in plugins that use Guava concurrency features - Resolves classloader isolation issues between transport-grpc and plugin dependencies Signed-off-by: karenx <[email protected]>
|
❌ Gradle check result for 1419ecf: 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? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is the best path forward at the moment for fixing KNN CI. It seems like there is some issue where KNN is deferring to transport-grpc class loader which can no longer load failureaccess since being moved to compile only.
@navneet1v @andrross @cwperks can you take a look?
Some additional context in these KNN PRs:
opensearch-project/k-NN#2885
opensearch-project/k-NN#2883
opensearch-project/k-NN#2878
Signed-off-by: karenx <[email protected]>
0a55f9f to
2d5d45e
Compare
|
❕ Gradle check result for 2d5d45e: UNSTABLE Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #19339 +/- ##
============================================
- Coverage 72.89% 72.89% -0.01%
+ Complexity 69870 69838 -32
============================================
Files 5673 5673
Lines 320754 320754
Branches 46367 46367
============================================
- Hits 233824 233819 -5
+ Misses 68012 68001 -11
- Partials 18918 18934 +16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@karenyrx , @andrross can we merge this change? k-NN build is broken and we are running red for our 10.3 upgrade fixes. I tested this change by doing this on the OpenSearch code with this commit. The output shows that tests are running which were failing earlier due to class def not found related to failure access. |
…ject#19339) Signed-off-by: Ankit Jain <[email protected]>
…ject#19339) Signed-off-by: Ankit Jain <[email protected]>
Description
Change failureaccess from compileOnly to runtimeOnly to bundle it with transport-grpc
The merging of b9c5bc7#diff-1767b7f709666659fcea718930d017913630119903232332e9b9b08627ca7c0fR29 caused the transport-grpc module to fail during runtime with NoClassDefFoundError for InternalFutureFailureAccess.
Related Issues
Resolves #[Issue number to be closed when this PR is merged]
Check List
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.