-
Notifications
You must be signed in to change notification settings - Fork 11
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
fix: use Python 3.10 on Ubuntu noble in Docker (#2073) #2074
fix: use Python 3.10 on Ubuntu noble in Docker (#2073) #2074
Conversation
WalkthroughThe changes include updates to the Python version in the CI workflow, specifically from Changes
Possibly related PRs
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 (
|
deps-report 🔍Commit scanned: 1716a21 Vulnerable dependencies4 dependencies have vulnerabilities 😱
Outdated dependencies63 outdated dependencies found (including 21 outdated major versions)😢
|
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: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
backend/Pipfile.lock
is excluded by!**/*.lock
📒 Files selected for processing (3)
.github/workflows/main.yml
(3 hunks)backend/Pipfile
(2 hunks)utils/docker/Dockerfile
(2 hunks)
🧰 Additional context used
📓 Learnings (1)
backend/Pipfile (1)
Learnt from: tedil
PR: varfish-org/varfish-server#2065
File: backend/Pipfile:8-8
Timestamp: 2024-10-28T12:59:54.398Z
Learning: When updating Python version support to a broader range (e.g., changing `python_version = "3.10"` to `python_version = "3"` in `backend/Pipfile`), the `altamisa` package should be upgraded to `~=0.3.0` to maintain compatibility.
🔇 Additional comments (4)
backend/Pipfile (1)
112-112
: Verify package compatibility with Python 3.10.
The Python version requirement has been made more restrictive (from "3" to "3.10"). While this aligns with the PR objective to use Python 3.10, we should verify that all dependencies support this specific version.
✅ Verification successful
Let me generate another script to get more detailed information about package compatibility.
This script will show us the specific Python version requirements for each package, which will help us verify compatibility with Python 3.10.
Let me try one more approach to verify package compatibility.
This script will query PyPI's JSON API directly to get Python version requirements for critical packages. This should give us more reliable information about version compatibility.
Python 3.10 compatibility verified for all critical dependencies
Based on the PyPI metadata, all critical packages support Python 3.10:
- Django (>=3.10)
- DjangoRestFramework (>=3.8)
- Pandas (>=3.9)
- NumPy (>=3.10)
- SQLAlchemy (>=3.7)
- Pydantic (>=3.8)
- Protobuf (>=3.8)
The version requirements show that these packages are compatible with Python 3.10, as they all specify minimum Python versions that are lower than or equal to 3.10.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if any packages have known compatibility issues with Python 3.10
# Extract package names and versions
echo "Checking package compatibility..."
cat backend/Pipfile | awk -F'=' '/^[a-zA-Z]/ && !/^\[/ {gsub(/[" =~*]/,"",$1); if($1!="") print $1}' | while read pkg; do
# Use pip to check package metadata for Python version support
pip index versions "$pkg" 2>/dev/null | head -n 1
done
Length of output: 5559
Script:
#!/bin/bash
# Get package versions and their Python version requirements
echo "Checking package metadata..."
cat backend/Pipfile | grep -v '^\[' | grep '=' | sed 's/[" =~*]//g' | while read -r line; do
pkg=$(echo "$line" | cut -d'=' -f1)
if [ ! -z "$pkg" ]; then
echo "=== $pkg ==="
pip show "$pkg" 2>/dev/null | grep -E "Requires-Python|Version"
fi
done
Length of output: 21799
Script:
#!/bin/bash
# Check PyPI API for package metadata
echo "Checking critical packages for Python 3.10 compatibility..."
# List of critical packages to check
PACKAGES=(
"django"
"djangorestframework"
"pandas"
"numpy"
"sqlalchemy"
"pydantic"
"protobuf"
)
for pkg in "${PACKAGES[@]}"; do
echo "=== $pkg ==="
curl -s "https://pypi.org/pypi/$pkg/json" | jq -r '.info.requires_python' 2>/dev/null
done
Length of output: 1329
.github/workflows/main.yml (3)
174-174
: LGTM: Python version consistent with test job.
The Python version (3.10.15) in the lint job matches the test job, maintaining consistency.
131-131
: Verify impact of removing ldap-packages category.
The removal of ldap-packages
from the pipenv install command might affect LDAP authentication functionality.
#!/bin/bash
# Description: Check LDAP package dependencies and usage
# Expected: LDAP functionality should either be removed or handled differently
# Search for LDAP-related imports and usage
echo "Searching for LDAP dependencies..."
rg -g '!.git' -i 'ldap'
# Check Pipfile for LDAP-related packages
echo "Checking Pipfile for LDAP packages..."
fd -g 'Pipfile*' --exec cat {}
122-122
: Verify Python version consistency across all environments.
The Python version update to 3.10.15 aligns with the PR objective. However, we should ensure this version is consistent with the Docker configuration.
✅ Verification successful
Python version 3.10.15 is correctly configured across environments
The Python version 3.10.15 specified in the GitHub Actions workflow matches the version installed in the Dockerfile through pyenv. The Dockerfile shows explicit installation and configuration of Python 3.10.15 using pyenv:
RUN pyenv install 3.10.15 && \
pyenv global 3.10.15 && \
pyenv local 3.10.15
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check Python version consistency across configuration files
# Expected: Python 3.10.15 in Dockerfile and other relevant files
# Search for Python version specifications
echo "Searching for Python version specifications..."
rg -g '!.git' '3\.10\.\d+'
# Search for pyenv configurations
echo "Searching for pyenv configurations..."
rg -g '!.git' 'pyenv.*3\.10'
Length of output: 312294
Script:
#!/bin/bash
# Search for Python version specifications in Dockerfile and other configuration files
echo "Searching for Python version specifications..."
rg -g '!.git' -g '!Pipfile.lock' '(python|PYTHON).*3\.10\.\d+'
# Search for Python version in Docker-related files specifically
echo "Searching in Docker-related files..."
fd -g 'Dockerfile*' -x cat {}
Length of output: 9166
@@ -5,11 +5,11 @@ name = "pypi" | |||
|
|||
[packages] | |||
aldjemy = "~=2.2" | |||
altamisa = "~=0.3.0" | |||
altamisa = "~=0.2.5" |
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.
Revert altamisa version downgrade.
Based on previous learnings, downgrading altamisa
from ~=0.3.0
to ~=0.2.5
could introduce compatibility issues. The package should be at version ~=0.3.0
to maintain compatibility with Python 3.x.
Apply this diff to fix the version:
-altamisa = "~=0.2.5"
+altamisa = "~=0.3.0"
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
altamisa = "~=0.2.5" | |
altamisa = "~=0.3.0" |
- name: Install pip and Pipenv | ||
run: pip install pip pipenv | ||
run: pip install -U pip pipenv |
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.
🛠️ Refactor suggestion
Consider caching pip packages to improve CI performance.
While updating pip is good practice, consider using GitHub Actions cache for pip and pipenv dependencies to speed up CI runs.
- name: Install pip and Pipenv
+ - uses: actions/cache@v3
+ with:
+ path: ~/.cache/pip
+ key: ${{ runner.os }}-pip-${{ hashFiles('**/Pipfile.lock') }}
+ restore-keys: |
+ ${{ runner.os }}-pip-
run: pip install -U pip pipenv
working-directory: backend
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
run: pip install -U pip pipenv | |
- uses: actions/cache@v3 | |
with: | |
path: ~/.cache/pip | |
key: ${{ runner.os }}-pip-${{ hashFiles('**/Pipfile.lock') }} | |
restore-keys: | | |
${{ runner.os }}-pip- | |
run: pip install -U pip pipenv | |
working-directory: backend |
--python=/opt/pyenv/shims/python \ | ||
--categories=packages,prod-packages,ldap-packages \ | ||
--deploy | ||
# Override installed Celery version (necessary until sodar-core v1) | ||
RUN pipenv run pip install -U celery==5.3.5 | ||
|
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.
🛠️ Refactor suggestion
Remove unnecessary --python
option from pipenv install
command
Since the base image now uses Python 3.10 by default, specifying the --python
parameter is unnecessary. You can simplify the pipenv install
command by removing the --python
option.
Apply this diff:
RUN PIPENV_VENV_IN_PROJECT=1 pipenv install \
- --python=/opt/pyenv/shims/python \
--categories=packages,prod-packages,ldap-packages \
--deploy
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
--python=/opt/pyenv/shims/python \ | |
--categories=packages,prod-packages,ldap-packages \ | |
--deploy | |
# Override installed Celery version (necessary until sodar-core v1) | |
RUN pipenv run pip install -U celery==5.3.5 | |
--categories=packages,prod-packages,ldap-packages \ | |
--deploy | |
# Install Python 3.10 using pyenv | ||
RUN apt-get install -y \ | ||
git \ | ||
wget \ | ||
build-essential \ | ||
libbz2-dev \ | ||
libffi-dev \ | ||
libgdbm-dev \ | ||
liblzma-dev \ | ||
libncurses5-dev \ | ||
libncurses-dev \ | ||
libnss3-dev \ | ||
libreadline-dev \ | ||
libsqlite3-dev \ | ||
libssl-dev \ | ||
pkg-config \ | ||
tk-dev \ | ||
zlib1g-dev | ||
RUN mkdir -p /opt && \ | ||
git clone https://github.com/yyuu/pyenv.git /opt/pyenv | ||
ENV PYENV_ROOT /opt/pyenv | ||
ENV PATH $PYENV_ROOT/shims:$PYENV_ROOT/bin:$PATH | ||
RUN pyenv install 3.10.15 && \ | ||
pyenv global 3.10.15 && \ | ||
pyenv local 3.10.15 && \ | ||
pyenv rehash && \ | ||
set -x && \ | ||
which python && \ | ||
which pip && \ | ||
python --version && \ | ||
pip --version && \ | ||
pip install -U pipenv --break-system-packages | ||
# Some cleanup |
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.
🛠️ Refactor suggestion
Simplify Python installation by using the official Python 3.10 base image
Installing Python 3.10 using pyenv
adds complexity and increases the image size. Consider using the official python:3.10-slim
base image, which already includes Python 3.10. This will simplify the Dockerfile and reduce build time.
Apply this diff to simplify the Dockerfile:
-FROM ubuntu:noble AS python-base
-
-ENV LANG C.UTF-8
-ENV LC_ALL C.UTF-8
-ENV PYTHONDONTWRITEBYTECODE 1
-ENV PYTHONFAULTHANDLER 1
-
-ENV DEBIAN_FRONTEND noninteractive
-ENV CUSTOM_STATIC_DIR /usr/src/app/local-static
-
-ENV SERVE_FRONTEND 1
-
-WORKDIR /usr/src/app
-
-# Install Python 3.10 using pyenv
-RUN apt-get install -y \
- git \
- wget \
- build-essential \
- libbz2-dev \
- libffi-dev \
- libgdbm-dev \
- liblzma-dev \
- libncurses5-dev \
- libncurses-dev \
- libnss3-dev \
- libreadline-dev \
- libsqlite3-dev \
- libssl-dev \
- pkg-config \
- tk-dev \
- zlib1g-dev
-RUN mkdir -p /opt && \
- git clone https://github.com/yyuu/pyenv.git /opt/pyenv
-ENV PYENV_ROOT /opt/pyenv
-ENV PATH $PYENV_ROOT/shims:$PYENV_ROOT/bin:$PATH
-RUN pyenv install 3.10.15 && \
- pyenv global 3.10.15 && \
- pyenv local 3.10.15 && \
- pyenv rehash && \
- set -x && \
- which python && \
- which pip && \
- python --version && \
- pip --version && \
- pip install -U pipenv --break-system-packages
-# Some cleanup
-RUN apt-get clean autoclean && \
- apt-get autoremove --yes && \
- rm -rf /var/lib/{apt,dpkg,cache,log}/
-
+FROM python:3.10-slim AS python-base
+
+LABEL org.opencontainers.image.authors="[email protected]"
+LABEL org.opencontainers.image.source https://github.com/varfish-org/varfish-server
+
+ENV LANG C.UTF-8
+ENV LC_ALL C.UTF-8
+ENV PYTHONDONTWRITEBYTECODE 1
+ENV PYTHONFAULTHANDLER 1
+ENV DEBIAN_FRONTEND noninteractive
+ENV CUSTOM_STATIC_DIR /usr/src/app/local-static
+ENV SERVE_FRONTEND 1
+
+WORKDIR /usr/src/app
+
+RUN pip install -U pipenv
Committable suggestion skipped: line range outside the PR's diff.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2074 +/- ##
======================================
Coverage ? 91%
======================================
Files ? 678
Lines ? 38532
Branches ? 0
======================================
Hits ? 35135
Misses ? 3397
Partials ? 0 |
Summary by CodeRabbit
New Features
Bug Fixes
altamisa
to ensure stability.Documentation
Pipfile
for clarity on package requirements.Chores