-
Notifications
You must be signed in to change notification settings - Fork 15
feat: bring your own condarc #43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThe changes introduce a new optional Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GitHub Actions Runner
participant Snakemake Action
participant Conda Setup
User->>GitHub Actions Runner: Push workflow with condarc-file input
GitHub Actions Runner->>Snakemake Action: Start action with condarc-file
Snakemake Action->>Snakemake Action: Validate condarc-file exists
alt condarc-file found
Snakemake Action->>Conda Setup: Setup Miniconda with condarc-file
Conda Setup->>Snakemake Action: Conda environment configured
else condarc-file missing
Snakemake Action->>GitHub Actions Runner: Fail with error
end
Snakemake Action->>GitHub Actions Runner: Run workflow with custom Conda config
Possibly related PRs
Suggested reviewers
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (7)
action.yml (3)
68-78: Generate default environment only when no custom file is provided
Usingif: ${{ inputs.environment-file == '' }}properly scopes the generation of.snakemake.environment.yaml.Optionally add comments or logging to clarify that this file is overwritten on each run.
80-90: Validate presence of custom environment file before setup
This early-fail guard is essential to prevent confusing downstream errors. For stricter error handling, consider enablingpipefailand an explicit exit code:- shell: bash -el {0} + shell: bash -eo pipefail {0}
97-97: Simplify environment-file selection for Conda setup
The current expression works but can be simplified using GitHub Actions’ fallback operator:- environment-file: ${{ inputs.environment-file != '' && inputs.environment-file || '.snakemake.environment.yaml' }} + environment-file: ${{ inputs.environment-file || '.snakemake.environment.yaml' }}README.md (4)
31-34: Fix grammar and document default forsnakemake-version
Add “the” before “latest version” and note the default:Snakemake version to use. If not specified, uses the latest version (`*`). Pin a specific version (e.g., '8.25.5') for reproducibility.🧰 Tools
🪛 LanguageTool
[uncategorized] ~33-~33: You might be missing the article “the” here.
Context: ... version to use. If not specified, uses latest version. Pin a specific version (e.g., ...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
35-38: Include default value forinstall-apptainer
Clarify default behavior to matchaction.yml:Install Apptainer (true/false). Default: `false`.
39-42: Document default forenvironment-fileinput
Specify default and behavior when omitted:Path to a custom Conda environment file containing Snakemake (e.g., 'etc/custom-environment.yml'). Default: `''` (generates a default environment).
61-61: Enhance example usage forenvironment-file
The example is clear; consider showing both cases (with and without custom file):# Without custom environment file: with: directory: '.test' snakefile: 'workflow/Snakefile' # environment-file omitted => default environment is generated
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(2 hunks)action.yml(3 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~33-~33: You might be missing the article “the” here.
Context: ... version to use. If not specified, uses latest version. Pin a specific version (e.g., ...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🔇 Additional comments (1)
action.yml (1)
41-44: Correctly introduceenvironment-fileinput for custom Conda environments
The newenvironment-fileparameter allows users to override the default environment YAML. Defaults to''to trigger auto-generation of.snakemake.environment.yaml.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.test/environment.yaml (1)
1-6: Add trailing newline
YAML files should end with a newline to satisfy linters and avoid potential parsing issues.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 6-6: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/main.yml(1 hunks).test/environment.yaml(1 hunks)action.yml(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- action.yml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.test/environment.yaml
[error] 6-6: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (1)
.github/workflows/main.yml (1)
25-31: Add CI test for custom environment-file
The new step correctly invokes the action with theenvironment-fileinput, ensuring the feature is exercised in CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (6)
action.yml (1)
76-76: Remove trailing whitespace
Line 76 contains unnecessary trailing spaces. Please trim to satisfy YAML lint rules.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 76-76: trailing spaces
(trailing-spaces)
.github/workflows/main.yml (2)
36-36: Trim trailing spaces
Line 36 has trailing whitespace; please remove to clean up the workflow file.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 36-36: trailing spaces
(trailing-spaces)
40-40: Fix indentation understeps
The- uses:entry undertest-condarcstepsis indented 4 spaces instead of the expected 6. Please adjust to match other job definitions.🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 40-40: wrong indentation: expected 6 but found 4
(indentation)
README.md (3)
31-33: Minor grammar: add missing article
Change to “If not specified, uses the latest version.” for smoother reading.🧰 Tools
🪛 LanguageTool
[uncategorized] ~33-~33: You might be missing the article “the” here.
Context: ... version to use. If not specified, uses latest version. Pin a specific version (e.g., ...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
39-42: Documentcondarc-fileinput
Thecondarc-filesection is consistent withaction.yml. You could mention that leaving it empty falls back to default Conda settings.
61-61: Example usage forcondarc-file
Including this example is helpful. Verify that the code block indentation matches other examples.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/main.yml(2 hunks).test/condarc.yaml(1 hunks)README.md(2 hunks)action.yml(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- .test/condarc.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/main.yml
[error] 36-36: trailing spaces
(trailing-spaces)
[warning] 40-40: wrong indentation: expected 6 but found 4
(indentation)
action.yml
[error] 76-76: trailing spaces
(trailing-spaces)
🪛 LanguageTool
README.md
[uncategorized] ~33-~33: You might be missing the article “the” here.
Context: ... version to use. If not specified, uses latest version. Pin a specific version (e.g., ...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🔇 Additional comments (6)
action.yml (3)
41-43: Add and document thecondarc-fileinput
The newcondarc-fileparameter is clearly defined with a description and default value. Ensure the downstream steps correctly handle an empty input.
77-87: Validate existence of custom Conda config early
Good approach to fail fast if the specifiedcondarc-fileis missing. Consider aligning the error output style (quotes, capitalization) with other validation steps.
95-96: Passcondarc-fileto setup-miniconda
Theconda-remove-defaultsandcondarc-fileoptions correctly configure the Conda environment. No further changes required here..github/workflows/main.yml (2)
11-11: Rename CI job frombuildtotest
The renamed job better reflects its purpose. Confirm there are no downstream references to the old job name.
37-49: Addtest-condarcjob for custom Conda config
This new job effectively tests thecondarc-filefeature. The steps appear correct and comprehensive.🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 40-40: wrong indentation: expected 6 but found 4
(indentation)
README.md (1)
35-37: Documentinstall-apptainerinput
The newinstall-apptainerparameter is clearly described with valid options. Good addition.
|
Hi @jakevc |
|
I would like to wait with this. Reason: Snakemake will soon move away from conda to directly using rattler for conda package deployment. This would deprecate the desired option. |
Summary by CodeRabbit