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

Fix Query Rules validation #2677

Merged
merged 5 commits into from
Jul 8, 2024
Merged

Fix Query Rules validation #2677

merged 5 commits into from
Jul 8, 2024

Conversation

pquentin
Copy link
Member

@pquentin pquentin commented Jul 5, 2024

For reference, now that we can use Query Rules YAML tests, here is the current validation status:

API Status Request Response Stability Visibility
delete_rule 🟢 6/6 6/6 stable public
delete_ruleset 🟢 7/7 7/7 stable public
get_rule 🔴 5/5 3/5 stable public
get_ruleset 🔴 8/8 5/8 stable public
list_rulesets 🔴 7/7 1/7 stable public
put_rule 🔴 0/7 7/7 stable public
put_ruleset 🔴 4/10 10/10 stable public
Summary 🔴 74% 78%

With only looking at docs and validation errors (not the code), I've fixed validation of all APIs in four commits:

Copy link
Contributor

github-actions bot commented Jul 5, 2024

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

API Status Request Response
query_rules.delete_rule 🟢 6/6 6/6
query_rules.delete_ruleset 🟢 7/7 7/7
query_rules.get_rule 🟢 5/5 5/5
query_rules.get_ruleset 🟢 8/8 8/8
query_rules.list_rulesets 🟢 7/7 7/7
query_rules.put_rule 🟢 7/7 7/7
query_rules.put_ruleset 🟢 10/10 10/10

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

Copy link
Member

@flobernd flobernd left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you 🙂

@pquentin
Copy link
Member Author

pquentin commented Jul 5, 2024

Since this is not urgent, I'd like to wait a few working days before merging in order to give @kderusso a change to review this.

Copy link
Contributor

@l-trotta l-trotta left a comment

Choose a reason for hiding this comment

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

very nice :D

Copy link
Member

@kderusso kderusso left a comment

Choose a reason for hiding this comment

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

Thanks for resolving this!

@pquentin pquentin merged commit 95e7482 into main Jul 8, 2024
7 checks passed
@pquentin pquentin deleted the query-rules-validation branch July 8, 2024 12:23
github-actions bot pushed a commit that referenced this pull request Jul 8, 2024
* Add missing priority field

* Add fuzzy criteria type

* Allow criteria and rules to be single objects

* Fix type of rule_criteria_types_counts

* Run make contrib

(cherry picked from commit 95e7482)
l-trotta pushed a commit that referenced this pull request Jul 8, 2024
* Add missing priority field

* Add fuzzy criteria type

* Allow criteria and rules to be single objects

* Fix type of rule_criteria_types_counts

* Run make contrib

(cherry picked from commit 95e7482)

Co-authored-by: Quentin Pradet <[email protected]>
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.

4 participants