fix: use conda only in cadd scripts #18
Conversation
WalkthroughThe changes in this pull request focus on the Changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (2)helpers/CADD_mocker.sh (2)
The implementation uses proper atomic file operations:
The script now expects the output path in Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
restapi/tasks.py (1)
Line range hint
42-53: Enhance error handling for conda-specific failures.Since we're enforcing conda-only usage, consider improving the error handling to catch and report conda-specific issues:
cmdline = [ "bash", "-xc", ( - "{conda_activate}{cadd_sh} -m -a -g {genome_build} -v {cadd_release} " + "{conda_activate:s}{cadd_sh} -m -a -g {genome_build} -v {cadd_release} " ).format( conda_activate=("source %s; " % CADD_CONDA) if CADD_CONDA else "", cadd_sh=CADD_SH, genome_build=args["genome_build"], cadd_release=args["cadd_release"], tmpdir=tmpdir, ), ] + # Validate conda environment + if not CADD_CONDA and not os.getenv("CONDA_DEFAULT_ENV"): + raise ValueError("No conda environment specified. Set CADD_CONDA or activate a conda environment.")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
restapi/tasks.py(1 hunks)
🔇 Additional comments (2)
restapi/tasks.py (2)
42-42: Verify CI/CD and deployment environment compatibility.
Since we're enforcing conda-only usage with the -m flag, we need to ensure all environments where this code runs have conda properly configured.
Let's check the CI configuration and deployment scripts:
✅ Verification successful
Conda configuration is properly handled in the codebase
The verification shows that:
- Conda is properly configured in the Docker environment through
utils/docker/Dockerfile - The code is designed to be flexible with conda activation through the
CADD_CONDAenvironment variable inconfig/settings/base.py - If
CADD_CONDAis not set, the code gracefully handles it by not including the conda activation command - The Docker setup ensures CADD scripts are installed via conda-forge
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check CI configuration for conda setup
fd -g "*ci*yml" -g "*workflow*yml" -X cat {}
# Check deployment/environment setup scripts
fd -g "*env*" -g "*setup*" -g "*deploy*" -X cat {}
Length of output: 354
Script:
#!/bin/bash
# Let's try a different approach to find CI/CD and environment setup files
# Look for common CI config files
fd -e yml -e yaml | grep -iE 'ci|workflow|github|gitlab|jenkins|azure|travis'
# Look for environment setup files
fd -e sh -e py -e yml -e yaml | grep -iE 'env|setup|deploy|install|requirements|conda'
# Also check if there's a conda environment file
fd -g "environment.y*ml"
Length of output: 278
Script:
#!/bin/bash
# Let's examine the installation scripts and setup.py to check for conda dependencies
cat utility/install_os_dependencies.sh
cat utility/install_python_dependencies.sh
cat setup.py
# Also check for any conda-related strings in the codebase
rg -i "conda" -A 2 -B 2
Length of output: 8024
42-42: Verify the impact of the -m flag addition.
The addition of the -m flag to CADD.sh command line arguments needs verification to ensure it's the correct flag for conda-only execution.
Let's verify the CADD.sh documentation and usage:
Consider documenting the meaning of these flags in a comment for future maintainability:
- "{conda_activate}{cadd_sh} -m -a -g {genome_build} -v {cadd_release} "
+ "{conda_activate}{cadd_sh} -m -a -g {genome_build} -v {cadd_release} " # -m: use conda only, -a: annotate
stolpeo
force-pushed
the
use-conda-only-in-cadd-scripts
branch
from
7c53c97 to
102afdc
Compare
1 participant
Summary by CodeRabbit
Bug Fixes
Chores