-
Notifications
You must be signed in to change notification settings - Fork 282
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
add testSnapshotFolder to path.repo #5286
Conversation
Hi @anntians could you add the same to Thanks. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5286 +/- ##
=======================================
Coverage 92.06% 92.06%
=======================================
Files 202 202
Lines 7017 7017
=======================================
Hits 6460 6460
Misses 557 557 ☔ View full report in Codecov by Sentry. |
@@ -69,6 +69,7 @@ components: | |||
additional-cluster-configs: | |||
path.repo: | |||
- /tmp | |||
- /testSnapshotFolder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this suppose to be for k-NN component?
Should be under that. Also /tmp
default exists on most of the platforms. Is testSnapshotFolder
being created by some gradle action?
Adding @peterzhuamazon @rishabh6788
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didnt realize they add it under index-management.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add the config under k-NN
component. Thanks!
@@ -69,6 +69,7 @@ components: | |||
additional-cluster-configs: | |||
path.repo: | |||
- /tmp | |||
- /testSnapshotFolder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didnt realize they add it under index-management.
Signed-off-by: AnnTian Shao <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approve under the assumption that this will be ported to 2.20 and 3.0.0-alpha1 later.
Thanks.
Description
The 2.19 release build was failing KNN integ tests https://build.ci.opensearch.org/blue/organizations/jenkins/integ-test/detail/integ-test/9318/pipeline/90. Specifically the
RestoreSnapshotIT
test. There is a prior PR opensearch-project/k-NN#2461 that added fixes to this test by adding a path topath.repo
. The current failure is related to that change. This PR attempts to fix it.Issues Resolved
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.