Skip to content

pipeline_steps should be used in templates#728

Merged
zeiler merged 5 commits intomasterfrom
zeiler-more-pipeline-fixes
Aug 4, 2025
Merged

pipeline_steps should be used in templates#728
zeiler merged 5 commits intomasterfrom
zeiler-more-pipeline-fixes

Conversation

@zeiler
Copy link
Copy Markdown
Member

@zeiler zeiler commented Aug 1, 2025

We should be consistent with our API in using pipeline_steps not pipeline-steps in the templates.

This make upload require some steps to upload for now so avoid confusion until lock is implemented.

This updates the version IDs for existing template that have versions to make sure the config.yaml is always the final one uploaded. This caused a hard to understand bug about step name not being defined from the API when it had an old version ID in the config.yaml (which I guess was also in the uploaded API representation from pl upload).

Also improved logging by using proto to dict.

@zeiler zeiler requested a review from nitinbhojwani August 1, 2025 21:12
@zeiler
Copy link
Copy Markdown
Member Author

zeiler commented Aug 1, 2025

@copilot fix the tests please

@zeiler zeiler requested a review from Copilot August 1, 2025 22:43
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Updates the API to consistently use pipeline_steps instead of pipeline-steps in templates and configurations. This change addresses inconsistencies in the API naming convention and fixes a bug where old version IDs in config.yaml caused "step name not defined" errors.

  • Standardizes template reference patterns from pipeline-steps to pipeline_steps across test files, validation logic, and templates
  • Updates pipeline builder to treat missing step directories as an error rather than success to prevent confusion
  • Improves error handling and logging by converting protobuf messages to dictionaries for better readability

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/cli/test_pipeline.py Updates test cases to use pipeline_steps instead of pipeline-steps in template references
clarifai/runners/utils/pipeline_validation.py Updates regex patterns and error messages to validate pipeline_steps naming convention
clarifai/runners/pipelines/pipeline_builder.py Changes return value for empty step directories and refactors template reference version updating logic
clarifai/client/pipeline.py Adds protobuf-to-dict conversion for better logging and enhanced error handling for missing pipelines
clarifai/cli/templates/pipeline_templates.py Updates pipeline config template to use pipeline_steps naming convention

Comment thread clarifai/runners/pipelines/pipeline_builder.py Outdated
zeiler and others added 4 commits August 1, 2025 18:44
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Aug 1, 2025

Code Coverage

Package Line Rate Health
clarifai 43%
clarifai.cli 44%
clarifai.cli.templates 28%
clarifai.client 68%
clarifai.client.auth 67%
clarifai.constants 100%
clarifai.datasets 100%
clarifai.datasets.export 80%
clarifai.datasets.upload 75%
clarifai.datasets.upload.loaders 37%
clarifai.models 100%
clarifai.modules 0%
clarifai.rag 72%
clarifai.runners 10%
clarifai.runners.models 59%
clarifai.runners.pipeline_steps 45%
clarifai.runners.pipelines 80%
clarifai.runners.utils 62%
clarifai.runners.utils.data_types 72%
clarifai.schema 100%
clarifai.urls 60%
clarifai.utils 52%
clarifai.utils.evaluation 67%
clarifai.workflows 95%
Summary 61% (7486 / 12178)

Minimum allowed line rate is 50%

@zeiler zeiler merged commit de8e03c into master Aug 4, 2025
11 checks passed
@zeiler zeiler deleted the zeiler-more-pipeline-fixes branch August 4, 2025 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants