chore: deprecate python 3.8 #289
Conversation
WalkthroughThe changes in this pull request primarily involve updates to the Python version used across various GitHub Actions workflows and project files. The workflows for building and testing packages have been modified to utilize Python 3.9 instead of Python 3.8. Additionally, the integration tests have seen a refinement in the Python version strategy matrix and toolbox image tags. Documentation in the README has been updated to clarify installation and migration processes, while the Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 1
🧹 Outside diff range and nitpick comments (6)
.github/workflows/tests-unit.yml (3)
10-10: LGTM: Python version matrix updated correctly.The removal of Python 3.8 from the test matrix aligns with the PR objective to deprecate support for this version. The updated matrix now includes all currently supported Python versions (3.9, 3.10, 3.11, and 3.12), which is excellent for ensuring compatibility across these versions.
Consider setting up a process to regularly review and update this matrix as new Python versions are released and older ones reach their end of life.
41-41: LGTM: Coverage reporting condition updated correctly.The condition for reporting coverage to Coveralls has been appropriately updated to use Python 3.9, which is now the oldest supported version in the test matrix. This change is consistent with the removal of Python 3.8 support.
Consider updating the comment to be more specific:
if: matrix.python-version == '3.9' # Run this for the oldest supported Python version only!This clarifies the intent and makes it easier to maintain in the future if the oldest supported version changes again.
Line range hint
1-45: Consider adding caching to improve workflow efficiency.While the current workflow is correct and functional, you could potentially improve its efficiency by implementing caching for Poetry dependencies. This would speed up subsequent runs of the workflow by reusing previously installed packages.
Here's an example of how you could implement caching:
- name: Cache Poetry dependencies uses: actions/cache@v2 with: path: ~/.cache/pypoetry key: ${{ runner.os }}-poetry-${{ hashFiles('**/poetry.lock') }} restore-keys: | ${{ runner.os }}-poetry- - name: Install dependencies using Poetry run: | echo "[INFO] Installing dependencies..." export ENV POETRY_VIRTUALENVS_CREATE=false export PATH="${PATH}:${HOME}/.poetry/bin" poetry install --with=devAdd this step just before the "Install dependencies using Poetry" step. This can significantly reduce the time taken for installing dependencies in subsequent runs.
.github/workflows/release-pypi-build-push-package.yml (1)
Line range hint
1-68: Consider the following improvements to enhance the workflow:
Virtual Environment: The workflow explicitly disables Poetry's virtual environment creation. While this might be intentional, it could lead to potential conflicts. Consider using a virtual environment for better isolation.
Git Push Timing: The
git pushstep is performed after the package upload. If the upload fails, this could lead to inconsistencies. Consider moving thegit pushstep before the upload or implementing a rollback mechanism.Error Handling: Implement explicit error handling and consider adding a rollback mechanism for critical steps like version bumping and package uploading.
Caching: Implement caching for Poetry dependencies to speed up subsequent runs. You can use GitHub Actions'
actions/cachefor this purpose.Python Version Matrix: Consider using a matrix strategy to test against multiple Python versions (e.g., 3.9, 3.10, 3.11) to ensure compatibility.
Here's an example of how you could implement caching and a Python version matrix:
jobs: build_push: strategy: matrix: python-version: ['3.9', '3.10', '3.11'] runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 with: ref: master - name: Set up Python ${{ matrix.python-version }} uses: actions/setup-python@v2 with: python-version: ${{ matrix.python-version }} - name: Cache Poetry dependencies uses: actions/cache@v2 with: path: ~/.cache/pypoetry key: ${{ runner.os }}-poetry-${{ hashFiles('**/poetry.lock') }} restore-keys: | ${{ runner.os }}-poetry- # ... rest of the steps ...This change would test the package against multiple Python versions and implement caching for Poetry dependencies.
🧰 Tools
🪛 actionlint
15-15: the runner of "actions/setup-python@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
README.md (2)
83-83: LGTM: Updated supported Python versionsThe removal of Python 3.8.x from the list of supported versions aligns with the PR objective to deprecate Python 3.8 support. This change is consistent with the project's goal of maintaining support for more recent Python versions.
Consider adding a brief note explaining why Python 3.8 support has been dropped (e.g., "Python 3.8 has reached its end of life"). This would provide context for users who might be affected by this change.
136-136: LGTM: Updated development environment setup instructionsThe switch to Poetry for dependency management is a positive change, as it provides better dependency resolution and project isolation. The instructions are clear and easy to follow.
Consider adding a brief explanation of why Poetry is preferred over previous methods (e.g., "Poetry provides better dependency resolution and project isolation"). This would help users understand the benefits of this change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- .github/workflows/release-pypi-build-push-package.yml (1 hunks)
- .github/workflows/release-pypi-build-push-test-package.yml (1 hunks)
- .github/workflows/tests-integration.yaml (1 hunks)
- .github/workflows/tests-unit.yml (2 hunks)
- README.md (4 hunks)
- pyproject.toml (1 hunks)
🧰 Additional context used
🔇 Additional comments (11)
pyproject.toml (3)
28-28: LGTM: Python version specification updated correctlyThe Python version specification has been updated to remove support for Python 3.8, which aligns with the PR objective to deprecate Python 3.8. The new specification correctly supports Python versions 3.9, 3.10, 3.11, and 3.12.
Line range hint
1-65: Changes align with PR objectivesThe modifications to the
pyproject.tomlfile are in line with the PR objective to deprecate Python 3.8 support. The removal of Python 3.8 from the supported versions list (line 28) and the absence of Python 3.8 in the classifiers section effectively implement this deprecation.
Line range hint
1-28: Verify complete removal of Python 3.8 dependenciesThe explicit references to Python 3.8 have been removed from the
pyproject.tomlfile. However, it's important to ensure that there are no implicit dependencies or configurations in the project that might still rely on Python 3.8 specific features or behaviors.Please run the following script to check for any remaining references to Python 3.8 in the project:
If any results are found, please review them to ensure they don't indicate remaining dependencies on Python 3.8.
Also applies to: 29-65
.github/workflows/release-pypi-build-push-package.yml (1)
17-17: LGTM: Python version updated correctly.The change from Python 3.8 to 3.9 aligns with the PR objective to deprecate Python 3.8 support.
Let's verify if this change is consistent across all workflow files:
✅ Verification successful
Verified: Python version consistently updated across all workflow files.
All workflow files now use Python 3.9 or later, aligning with the PR objective to deprecate Python 3.8 support.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if all workflow files use Python 3.9 or later # Test: Search for Python version specifications in workflow files echo "Checking Python versions in workflow files:" rg --type yaml 'python-version:' .github/workflows/Length of output: 705
.github/workflows/release-pypi-build-push-test-package.yml (4)
Line range hint
1-67: Overall, the workflow update looks good.The change to use Python 3.9 is consistent with the PR objective of dropping support for Python 3.8. The rest of the workflow remains unchanged, which should be fine as long as all dependencies and build processes are compatible with Python 3.9. The suggested verifications for Poetry version and package compatibility will help ensure a smooth transition.
🧰 Tools
🪛 actionlint
19-19: the runner of "actions/setup-python@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
21-21: Python version update aligns with PR objectives.The change from Python 3.8 to 3.9 is consistent with the PR objective of dropping support for Python 3.8 due to its EOL status. This update ensures that the package is built and tested using a supported Python version.
To ensure consistency across the repository, let's check if other workflow files have been updated similarly:
#!/bin/bash # Description: Check for Python version specifications in other workflow files # Test: Search for Python version specifications in other workflow files echo "Checking Python version specifications in other workflow files:" rg --type yaml 'python-version:' .github/workflows
Line range hint
26-26: Verify Poetry version.The workflow is using Poetry version 1.8.2. While this is a recent version, it's worth checking if there's a newer stable version available that might include improvements or bug fixes relevant to Python 3.9 support.
To check the latest Poetry version:
🧰 Tools
🪛 actionlint
19-19: the runner of "actions/setup-python@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
Line range hint
30-33: Verify package compatibility with Python 3.9.With the update to Python 3.9, it's important to ensure that all dependencies and build processes are compatible with this version. Consider reviewing the
pyproject.tomlfile to check if any dependencies need updates or if any build configurations need adjustments for Python 3.9.Let's check the
pyproject.tomlfile for Python version specifications:✅ Verification successful
Package Compatibility Verified
The
pyproject.tomlspecifies Python versions:~3.9 || ~3.10 || ~3.11 || ~3.12, confirming compatibility with Python 3.9.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check Python version specifications in pyproject.toml # Test: Search for Python version specifications in pyproject.toml echo "Python version specifications in pyproject.toml:" rg 'python' pyproject.tomlLength of output: 315
Script:
#!/bin/bash # Description: Check Python version specifications in pyproject.toml # Search for Python version specifications in pyproject.toml echo "Python version specifications in pyproject.toml:" rg 'python' pyproject.tomlLength of output: 198
🧰 Tools
🪛 actionlint
19-19: the runner of "actions/setup-python@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
README.md (3)
53-53: LGTM: Updated Terraform image tag formatThe TERRAFORM_IMAGE_TAG has been updated to the new format '1.5.0-0.2.0', which aligns with the migration instructions provided. This change ensures compatibility with the new version of Leverage CLI.
230-230: LGTM: Updated Release Candidate version formatThe change in the Release Candidate version format from '1.2.3-rc.1' to '1.2.3rc.1' has been noted. This update ensures consistency in version naming conventions.
Could you please clarify if this change in the Release Candidate version format is related to the Python 3.8 deprecation or if it's a separate improvement? This context would be helpful for understanding the scope of changes in this PR.
Line range hint
1-230: Overall assessment: Changes align with PR objectiveThe updates to the README.md file successfully address the deprecation of Python 3.8 support and provide clear instructions for users on supported versions, installation, and development environment setup. The switch to Poetry for dependency management is a positive change that enhances project management.
Key points:
- Removed Python 3.8.x from supported versions.
- Updated development environment setup instructions to use Poetry.
- Minor updates to Terraform image tag format and Release Candidate version format.
These changes effectively communicate the new requirements and processes to users and contributors.
| python-version: ['3.9.15', '3.10.8', '3.11.8', '3.12.5'] | ||
| toolbox-image-tag: ['1.3.5-0.2.0', '1.5.0-0.2.0', '1.6.0-0.2.0'] |
There was a problem hiding this comment.
💡 Codebase verification
❌ Toolbox Image Tags Missing
The new toolbox image tags (1.3.5-0.2.0, 1.5.0-0.2.0, 1.6.0-0.2.0) do not exist in the registry. Please ensure that these tags are correctly built and pushed to the registry to maintain compatibility with the integration tests.
🔗 Analysis chain
LGTM on Python version update. Verify toolbox image tag compatibility.
The removal of Python 3.8.14 from the test matrix aligns with the PR objective to deprecate Python 3.8 support. This change is approved.
The toolbox image tags have been updated to newer versions (1.3.5-0.2.0, 1.5.0-0.2.0, 1.6.0-0.2.0). While this update is likely beneficial, it's important to ensure these new versions are compatible with the project requirements and don't introduce any breaking changes.
To verify the compatibility of the new toolbox image tags, please run the following script:
This script will help identify any significant changes between the old and new toolbox image tags. Please review the output to ensure compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the new toolbox image tags exist and compare their contents with the old ones
old_tags=("1.3.5-0.1.15" "1.5.0-0.1.15" "1.6.0-0.1.15")
new_tags=("1.3.5-0.2.0" "1.5.0-0.2.0" "1.6.0-0.2.0")
for i in "${!new_tags[@]}"; do
old_tag=${old_tags[$i]}
new_tag=${new_tags[$i]}
echo "Comparing ${old_tag} with ${new_tag}:"
# Check if the new tag exists
if ! docker pull "your-registry/toolbox:${new_tag}" &> /dev/null; then
echo "Error: New tag ${new_tag} does not exist!"
continue
fi
# Compare the contents of the old and new images
diff <(docker run --rm "your-registry/toolbox:${old_tag}" ls -R /) <(docker run --rm "your-registry/toolbox:${new_tag}" ls -R /)
if [ $? -eq 0 ]; then
echo "No differences found between ${old_tag} and ${new_tag}"
else
echo "Differences found between ${old_tag} and ${new_tag}. Please review the changes carefully."
fi
echo
done
Length of output: 1069
There was a problem hiding this comment.
@coderabbitai they do exist.
There was a problem hiding this comment.
@exequielrafaela Thank you for the clarification. It appears the new toolbox image tags do exist. Apologies for any confusion.
✏️ Learnings added
Learnt from: exequielrafaela
PR: binbashar/leverage#289
File: .github/workflows/tests-integration.yaml:28-29
Timestamp: 2024-10-17T16:31:27.344Z
Learning: The toolbox image tags `1.3.5-0.2.0`, `1.5.0-0.2.0`, and `1.6.0-0.2.0` exist in the registry.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Pull Request Test Coverage Report for Build 11612216358Details
💛 - Coveralls |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
- .github/workflows/tests-integration.yaml (1 hunks)
- .github/workflows/tests-unit.yml (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/tests-unit.yml
🧰 Additional context used
🔇 Additional comments (1)
.github/workflows/tests-integration.yaml (1)
29-29: LGTM on toolbox image tag update.The toolbox image tags have been updated to newer versions (1.3.5-0.2.0, 1.5.0-0.2.0, 1.6.0-0.2.0). This change is approved, and the existence of these tags has been confirmed in a previous review.
What?
Summary by CodeRabbit
pyproject.tomland workflow files.