feat: add template tag to preset pipelines and add list templates api#24
feat: add template tag to preset pipelines and add list templates api#24MOLYHECI merged 1 commit intoOpenDCAI:backendfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a "template" tag to preset (API) pipelines and introduces a new API endpoint to list template pipelines. The changes enable better categorization and retrieval of preset pipeline configurations.
- Adds "template" tag to newly created API pipelines
- Implements backward-compatible migration logic to add "template" tag to existing API pipelines
- Introduces new
/templatesendpoint to filter and list template pipelines
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| backend/app/services/pipeline_registry.py | Adds "template" tag to new API pipelines, implements migration logic for existing pipelines, and introduces list_templates() method to filter template pipelines |
| backend/app/api/v1/endpoints/pipelines.py | Adds new GET /templates endpoint to expose template pipeline listing functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if "template" not in tags: | ||
| tags.append("template") | ||
| pipeline_data["tags"] = tags | ||
| data["pipelines"][pipeline_id] = pipeline_data |
There was a problem hiding this comment.
The assignment on line 192 is redundant since pipeline_data is already a reference to data["pipelines"][pipeline_id]. When you modify pipeline_data["tags"] on line 191, it already modifies the original dictionary. The assignment data["pipelines"][pipeline_id] = pipeline_data doesn't change anything and can be removed to improve code clarity.
| data["pipelines"][pipeline_id] = pipeline_data |
| @router.get("/templates", response_model=ApiResponse[List[PipelineOut]], operation_id="list_template_pipelines", summary="返回所有预置(template) Pipeline列表") | ||
| def list_template_pipelines(request: Request): | ||
| try: | ||
| logger.info(f"Request: {request.method} {request.url.path}") | ||
| pipelines = container.pipeline_registry.list_templates() | ||
| logger.info(f"Successfully listed {len(pipelines)} template pipelines") | ||
| return ok(pipelines) | ||
| except Exception as e: | ||
| logger.error(f"Failed to list template pipelines: {str(e)}", exc_info=True) | ||
| raise HTTPException(500, f"Failed to list template pipelines: {str(e)}") |
There was a problem hiding this comment.
The /templates endpoint should be defined before the /{pipeline_id} endpoint. In FastAPI, route order matters - more specific routes should come before parametric routes. Currently, if someone tries to access /templates, it could be matched by the /{pipeline_id} route first with pipeline_id="templates", which would cause incorrect behavior. Move this endpoint definition to before line 84.
| def list_templates(self) -> List[Dict[str, Any]]: | ||
| """列出所有预置(template) Pipelines""" | ||
| data = self._read() | ||
| pipelines = list(data.get("pipelines", {}).values()) | ||
| templates = [p for p in pipelines if "template" in (p.get("tags") or [])] | ||
| templates.sort(key=lambda x: x.get("updated_at", ""), reverse=True) | ||
| return templates |
There was a problem hiding this comment.
The new list_templates method lacks test coverage. The test suite at backend/tests/test_pipeline_registry.py includes comprehensive tests for other methods like list_pipelines, list_executions, create_pipeline, etc., but doesn't include tests for this new method. Consider adding test cases to verify the filtering logic and sorting behavior.
No description provided.