Skip to content

Conversation

@kristinelacek
Copy link
Collaborator

No description provided.

@nbx0 nbx0 requested a review from Copilot June 4, 2025 18:45
Copy link

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

This PR updates the project to version 2.1.0, introducing scripts to remove and fix vulnerable Python packages, bumping container tags to latest, and adding support for a new "SC2-Spike-Only-Illumina" experiment type and primer schema.

  • Add and integrate remove_vulnerability_pkgs.sh and fixed_vulnerability_pkgs.sh in the Dockerfile
  • Update all docker-compose files to use :latest tags and ensure containers stay alive
  • Enhance app.py with SC2 spike-only Illumina support and add two new RSV primer options
  • Bump version numbers in NEWS.md and DESCRIPTION

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
remove_vulnerability_pkgs.sh Wrapper script to remove vulnerable site-packages
fixed_vulnerability_pkgs.sh Wrapper script to upgrade fixed vulnerable packages
docker-compose*.yml Switch image tags to latest, adjust bind mounts, keep containers running
Dockerfile Install vulnerability-fix scripts, run them, remove deprecated Python packages
app.py Add SC2-Spike-Only-Illumina CLI flag and two new RSV primer options
NEWS.md, DESCRIPTION Bump project version to 2.1.0
Comments suppressed due to low confidence (3)

remove_vulnerability_pkgs.sh:23

  • The loop condition is comparing the literal string i instead of the variable. It should be while [[ $i -le $n ]];.
while [[ i -le $n ]];

fixed_vulnerability_pkgs.sh:23

  • The loop condition uses i instead of $i. Update to while [[ $i -le $n ]]; to correctly evaluate the counter.
while [[ i -le $n ]];

docker-compose-git.yml:48

  • YAML merge key syntax should be <<: [...] without a space before the colon.
<< : [*base-image, *python-version, *irma-image, *dais-image]

awk NF < ${remove_pkgs} > ${remove_pkgs_clean}

# Get number of packages in the file
n=`wc -l < ${remove_pkgs_clean}`
Copy link

Copilot AI Jun 4, 2025

Choose a reason for hiding this comment

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

[nitpick] Use $(...) for command substitution instead of backticks for improved readability: n=$(wc -l < "${remove_pkgs_clean}").

Suggested change
n=`wc -l < ${remove_pkgs_clean}`
n=$(wc -l < ${remove_pkgs_clean})

Copilot uses AI. Check for mistakes.
awk NF < ${fixed_pkgs} > ${fixed_pkgs_clean}

# Get number of packages in the file
n=`wc -l < ${fixed_pkgs_clean}`
Copy link

Copilot AI Jun 4, 2025

Choose a reason for hiding this comment

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

[nitpick] Prefer n=$(wc -l < "${fixed_pkgs_clean}") over backticks for consistency and readability.

Suggested change
n=`wc -l < ${fixed_pkgs_clean}`
n=$(wc -l < "${fixed_pkgs_clean}")

Copilot uses AI. Check for mistakes.
pkg_name=$(head -${i} ${remove_pkgs_clean} | tail -1 | sed 's,\r,,g')
echo $pkg_name
# Remove the package if it exists
find /usr/local/lib/${python_version}/site-packages -name "*${pkg_name}*" -exec rm -rf {} \;
Copy link

Copilot AI Jun 4, 2025

Choose a reason for hiding this comment

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

[nitpick] Quote variable expansions to avoid word-splitting or glob issues: find "/usr/local/lib/${python_version}/site-packages" -name "*${pkg_name}*" ....

Suggested change
find /usr/local/lib/${python_version}/site-packages -name "*${pkg_name}*" -exec rm -rf {} \;
find "/usr/local/lib/${python_version}/site-packages" -name "*${pkg_name}*" -exec rm -rf {} \;

Copilot uses AI. Check for mistakes.
pkg_name=$(echo ${updated_pkg} | sed -e "s/\(.*\)\(==\)\(.*\)/\1/g")
echo $pkg_name
# Check if package already existed in pip install
check_pip_pkg=$(pip list | grep -w ${pkg_name} | sed -e "s/\s\+/==/g")
Copy link

Copilot AI Jun 4, 2025

Choose a reason for hiding this comment

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

[nitpick] Wrap ${pkg_name} in quotes in the pipeline to prevent unintended word splitting: grep -w "${pkg_name}".

Suggested change
check_pip_pkg=$(pip list | grep -w ${pkg_name} | sed -e "s/\s\+/==/g")
check_pip_pkg=$(pip list | grep -w "${pkg_name}" | sed -e "s/\s\+/==/g")

Copilot uses AI. Check for mistakes.
# Copy all files to docker images
COPY fixed_vulnerability_pkgs.txt ${MIRA_PROGRAM_DIR}/fixed_vulnerability_pkgs.txt

# Copy all files to docker images
Copy link

Copilot AI Jun 4, 2025

Choose a reason for hiding this comment

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

[nitpick] The repeated comments and individual COPY/RUN blocks increase layer count; consider combining related steps into fewer RUN/COPY instructions to optimize the image.

Copilot uses AI. Check for mistakes.
@kristinelacek kristinelacek marked this pull request as ready for review June 9, 2025 14:44
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