-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Query rules retriever #114855
Query rules retriever #114855
Conversation
….ingest.geoip.HttpClientTests elastic#112618
…pmTracingRestIT testTracingCrossCluster elastic#112731
* Add data stream template validation to snapshot restore * Add data stream template validation to data stream promotion endpoint * Add new assertion for response headers Add a new assertion to synchronously execute a request and check the response contains a specific warning header * Test for warning header on snapshot restore When missing templates * Test for promotion warnings * Add documentation for the potential error states * PR changes * Spotless reformatting * Add logic to look in snapshot global metadata This checks if the snapshot contains a matching template for the DS * Comment on test cleanup to explain it was copied * Removed cluster service field
@elasticmachine merge upstream |
@elasticmachine update branch |
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.
Thanks @kderusso ! Some minor comments only, but this looks good!
server/src/main/java/org/elasticsearch/search/retriever/CompoundRetrieverBuilder.java
Outdated
Show resolved
Hide resolved
...main/java/org/elasticsearch/xpack/application/rules/retriever/QueryRuleRetrieverBuilder.java
Outdated
Show resolved
Hide resolved
...main/java/org/elasticsearch/xpack/application/rules/retriever/QueryRuleRetrieverBuilder.java
Show resolved
Hide resolved
...main/java/org/elasticsearch/xpack/application/rules/retriever/QueryRuleRetrieverBuilder.java
Show resolved
Hide resolved
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.
Thanks @kderusso
I left more comments.
We also need a test that extends AbstractXContentTestCase<QueryRuleRetrieverBuilder>
to validate the rest parsing.
server/src/main/java/org/elasticsearch/search/retriever/CompoundRetrieverBuilder.java
Outdated
Show resolved
Hide resolved
...main/java/org/elasticsearch/xpack/application/rules/retriever/QueryRuleRetrieverBuilder.java
Show resolved
Hide resolved
...arch/src/main/java/org/elasticsearch/xpack/application/rules/retriever/QueryRuleRankDoc.java
Outdated
Show resolved
Hide resolved
...t/src/yamlRestTest/resources/rest-api-spec/test/entsearch/rules/80_query_rules_retriever.yml
Show resolved
Hide resolved
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.
LGTM!
server/src/main/java/org/elasticsearch/search/retriever/CompoundRetrieverBuilder.java
Outdated
Show resolved
Hide resolved
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.
LGTM
💔 Backport failed
You can use sqren/backport to manually backport by running |
(cherry picked from commit 690ad1e)
Example script to demonstrate: