feat: Search annotations in pipeline run API#107
feat: Search annotations in pipeline run API#107yuechao-qin wants to merge 1 commit intoycq/search-pipeline-run-pydantic-modelsfrom
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
78fdccf to
f7dc9bc
Compare
41a18a2 to
7606c83
Compare
98e7d41 to
ecd7842
Compare
f7dc9bc to
b5f0ba9
Compare
b5f0ba9 to
88b265f
Compare
ecd7842 to
ba5594e
Compare
ba5594e to
d6f04df
Compare
88b265f to
9a0930e
Compare
|
|
||
| @dataclasses.dataclass(kw_only=True) | ||
| class PageToken: | ||
| offset: int = 0 |
There was a problem hiding this comment.
Will you be moving to cursor way of filtering? If you plan to make changes soon, this class should not be public. Otherwise it's a breaking change.
Maybe you should just make the whole module private. Like _api_server_search_utils.py. ("utils" is an anti-pattern, but I do not have a better idea yet.)
|
|
||
| # # Needed to put a union type into DB | ||
| # class SqlIOTypeStruct(_BaseModel): | ||
| # type: structures.TypeSpecType |
There was a problem hiding this comment.
This indentation was correct (it's a class variable inside a class).
| predicate, | ||
| ) -> sql.ColumnElement: | ||
| match predicate: | ||
| case filter_query_models.AndPredicate(): |
There was a problem hiding this comment.
I don't like that we construct all these different objects just for comparison.
You can do:
match type(predicate):
case filter_query_models.AndPredicate:| ) | ||
| clause = filter_query_sql.filter_query_to_where_clause(filter_query=fq) | ||
| compiled = _compile(clause) | ||
| assert "ml-ops" in compiled |
There was a problem hiding this comment.
Maybe we can assert the whole query in some of these tests? It would be nice to see the full queries here.
I guess, there is a possibility that this query could be brittle and change with new version of SqlAlchemy. But we can wait for that moment and then make tests more robust if that happens.
d6f04df to
7369ffc
Compare
9a0930e to
fdbdc71
Compare

TL;DR
Implemented annotation-based filtering for Pipeline Run API and proper pagination handling.
What changed?
API
GET /api/pipeline_runs/Functionality
filter_querywhich handles annotation searches in pipeline runs.(key, pipeline_run_id, value).filterand newfilter_queryacross pages.Other
filter_query_sql.pymodule with comprehensive filter query processing:PageTokenclass for encoding/decoding pagination statekey_exists,value_equals,value_contains,value_inand,or,notwith proper nestingHow to test?
{"and": [{"key_exists": {"key": "team"}}]}{"and": [{"value_equals": {"key": "env", "value": "prod"}}]}and/or/notpredicatesfilterandfilter_queryparametersWhy make this change?
Annotation search support is the backbone for all the upcoming search features (
create_by,pipeline name,status) for Pipeline Runs. The other searches will be search against the annotations table.