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

Implement mongodb query syntax for task filters #258

Merged
merged 20 commits into from
Sep 10, 2024
Merged

Conversation

msm-cert
Copy link
Member

@msm-cert msm-cert commented Aug 12, 2024

This adopts code from https://github.com/kapouille/mongoquery to implement a mongodb-like syntax for task filtering in karton.

As mentioned in the code, there are some pitfalls since the old negative matching works in three-state logic and can't be translated to the new query syntax literally. For a short example:

[
  {"platform": "!win32"},
  {"platform": "!linux"}
]

will match all non-linux non-windows samples, as was probably intended, but:

[
  {"platform": {"$not": "win32"}},
  {"platform": {"$not": "linux"}}
]

is wrong - this means literally task.headers["platform"] != "win32" or task.headers["platform"] != "linux" and hence will match every task.

To get equivalent behaviour with mongo syntax, you should use:

[{
  "platform": {
    "$not": { "$or": ["win32", "linux"] }
  }
}]

See #246 for original reasoning behind the old-style behaviour.

Background compatibility considerations

  1. "Old style" filters should keep working (any regression here is a bug). We don't plan to remove or obsolete that syntax in the forseeable future. No migration is necessary.
  2. It's not recommended to mix mongodb-style filters and negative matches, to avoid unexpected behaviour. We may actually want to explicitly check it and forbid it?
  3. "Nesting" old style wildcards and mongodb syntax doesn't work. "platform": {"$or": ["win*", "linux*"]} will only work for a win* literal, for example.

@msm-cert msm-cert requested a review from psrok1 August 12, 2024 16:26
tests/test_task_filters.py Outdated Show resolved Hide resolved
tests/test_task_filters.py Outdated Show resolved Hide resolved
tests/test_task_filters.py Outdated Show resolved Hide resolved
karton/core/task.py Outdated Show resolved Hide resolved
Comment on lines 326 to 327
regular_filter, negative_filter = [], []
negative_filter = []
Copy link
Member

Choose a reason for hiding this comment

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

Repeated negative_filter definition

Suggested change
regular_filter, negative_filter = [], []
negative_filter = []
regular_filter, negative_filter = [], []

@psrok1 psrok1 changed the title Implement mongodb query syntax Implement mongodb query syntax for task filters Aug 14, 2024
@psrok1
Copy link
Member

psrok1 commented Aug 14, 2024

closes #245

@psrok1 psrok1 linked an issue Aug 14, 2024 that may be closed by this pull request
Copy link
Member

@psrok1 psrok1 left a comment

Choose a reason for hiding this comment

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

Looks good overall: it would be nice to have an entry in documentation about new syntax and its limitations.

@psrok1 psrok1 merged commit 89a885d into master Sep 10, 2024
6 checks passed
@psrok1 psrok1 deleted the feature/mongo-query branch September 10, 2024 09:28
@psrok1 psrok1 mentioned this pull request Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add more options for matching task headers
3 participants