Skip to content

refactor: Search pipeline run refactoring and baseline unit tests#105

Merged
yuechao-qin merged 1 commit into
masterfrom
ycq/search-pipeline-run-query-params
Mar 6, 2026
Merged

refactor: Search pipeline run refactoring and baseline unit tests#105
yuechao-qin merged 1 commit into
masterfrom
ycq/search-pipeline-run-query-params

Conversation

@yuechao-qin
Copy link
Copy Markdown
Collaborator

@yuechao-qin yuechao-qin commented Feb 24, 2026

TL;DR

Refactored the list method in PipelineRunsApiService_Sql to improve code organization and added comprehensive test coverage for pipeline run operations.

What changed?

Functional

None.

Other

  • Extracted filter parsing logic into separate helper functions _resolve_filter_value and _build_filter_where_clauses
  • Moved pipeline run response creation logic into a dedicated _create_pipeline_run_response method
  • Added constants _PAGE_TOKEN_OFFSET_KEY and _PAGE_TOKEN_FILTER_KEY for page token field names
  • Added comprehensive test suite covering listing, filtering, pagination, annotation CRUD operations, and helper functions

How to test?

uv run pytest tests/test_api_server_sql.py

Run the new test suite in tests/test_api_server_sql.py which includes:

  • Tests for listing pipeline runs with various filters and pagination
  • Tests for pipeline name resolution and execution stats inclusion
  • Tests for annotation management (set, delete, list)
  • Tests for filter parsing and page token handling
  • Edge cases like empty results and unsupported filters

Why make this change?

  1. Refactoring upfront — structural changes land here so upstream PR diffs only show new functionality, not mixed refactoring noise.
  2. Baseline tests — establishing coverage now means each upstream PR's test additions map directly to its new features, making review clearer.
Comment thread cloud_pipelines_backend/api_server_sql.py Fixed
Comment thread cloud_pipelines_backend/api_server_sql.py Outdated
Comment thread cloud_pipelines_backend/api_server_sql.py Outdated
@yuechao-qin yuechao-qin force-pushed the ycq/search-pipeline-run-query-params branch from 962d30e to 44babd7 Compare February 25, 2026 04:25
Comment thread cloud_pipelines_backend/api_server_sql.py Outdated
Comment thread cloud_pipelines_backend/api_server_sql.py Outdated
Comment thread cloud_pipelines_backend/api_server_sql.py Outdated
Comment thread cloud_pipelines_backend/api_server_sql.py Fixed
@yuechao-qin yuechao-qin changed the title refactor: Search pipeline run using Pydantic query parameter refactor: Search pipeline run refactoring and baseline unit tests Feb 27, 2026
@yuechao-qin yuechao-qin force-pushed the ycq/search-pipeline-run-query-params branch 2 times, most recently from f0a5c13 to b57b335 Compare February 28, 2026 07:20
Comment thread tests/test_api_server_sql.py Outdated
Comment thread cloud_pipelines_backend/api_server_sql.py Outdated
Comment thread tests/test_api_server_sql.py
Comment thread cloud_pipelines_backend/api_server_sql.py
Copy link
Copy Markdown
Contributor

@Ark-kun Ark-kun 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. Approved with comments.

@yuechao-qin yuechao-qin force-pushed the ycq/search-pipeline-run-query-params branch from b57b335 to e7cfd12 Compare March 3, 2026 20:14
Comment thread tests/test_api_server_sql.py Fixed
Comment thread cloud_pipelines_backend/api_server_sql.py
@yuechao-qin yuechao-qin force-pushed the ycq/search-pipeline-run-query-params branch from e7cfd12 to 9fc3134 Compare March 3, 2026 20:21
elif key == "created_by":
if value == "me":
if current_user is None:
current_user = ""
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Question: Shouldn't we raise an error here if an unauthenticated user is trying to filter by me? Right now, we end doing WHERE created_by IS NULL if there is no current_user which does not feel correct

Copy link
Copy Markdown
Collaborator Author

yuechao-qin commented Mar 6, 2026

Merge activity

  • Mar 6, 8:42 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Mar 6, 8:42 PM UTC: @yuechao-qin merged this pull request with Graphite.
@yuechao-qin yuechao-qin merged commit 098f799 into master Mar 6, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants