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

[Backport Main] Add path.repo for registering snapshot repository in integ tests #2463

Conversation

anntians
Copy link
Contributor

Description

A prior PR #2445 was breaking the release build of 2.19 due to #2458. This PR fixes the issue by updating the path.repo used to register snapshot repositories in integ tests

Related Issues

Resolves #2458

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.

@anntians
Copy link
Contributor Author

This PR is a fix for a previous PR #2445. Can I get help with adding the skip-changelog label to this PR. Thanks

@anntians anntians changed the title Add path.repo for registering snapshot repository in integ tests [Backport Main] Add path.repo for registering snapshot repository in integ tests Jan 29, 2025
@anntians anntians force-pushed the backport/backport-2461-to-main branch from 2dc6738 to a9ed1db Compare January 30, 2025 18:53
@@ -53,7 +53,7 @@ public class ModelIT extends AbstractRestartUpgradeTestCase {
private static final int DELAY_MILLI_SEC = 1000;
private static final int MIN_NUM_OF_MODELS = 2;
private static final int K = 5;
private static final int NUM_DOCS = 10;
private static final int NUM_DOCS = 1100;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this change needed ? Don't see any correlation with this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is related to another backport PR #2443 about updating error messages for faiss training. That other backport PR made the same fix as above, but it hasn't been merged yet, which is why I made this change in this PR to get the tests passing

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. We should not make it as part of this PR then and let other PR merge, then rebase this PR on top of it before merging.

@kotwanikunal
Copy link
Member

@anntians Can you please rebase?

…nsearch-project#2461)

Signed-off-by: AnnTian Shao <[email protected]>
Co-authored-by: AnnTian Shao <[email protected]>
(cherry picked from commit 168ee3c)
@anntians anntians force-pushed the backport/backport-2461-to-main branch from 6bd941a to 06a292f Compare February 25, 2025 17:37
@anntians anntians requested a review from Vikasht34 as a code owner February 25, 2025 17:37
@anntians anntians force-pushed the backport/backport-2461-to-main branch from 06a292f to 88c230e Compare February 25, 2025 17:52
Signed-off-by: AnnTian Shao <[email protected]>
@anntians anntians force-pushed the backport/backport-2461-to-main branch 3 times, most recently from 019a8d2 to bc9dbe9 Compare February 26, 2025 04:14
Signed-off-by: Tommy Shao <[email protected]>
Signed-off-by: AnnTian Shao <[email protected]>
@anntians anntians force-pushed the backport/backport-2461-to-main branch from bc9dbe9 to 8c032b8 Compare February 26, 2025 16:48
@jed326 jed326 added the v3.0.0 label Feb 27, 2025
@naveentatikonda naveentatikonda merged commit f4779a5 into opensearch-project:main Feb 27, 2025
38 checks passed
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.

[AUTOCUT] Integration Test Failed for k-NN-2.19.0
6 participants