Skip to content
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

Move old manifests to legacy #3473

Merged
merged 1 commit into from
May 4, 2023
Merged

Conversation

gaiksaya
Copy link
Member

@gaiksaya gaiksaya commented May 4, 2023

Description

Moves some old version manifests to legacy manifest folder

Issues Resolved

List any issues this PR will resolve, e.g. Closes [...].

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.

@gaiksaya gaiksaya changed the title Move old manifest to legacy and remove ml-commons from 3.0 Move old manifests to legacy and remove ml-commons from 3.0 May 4, 2023
@codecov-commenter
Copy link

codecov-commenter commented May 4, 2023

Codecov Report

Merging #3473 (8520ea0) into main (aec6965) will not change coverage.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##             main    #3473   +/-   ##
=======================================
  Coverage   91.81%   91.81%           
=======================================
  Files         181      181           
  Lines        5268     5268           
=======================================
  Hits         4837     4837           
  Misses        431      431           

@peterzhuamazon
Copy link
Member

So the only thing that is not legacy is the latest version?

@gaiksaya
Copy link
Member Author

gaiksaya commented May 4, 2023

So the only thing that is not legacy is the latest version?

Kept few more for reference in manifests folder along with latest version. Here is the list https://github.com/gaiksaya/opensearch-build/tree/ml/manifests

@gaiksaya gaiksaya mentioned this pull request May 4, 2023
@gaiksaya
Copy link
Member Author

gaiksaya commented May 4, 2023

Looks like 3.0.0 manifest check is failing for multiple components such as ml-commons, neural-search, k-NN. Should we disable CI checks for 3.0.0 OS manifest?
@bbarani @dblock @prudhvigodithi @peterzhuamazon

@gaiksaya gaiksaya changed the title Move old manifests to legacy and remove ml-commons from 3.0 Move old manifests to legacy and disable CI checks for 3.0.0 Os manifest May 4, 2023
@gaiksaya gaiksaya changed the title Move old manifests to legacy and disable CI checks for 3.0.0 Os manifest Move old manifests to legacy and disable CI checks for 3.0.0 OS manifest May 4, 2023
@dblock
Copy link
Member

dblock commented May 4, 2023

Looks like 3.0.0 manifest check is failing for multiple components such as ml-commons, neural-search, k-NN. Should we disable CI checks for 3.0.0 OS manifest? @bbarani @dblock @prudhvigodithi @peterzhuamazon

No, we should be fixing the issues!

@jmazanec15
Copy link
Member

jmazanec15 commented May 4, 2023

For k-NN, we are in process of fixing our CI. Problem is there were a couple breaks that have caused a bit of a tangle up in the k-NN CI.

The jackson/gradle upgrade (opensearch-project/OpenSearch#7300) and immutable maps removal (opensearch-project/OpenSearch#7309) have broken a few things. We raised this PR: opensearch-project/k-NN#885. However, CI is failing for secure test because the docker distribution has yet to be updated due to the jackson/gradle issue (circular). Additionally, BWC tests are failing because k-NN 2.8 has not been added to distribution yet (#3472). neural-search is stuck because it depends on k-NN.

In the long term, I believe solution is for plugins to not take dependencies on distribution for secure tests and bwc tests - instead, depend on opensearch-core min artifact and individual dependent plugin artifacts. I think this will break the circular CI dependency chain mentioned above.

In the meantime, for k-NN, I would like to merge this: #3472 so that BWC tests can pass. Then merge opensearch-project/k-NN#885 to unblock distro build and then fix neural search.

@gaiksaya gaiksaya changed the title Move old manifests to legacy and disable CI checks for 3.0.0 OS manifest Move old manifests to legacy May 4, 2023
Signed-off-by: Sayali Gaikawad <[email protected]>
@gaiksaya
Copy link
Member Author

gaiksaya commented May 4, 2023

In that case just moving the old manifests to legacy for now. 3.0.0 will remain broken until plugin teams fix the issue.
Thanks!

@gaiksaya gaiksaya merged commit ef1a068 into opensearch-project:main May 4, 2023
@gaiksaya gaiksaya deleted the ml branch May 4, 2023 22:09
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.

5 participants