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.

Sign up for GitHub

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

Jump to bottom

Add specification for query rule test API #3029

Merged
merged 4 commits into from
Oct 24, 2024
Merged

Add specification for query rule test API #3029

merged 4 commits into from
Oct 24, 2024

Conversation

kderusso
Copy link
Member

Adds a specification for the query rules test API added in elastic/elasticsearch#114168

@kderusso kderusso requested a review from a team as a code owner October 18, 2024 13:15
@kderusso kderusso mentioned this pull request Oct 18, 2024
Open
@github-actions GitHub Actions
Copy link
Contributor

Following you can find the validation results for the APIs you have changed.

API Status Request Response
query_rules.test 🟠 Missing recording Missing recording

You can validate these APIs yourself by using the make validate target.

@l-trotta l-trotta self-assigned this Oct 18, 2024
pquentin
pquentin reviewed Oct 18, 2024
View reviewed changes
Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

Thank you for this!

specification/query_rules/test/QueryRulesetTestRequest.ts Outdated Show resolved Hide resolved
specification/query_rules/test/QueryRulesetTestResponse.ts Outdated
*/

import { integer } from '@_types/Numeric'
import { QueryRulesetMatchedRule } from './types'
Copy link
Member

Choose a reason for hiding this comment

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

We recommend keeping classes that are used only once in the same file as the class that uses it - it's allowed as this is not Java!

@github-actions GitHub Actions
Copy link
Contributor

Following you can find the validation results for the APIs you have changed.

API Status Request Response
query_rules.test 🟠 Missing recording Missing recording

You can validate these APIs yourself by using the make validate target.

@github-actions GitHub Actions
Copy link
Contributor

Following you can find the validation results for the APIs you have changed.

API Status Request Response
query_rules.test 🟠 Missing recording Missing recording

You can validate these APIs yourself by using the make validate target.

@pquentin
Copy link
Member

pquentin commented Oct 18, 2024 •
edited
Loading

Can you please confirm whether there's a Elasticsearch YAML test for API or not?

@l-trotta
Copy link
Contributor

l-trotta commented Oct 18, 2024 •
edited
Loading

I have a question about match_criteria: in the documentation it says that:

Match criteria should match the keys defined in the criteria.metadata field of the rule.

so can it only be something like field_name: field_value? since criteria.metadata was defined as string in the spec. If so then we could use a more specific type, like Dictionary<string,string>?

everything else looks good to me!

@github-actions GitHub Actions
Copy link
Contributor

Following you can find the validation results for the APIs you have changed.

API Status Request Response
query_rules.test 🟠 Missing recording Missing recording

You can validate these APIs yourself by using the make validate target.

@kderusso
Copy link
Member Author

Can you please confirm whether there's a Elasticsearch YAML test for API or not?

@pquentin Yes, the yaml REST tests may be found here.

so can it only be something like field_name: field_value? since criteria.metadata was defined as string in the spec. If so then we could use a more specific type, like Dictionary<string,string>?

@l-trotta We can't use Dictionary<string,string> because the Java code supports Object such as numeric input.

@kderusso kderusso merged commit d14e088 into main Oct 24, 2024
6 checks passed
@kderusso kderusso deleted the kderusso/add-query-rules-test-api-to-specification branch October 24, 2024 12:11
github-actions bot pushed a commit that referenced this pull request Oct 24, 2024
@kderusso @github-actions
Add specification for query rule test API (#3029)
06754b7 
* Add query rules tester spec

* code style + output

* Update specification/query_rules/test/QueryRulesetTestRequest.ts

Co-authored-by: Quentin Pradet <[email protected]>

* PR feedback

---------

Co-authored-by: Laura Trotta <[email protected]>
Co-authored-by: Quentin Pradet <[email protected]>
(cherry picked from commit d14e088)
@github-actions github-actions bot mentioned this pull request Oct 24, 2024
Merged
github-actions bot pushed a commit that referenced this pull request Oct 24, 2024
@kderusso @github-actions
Add specification for query rule test API (#3029)
9cfc632 
* Add query rules tester spec

* code style + output

* Update specification/query_rules/test/QueryRulesetTestRequest.ts

Co-authored-by: Quentin Pradet <[email protected]>

* PR feedback

---------

Co-authored-by: Laura Trotta <[email protected]>
Co-authored-by: Quentin Pradet <[email protected]>
(cherry picked from commit d14e088)
@github-actions github-actions bot mentioned this pull request Oct 24, 2024
Merged
l-trotta pushed a commit that referenced this pull request Oct 24, 2024
@github-actions @kderusso
Add specification for query rule test API (#3029) (#3076)
5d3b33b 
* Add query rules tester spec

* code style + output

* Update specification/query_rules/test/QueryRulesetTestRequest.ts

Co-authored-by: Quentin Pradet <[email protected]>

* PR feedback

---------

Co-authored-by: Laura Trotta <[email protected]>
Co-authored-by: Quentin Pradet <[email protected]>
(cherry picked from commit d14e088)

Co-authored-by: Kathleen DeRusso <[email protected]>
l-trotta pushed a commit that referenced this pull request Oct 24, 2024
@github-actions @kderusso
Add specification for query rule test API (#3029) (#3077)
c324fba 
* Add query rules tester spec

* code style + output

* Update specification/query_rules/test/QueryRulesetTestRequest.ts

Co-authored-by: Quentin Pradet <[email protected]>

* PR feedback

---------

Co-authored-by: Laura Trotta <[email protected]>
Co-authored-by: Quentin Pradet <[email protected]>
(cherry picked from commit d14e088)

Co-authored-by: Kathleen DeRusso <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@l-trotta l-trotta l-trotta approved these changes

@pquentin pquentin Awaiting requested review from pquentin

Assignees

@l-trotta l-trotta

Labels
backport 8.x backport 8.16 specification
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kderusso @pquentin @l-trotta