-
Notifications
You must be signed in to change notification settings - Fork 125
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
[FEATURE] Publish opensearch-knn jar #1351
Comments
Adding @vamshin @navneet1v to please take look at this issue. |
@erezgong we do publish the jars. We have another plugin neural search that takes dependency on k-NN plugin. Ref: https://github.com/opensearch-project/neural-search/blob/main/build.gradle#L143 Also publishing actions for gradle: https://github.com/opensearch-project/k-NN/blob/main/build.gradle#L90-L122 which is called here: https://github.com/opensearch-project/k-NN/blob/main/.github/workflows/maven-publish.yml So not sure what exactly we need with this issue? |
Hey @navneet1v, the published type is zip and the gradle logic uses the zip file, unzip it and uses the jar inside the zip file. Notice for job-scheduler we have both the jar and zip published. The ask is to do the same for k-NN to publish the jar file as well (Should be under https://aws.oss.sonatype.org/content/repositories/releases/org/opensearch/opensearch-knn/). |
@prudhvigodithi you can check the same here for k-NN: https://aws.oss.sonatype.org/content/repositories/releases/org/opensearch/plugin/opensearch-knn/ |
@navneet1v the published ones are of type zip |
@prudhvigodithi on checking further I can see that only job-schedular plugin is publishing the jars. All other plugins are publishing the zips. In the light of that evidence I don't think so k-NN should publish the jars separately. |
@navneet1v no there are more example opensearch-ml-client, opensearch-custom-codecs, opensearch-geo, for more check this link https://aws.oss.sonatype.org/content/repositories/releases/org/opensearch/, Also it depends on the plugin team based on the use case. |
Fyi Ml-client is not a plugin. Its more of a library to call the ml apis from plugin to interact with ml commons. plugins that i dont see is ml commons, sql, alterting, security. here is my understanding if all the jars are present in the zip then why to publish the jars separately. There was campaign which was started after which all the plugins started publishing their zips from plugin repo only and not the opensearch ci. The plugins who are still publishing the jars could have been from left over code where we have added the jar publish code so that other plugins can take dependency on. I don’t see any precedent from opensearch project where plugins needs to publish their jars mandatorily. Please provide a strong usecase why k-nm should publish the jars too when zip have the same thing in it |
Thanks @navneet1v, FYI this ask was from @erezgong I'm just trying to emphasize on a point (as I was tagged in the issue) on which its not very hard to publish a jar of k-nn to maven when k-nn acts as an upstream, even neural search uses k-nn as upstream which adds a logic to crack open the zip and use the jar, Refer: https://github.com/opensearch-project/neural-search/blob/main/build.gradle#L161-L165 @navneet1v also here is the campaign opensearch-project/opensearch-build#1916 to publish the plugin zips to maven which is an enhancement and I dont think jar publish code is the left over code. Also if this the ask from the customer why not? :) I would suggest @erezgong to please add the usecase that would help k-nn team to get more details. |
To only depend on k-NN jar, see neural example here: https://github.com/opensearch-project/neural-search/blob/main/build.gradle#L253. This should be good enough. Closing |
Is your feature request related to a problem?
Cannot use opensearch-knn as a regular dependency in maven and IntelliJ.
What solution would you like?
Publish jar, like you do in other plugins.
@prudhvigodithi
The text was updated successfully, but these errors were encountered: