Repository navigation
feat: add --commit flag to deploy-workflow - #127
Conversation
snakedeploy deploy-workflow previously only allowed pinning a workflow deployment to a --tag or --branch, even though the underlying Snakemake github() helper also supports pinning to an exact commit via commit=. This meant users wanting full reproducibility (pinning to a specific commit rather than a potentially moving branch, or a repo with no tags) had to manually edit the generated Snakefile after deployment. This adds a --commit option to the deploy-workflow subcommand: - --commit can be used standalone, or combined with --branch/--tag. - When given, it takes precedence for both the local checkout performed during deployment and the ref written into the generated module's github(...) call (commit="<sha>" instead of tag=/branch=). - The local/Gitlab providers were updated accordingly, and a new test case was added to tests/test_client.sh. - Docs in workflow_deployment.rst updated to document the new flag.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughWorkflow deployment now requires exactly one of ChangesCommit-pinned deployment
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as deploy-workflow CLI
participant Deployer as WorkflowDeployer
participant Repository as Git repository
participant Provider as Github provider
CLI->>Deployer: pass selected commit
Deployer->>Repository: checkout commit
Deployer->>Provider: request source declaration with commit
Provider-->>Deployer: return commit-based declaration
Suggested reviewers: Merge Risk: ⚪ Minimal · up to Workflow deployments can now be pinned to a commit while tag, branch, and commit selections are enforced as mutually exclusive. The checkout, generated source declaration, documentation, and validation coverage are aligned, with no remaining merge-blocking risk identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 27.27% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 4 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ntnn19
changed the title
ntnn19
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
tests/test_client.sh (1)
54-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest checkout precedence, not only declaration rendering.
The current assertion would pass even if checkout still used the branch or skipped commit checkout. Add a commit-only invocation and verify a copied file or focused unit test reflects the requested commit.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_client.sh` around lines 54 - 55, The deploy-workflow test currently verifies only that the commit is rendered in the Snakefile, not that checkout uses it. Extend the test around the existing runTest invocations with a commit-only deployment and assert that a copied file or focused unit test matches the requested commit, ensuring commit checkout takes precedence over the branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@snakedeploy/client.py`:
- Around line 92-98: The precedence documentation must state that commit
overrides branch and tag for both checkout and generated declarations. Update
the --commit help text in snakedeploy/client.py lines 92-98 and the public API
docstring in snakedeploy/deploy.py lines 283-284 to describe this precedence
consistently.
In `@snakedeploy/deploy.py`:
- Around line 261-262: Preserve positional API compatibility by placing the
existing force parameter before the newly added commit parameter in both
deploy() and WorkflowDeployer.__init__. Update snakedeploy/deploy.py lines
261-262 and 21-24 respectively; keep all other parameter behavior unchanged.
In `@snakedeploy/providers.py`:
- Around line 77-80: Ensure the Local provider does not silently ignore a
requested commit: either implement commit-aware checkout in Local.checkout or
explicitly reject --commit for Local before deployment proceeds. Update the
related get_source_file_declaration flow only as needed to preserve the
requested snapshot behavior.
In `@tests/test_client.sh`:
- Around line 54-55: Quote the new shell expansions in the deploy-workflow test:
update the commit argument to use "${commit_sha}" and the Snakefile path passed
to grep to use "${dest}/workflow/Snakefile", preserving the existing runTest
behavior.
---
Nitpick comments:
In `@tests/test_client.sh`:
- Around line 54-55: The deploy-workflow test currently verifies only that the
commit is rendered in the Snakefile, not that checkout uses it. Extend the test
around the existing runTest invocations with a commit-only deployment and assert
that a copied file or focused unit test matches the requested commit, ensuring
commit checkout takes precedence over the branch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9d53391b-09b9-4175-99a4-903703d0f657
📒 Files selected for processing (5)
docs/workflow_users/workflow_deployment.rstsnakedeploy/client.pysnakedeploy/deploy.pysnakedeploy/providers.pytests/test_client.sh
| def get_source_file_declaration( | ||
| self, path: str, tag: str, branch: str, commit: Optional[str] = None | ||
| ): | ||
| relative_path = path.replace(self.source_url, "").strip(os.sep) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Do not silently ignore --commit for local repositories.
repo_clone invokes provider.checkout(..., commit), but Local.checkout() is a no-op and this declaration cannot encode the commit. A local deployment therefore copies the current working tree rather than the requested snapshot. Either implement commit checkout for Git-backed local sources or reject --commit for the Local provider.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@snakedeploy/providers.py` around lines 77 - 80, Ensure the Local provider
does not silently ignore a requested commit: either implement commit-aware
checkout in Local.checkout or explicitly reject --commit for Local before
deployment proceeds. Update the related get_source_file_declaration flow only as
needed to preserve the requested snapshot behavior.
| runTest 0 $output snakedeploy deploy-workflow "${repo}" "${dest}" --branch master --commit ${commit_sha} --name dna-seq-commit | ||
| runTest 0 $output grep "commit=\"${commit_sha}\"" ${dest}/workflow/Snakefile |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Quote the new shell variables.
commit_sha and the Snakefile path are expanded unquoted, allowing word splitting or glob expansion. Use "${commit_sha}" and "${dest}/workflow/Snakefile".
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 54-54: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 55-55: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 55-55: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_client.sh` around lines 54 - 55, Quote the new shell expansions in
the deploy-workflow test: update the commit argument to use "${commit_sha}" and
the Snakefile path passed to grep to use "${dest}/workflow/Snakefile",
preserving the existing runTest behavior.
Source: Linters/SAST tools
|
Thank you so much! |
🤖 I have created a release *beep* *boop* --- ## [0.17.0](v0.16.2...v0.17.0) (2026-09-22) ### Features * add --commit flag to deploy-workflow ([#127](#127)) ([9bb3cc7](9bb3cc7)) ### Bug Fixes * empty PRs for when meta wrappers have no changes ([#128](#128)) ([54d8c48](54d8c48)) * Fix --version ([#125](#125)) ([fb1ea2b](fb1ea2b)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2 participants
snakedeploy deploy-workflow previously only allowed pinning a workflow deployment to a --tag or --branch, even though the underlying Snakemake github() helper also supports pinning to an exact commit via commit=. This meant users wanting full reproducibility (pinning to a specific commit rather than a potentially moving branch, or a repo with no tags) had to manually edit the generated Snakefile after deployment.
This adds a --commit option to the deploy-workflow subcommand:
LLM usage disclosure: LLM tools were used to assist in generating parts of the code and PR description. The resulting changes were manually reviewed, edited, and tested by the author before submission.
Summary by CodeRabbit
New Features
--commit.--commit,--tag, or--branchmust be provided.Documentation