Skip to content

Conversation

owaiskazi19
Copy link
Member

@owaiskazi19 owaiskazi19 commented May 9, 2025

Description

In opensearch-project/job-scheduler#773, guava was removed from JS which means these deps are no longer available to AD at runtime. This PR to change the guava deps from compileOnly to implementation to ensure they are included in the assembly and available at runtime.

Related Issues

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

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff.
  • 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.

@owaiskazi19
Copy link
Member Author

Merging opensearch-project/ml-commons#3844 wouldn't require change on neural search and we can close this PR then

@heemin32
Copy link
Collaborator

heemin32 commented May 9, 2025

k-NN has com.google.guava dependency. Adding com.google.guava dependency in neural might cause jar-hell?

@martin-gaievski
Copy link
Member

k-NN has com.google.guava dependency. Adding com.google.guava dependency in neural might cause jar-hell?

should that be added at the core level to avoid conflicting versions in each plugin?

@heemin32
Copy link
Collaborator

heemin32 commented May 9, 2025

k-NN has com.google.guava dependency. Adding com.google.guava dependency in neural might cause jar-hell?

should that be added at the core level to avoid conflicting versions in each plugin?

Core has made a move to remove guava dependency from their code.

@owaiskazi19
Copy link
Member Author

k-NN has com.google.guava dependency. Adding com.google.guava dependency in neural might cause jar-hell?

Should we raise a similar PR on k-NN to switch guava deps from compileOnly to implementation for neural search to build to pass?

@owaiskazi19
Copy link
Member Author

@navneet1v @jmazanec15 @kotwanikunal can you take a look at fixing the same for k-NN?

@jmazanec15
Copy link
Member

KNN has API guava dependency (https://github.com/opensearch-project/k-NN/blob/main/build.gradle#L342-L343). So I think neural should have access via knn?

@owaiskazi19
Copy link
Member Author

closing this as #1313 is resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants