[FEATURE] Avoid making backward incompatible changes and have mechanism to ensure it

[chishui]

Is your feature request related to a problem?
Recently, there was one breaking change in kNN which caused neural-search repo workflow build failures and blocked PR merging for several days.

What solution would you like?
Can we try to avoid making backward incompatible changes by modifying the signatures of public entities as it could easily break the build of all packages which depend on k-NN.

Some mechanisms to ensure it'll happen less likely:

1. Add checklist item in PR asking PR owner/reviewer to check if they've made breaking changes

What alternatives have you considered?
This is not an alternative solution but a supplement option.

1. Add neural-search's build in workflow, it can be a non-blocking check but works as signal

Do you have any additional context?
Add any other context or screenshots about the feature request here.

[heemin32]

@chishui I think this is a corresponding PR to fix the build failure in neural search. https://github.com/opensearch-project/neural-search/pull/836/files. k-NN plugin tries to keep backward compatibility for its users but not another plugin which uses k-NN plugin's class directly.

For the PR, I think it should be possible to write the code without using k-NN's class directly?

[chishui]

@heemin32 thanks for the prompt reply. I don't get why another plugin using k-NN library directly makes it not a k-NN user. I think when changing signatures of public functions of a library, it's better to consider how many external libraries are depending on it and could be broke by the change. If it's not used anywhere except for the library itself, it's totally fine to change the signature, but if it is used, maybe have the existing functions to call new functions and mark existing ones as deprecated could be a better option.

[jmazanec15]

I like approach taken by core (https://github.com/opensearch-project/OpenSearch/tree/0040f4b766459610fae3a0342f26d8f78735778e/libs/common/src/main/java/org/opensearch/common/annotation). We should have public (major version compatibility guarantees), experimental (maybe will be promoted to public), and internal (no compatibility guarantees).

[heemin32]

@chishui I didn't regard another plugins as k-NN's user because k-NN is not built for being used as a library or for supporting extension point where other plugins can implement their own feature on top of it. In that sense, except the API and its behavior, all classes are internal.

The OpenSearch core have made some of its class as public because it supports an extension point which plugins will rely upon.

I think it is time to see why neural search is directly accessing k-nn plugins class and if it is necessary or not. If accessing k-NN plugin class is unavoidable, we can take an approach that OpenSearch core took. (Thanks for sharing it @jmazanec15)

[jmazanec15]

See https://github.com/search?q=repo%3Aopensearch-project%2Fneural-search+%22org.opensearch.knn%22&type=code. The main thing that is kind of a hard dependency is the KNNQueryBuilder. Things like space type might also matter though.