feat: Search created by user in pipeline run API#108
feat: Search created by user in pipeline run API#108yuechao-qin wants to merge 1 commit intoycq/search-pipeline-run-annotationsfrom
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. |
6789217 to
01de682
Compare
41a18a2 to
7606c83
Compare
01de682 to
9a3927b
Compare
7606c83 to
98e7d41
Compare
9a3927b to
0e3ac9b
Compare
98e7d41 to
ecd7842
Compare
0e3ac9b to
df1b586
Compare
ecd7842 to
ba5594e
Compare
df1b586 to
05e42b2
Compare
05e42b2 to
afa68d5
Compare
ba5594e to
d6f04df
Compare
afa68d5 to
527caa1
Compare
527caa1 to
7b5d7cb
Compare
| backfill_created_by_annotations(db_engine=db_engine) | ||
|
|
||
|
|
||
| def is_annotation_key_already_backfilled( |
There was a problem hiding this comment.
- It's a public function. Let's make it private.
- The name mentions "annotation", but that work is used everywhere. We need to be more specific.
_is_pipeline_run_annotation_key_already_backfilled
| ).scalar() | ||
|
|
||
|
|
||
| def backfill_created_by_annotations(*, db_engine: sqlalchemy.Engine): |
There was a problem hiding this comment.
- It's a public function. Let's make it private.
- The name mentions "annotation", but that work is used everywhere. We need to be more specific.
_backfill_pipeline_run_created_by_annotations
| SYSTEM_KEY_PREFIX: Final[str] = "system/" | ||
|
|
||
|
|
||
| class SystemKey(enum.StrEnum): |
There was a problem hiding this comment.
PipelineRunAnnotationSystemKey
| def _get_predicate_key(*, predicate: filter_query_models.Predicate) -> str | None: | ||
| """Extract the annotation key from a leaf predicate, or None for logical operators.""" | ||
| match predicate: | ||
| case filter_query_models.KeyExistsPredicate(): |
There was a problem hiding this comment.
You can have some base class like filter_query_models.KeyPredicateBase with field key. Then getting the key from the predicates would be trivial.
| *, | ||
| predicate, | ||
| predicate: filter_query_models.Predicate, | ||
| current_user: str | None = None, |
There was a problem hiding this comment.
Maybe it's simpler to first preprocess the predicates so that you don't need to do that in functions like _predicate_to_clause ?
|
|
||
| @pytest.fixture() | ||
| def session_factory(): | ||
| engine = sqlalchemy.create_engine( |
There was a problem hiding this comment.
Let's use the create_db_engine function from database_ops (without migration).
|
|
||
| class TestIsAnnotationKeyAlreadyBackfilled: | ||
| def test_false_on_empty_db(self, session_factory): | ||
| engine = session_factory.kw["bind"] |
There was a problem hiding this comment.
Maybe, is_annotation_key_already_backfilled can just accept session_factory instead of engine?
| skip_user_check: bool = False, | ||
| ): | ||
| if key.startswith(filter_query_sql.SYSTEM_KEY_PREFIX): | ||
| raise errors.InvalidAnnotationKeyError(_SYSTEM_KEY_RESERVED_MSG) |
There was a problem hiding this comment.
Maybe, self._fail_if_changing_system_annotation(key)?

TL;DR
Implemented search
created_byuser in Pipeline Runs.What changed?
Functionality
API
GET /api/pipeline_runs/created_byuser infilter_query. Example query (not URL encoded for example):/api?filter_query={"and": [{"value_equals": {"key": "system/pipeline_run.created_by", "value": "alice@example.com"}}]}meis supported forcreated_bysearch.API
POST /api/pipeline_runs/created_bydata to the annotations table for future searching. Example pipeline run annotations table after API is called:abc123system/pipeline_run.created_byalice@example.comAPI
[PUT|DELETE] /api/pipeline_runs/{id}/annotations/{key}system/prefix. Reserved for system annotation usage.Other
created_byannotations. Example what a pipeline run's created by user would look like in the annotations table:How to test?
created_byvalues{"and": [{"value_equals": {"key": "system/pipeline_run.created_by", "value": "alice"}}]}to search by creator"me"placeholder:{"and": [{"value_equals": {"key": "system/pipeline_run.created_by", "value": "me"}}]}value_contains) are rejectedWhy make this change?
create_by) to be searchable with the new filter.created_byto annotations table when starting a Pipeline Run