refactor: Move code to appropriate folders for CODEOWNERS#199
refactor: Move code to appropriate folders for CODEOWNERS#199yuechao-qin wants to merge 1 commit intomasterfrom
Conversation
yuechao-qin
force-pushed
the
ycq/refactor-folders-codeowners
branch
from
549f6b8 to
628450d
Compare
yuechao-qin
force-pushed
the
ycq/refactor-folders-codeowners
branch
from
628450d to
cef2844
Compare
There was a problem hiding this comment.
Let's leave it in api_server. Too small.
There was a problem hiding this comment.
Let's keep it where it was.
There was a problem hiding this comment.
Let's move this to search/pipeline_run_annotations.py
There was a problem hiding this comment.
General style:
All module names should be either plural or uncountable nouns (to avoid clashes with local variables). "search" seems kind of fine. Although if we were a search system dealing with many searches, it would not hav been fine.
Generic catch-all module names like "utils" should be avoided as they tend to accumulate unrelated functionality.
| class ListPipelineJobsResponse: | ||
| pipeline_runs: list[PipelineRunResponse] | ||
| next_page_token: str | None = None | ||
|
|
There was a problem hiding this comment.
I think that such "response" classes belong to api_server_sql.
I suggest returning pipeline_runs + next page token
|
|
||
| return ListPipelineJobsResponse( | ||
| pipeline_runs=[ | ||
| self._create_pipeline_run_response( | ||
| session=session, | ||
| pipeline_run=pipeline_run, | ||
| include_pipeline_names=include_pipeline_names, | ||
| include_execution_stats=include_execution_stats, | ||
| ) | ||
| for pipeline_run in pipeline_runs | ||
| ], | ||
| next_page_token=next_page_token, | ||
| ) | ||
|
|
||
| def _create_pipeline_run_response( | ||
| self, | ||
| *, | ||
| session: orm.Session, | ||
| pipeline_run: bts.PipelineRun, | ||
| include_pipeline_names: bool, | ||
| include_execution_stats: bool, | ||
| ) -> PipelineRunResponse: | ||
| response = PipelineRunResponse.from_db(pipeline_run) | ||
| if include_pipeline_names: | ||
| pipeline_name = None | ||
| extra_data = pipeline_run.extra_data or {} | ||
| if self._PIPELINE_NAME_EXTRA_DATA_KEY in extra_data: | ||
| pipeline_name = extra_data[self._PIPELINE_NAME_EXTRA_DATA_KEY] |
There was a problem hiding this comment.
Let's keep this code in this module.
The basic functionality unrelated to search should remain here.
2 participants
Overview
Closes https://github.com/Shopify/oasis-frontend/issues/525
Reorganized
cloud_pipelines_backend/by extracting search, annotation, execution, and backfill logic from the monolithicapi_server_sql.pyinto dedicated subpackages (i.e. folders). Tests were split and relocated to mirror the new source structure. No behavioral changes.New Directories
cloud_pipelines_backend/search/cloud_pipelines_backend/annotation/cloud_pipelines_backend/execution/ExecutionStatusSummarydataclasscloud_pipelines_backend/backfill/database_migrations.py)Why make this change?
The new dedicated folders can be applied to the appropriate
CODEOWNERS.