-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Remove SEARCH_PIPELINE feature flag (#8513) #8738
Remove SEARCH_PIPELINE feature flag (#8513) #8738
Conversation
fda23ee
to
24814cb
Compare
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
d09627a
to
8945bb0
Compare
Gradle Check (Jenkins) Run Completed with:
|
8945bb0
to
414e123
Compare
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
Need rerun on the flaky test. Also requesting review since this PR is for search pipelien GA in 2.9. @reta @andrross @saratvemulapalli |
414e123
to
b66407e
Compare
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Louis Chu <clingzhi@amazon.com>
b66407e
to
7a3b799
Compare
Gradle Check (Jenkins) Run Completed with:
|
@noCharger could you please dig through gradle check failures and fix failures or link to existing flaky tests if those are the problem? |
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.
am I correct that without the feature flag the feature can be enabled by making a certain query?
Gradle Check (Jenkins) Run Completed with:
|
@dblock This is for search pipeline feature GA. The gradle check failed on mixed-cluster test because the node with new code can't find the flag setting from the node with old code and vice versa (see #8513 (comment) for the full evaluation). It's related to this issue. |
Right now you have to enable it by setting
|
Why did we not remove the experimental flag in #8613 and decided to just flip it? I would like to confirm that this feature is meant to go GA, aka not experimental and not behind a feature flag, in 2.9, yes? |
Yes search pipeline is ready to go GA in 2.9 We started with removing the experimental flag in #8513 and use #8613 as an short term mitigation on this mixed cluster test issue: #8513 (comment). |
Thanks @noCharger. There's an order in which these PRs turn 🟢, let's try to push them as such. |
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.
so, we've cut 2.9 branch, which makes this a chicken-and-egg problem - I am going to make a call and merge this with failing bcw tests, and we're going to ensure CI / gradle check passes on 2.9 after it's merged, even if we need to retry
Signed-off-by: Louis Chu <clingzhi@amazon.com> (cherry picked from commit 1164221) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
(cherry picked from commit 1164221) Signed-off-by: Louis Chu <clingzhi@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Description
Manual backport #8513. See details in #8513 (comment)
Check List
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.