-
Notifications
You must be signed in to change notification settings - Fork 0
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
Update/py3 13 deps #88
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request introduces a series of updates to the project's configuration and dependencies, primarily focusing on upgrading the Python version support from 3.8 to 3.13. Key changes include modifications to GitHub Actions workflows, the Makefile, and the setup.py file, which now reflect the new Python version requirements. The project's test dependencies have been updated to include newer versions of various packages, and the version number of the module has been incremented from '0.2.2' to '0.2.3.dev4'. The updates enhance compatibility with Python versions 3.9 through 3.13 and adjust the CI/CD pipeline to test across these versions. Additionally, a minor formatting change was made in an encryption-related file, and a new test case was added to enhance coverage for the 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Finishing Touches
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 (
|
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
🔭 Outside diff range comments (1)
.github/workflows/test.yml (1)
Line range hint
45-58
: Security: Update Codecov action and protect the token.Two important security considerations:
- The Codecov action is using an outdated version (v1.5.0)
- The Codecov token is hardcoded in the workflow file
Apply these changes to improve security:
- - uses: codecov/[email protected] + - uses: codecov/codecov-action@v4 with: - token: 1455e721-304d-43a9-b346-bcdacfac6631 + token: ${{ secrets.CODECOV_TOKEN }} file: ./coverage.xml flags: unittests name: codecov-umbrella fail_ci_if_error: trueMake sure to:
- Add the token to your repository secrets as
CODECOV_TOKEN
- Revoke and regenerate the current token as it has been exposed in the workflow file
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
Line range hint
1-58
: General workflow improvements needed.The workflow could benefit from the following improvements:
- Update the checkout action to the latest version:
- uses: actions/checkout@v2 + uses: actions/checkout@v4
- Consider making the SSL certificate setup more DRY by creating a reusable step:
# Create a composite action in .github/actions/setup-ssl/action.yml name: 'Setup SSL Certificate' runs: using: "composite" steps: - shell: bash run: export SSL_CERT_FILE=$(pwd)/tests/localhost.crtThen use it in both jobs:
- uses: ./.github/actions/setup-ssl
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/release.yml
(1 hunks).github/workflows/test.yml
(3 hunks)Makefile
(1 hunks)mongoengine_plus/types/encrypted_string/__init__.py
(1 hunks)mongoengine_plus/version.py
(1 hunks)requirements-test.txt
(1 hunks)setup.py
(1 hunks)tests/models/test_helpers.py
(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- mongoengine_plus/version.py
- mongoengine_plus/types/encrypted_string/init.py
- requirements-test.txt
🔇 Additional comments (8)
.github/workflows/test.yml (1)
23-23
: Well-structured Python version matrix!The matrix configuration perfectly aligns with the PR objectives, covering all intended Python versions (3.9-3.13) and properly dropping Python 3.8 support.
setup.py (3)
26-26
: Confirm intended minimum Python requirement
You've updated the minimum Python requirement to 3.9. This strictly excludes Python 3.8 users, which aligns with the PR's objective of dropping Python 3.8. Ensure that associated documentation and any distribution metadata clearly emphasize 3.9+ compatibility.
32-32
: Double-check pinned Boto3 version
You introduced an upper bound forboto3
(<1.35.89
). This approach can help ensure stability but may cause conflicts if other dependencies require a newerboto3
release.Would you like a quick script to scan the repo for references to
boto3
usage and confirm no version conflicts?
35-39
: Expanded classifiers for Python 3.9–3.13
Listing all supported versions in the classifiers is consistent with the newpython_requires
requirement and ensures PyPI accurately reflects compatibility.tests/models/test_helpers.py (1)
106-110
: Great addition to test coverage
This test verifyingmongo_to_dict(None)
is a valuable edge-case check. Well done..github/workflows/release.yml (1)
18-21
: Updated workflow to use Python 3.13
Switching from 3.8 to 3.13 fulfills the PR’s objective of testing with a modern Python version. Confirm that the repository’s release process remains stable on Python 3.13.
[approve]Makefile (2)
3-3
: Align Makefile Python version
ChangingPYTHON
from 3.8 to 3.13 ensures local development aligns with your updated environment. Keep in mind that many devs may use alternative Python versions.
6-6
: Update Black target version
Reflecting Black’s--target-version py313
is consistent with your new environment. This provides more accurate formatting expectations for Python 3.13 syntax and features.
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.
Solo el comentario de coderabbit para aprovechar el pr
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 (2)
setup.py (1)
35-39
: Consider adding development status classifierWhile Python version classifiers are added, consider also adding a development status classifier to indicate the project's maturity level.
Add one of these classifiers:
classifiers=[ + 'Development Status :: 5 :: Production/Stable', 'Programming Language :: Python :: 3.9',
.github/workflows/test.yml (1)
21-23
: Consider adding fail-fast strategy for matrix builds.To optimize CI time, consider adding fail-fast to the matrix strategy. This will cancel all in-progress jobs if any matrix job fails.
strategy: + fail-fast: true matrix: python-version: ['3.9', '3.10', '3.11', '3.12', '3.13']
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/release.yml
(1 hunks).github/workflows/test.yml
(4 hunks)mongoengine_plus/types/enum_field.py
(1 hunks)mongoengine_plus/version.py
(1 hunks)requirements-test.txt
(1 hunks)requirements.txt
(1 hunks)setup.py
(1 hunks)tests/types/test_encrypted_string.py
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- requirements.txt
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/release.yml
- mongoengine_plus/version.py
🔇 Additional comments (7)
mongoengine_plus/types/enum_field.py (1)
22-22
: Verify potential breaking change in enum handlingThe change from returning
enum
tostr(enum)
whenvalue
attribute is missing could be a breaking change for code relying on enum instances.Let's check for potential impacts:
✅ Verification successful
No breaking changes detected in enum handling
The change is safe as all identified enum usages in the codebase properly define enum values. Python's built-in Enum types (including IntEnum) always have the 'value' attribute, making the fallback to
str(enum)
an unused safety net.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for EnumField usage in the codebase rg -l "EnumField" | xargs rg -A 5 "EnumField\(" # Search for any custom Enum implementations that might be affected ast-grep --pattern 'class $NAME(Enum): $$$ 'Length of output: 1677
tests/types/test_encrypted_string.py (1)
114-114
: Great improvement in type checking!Using
isinstance()
instead oftype()
is more Pythonic and correctly handles inheritance.requirements-test.txt (1)
1-10
: Verify compatibility between updated packagesMajor version updates detected:
- flake8: 3.9.* → 7.1.1
- pytest: 6.2.* → 8.3.4
- pytest-cov: 2.12.* → 6.0.0
These significant version jumps might introduce breaking changes.
Let's check for potential conflicts:
setup.py (1)
26-32
: Verify Python 3.13 compatibility for all dependenciesThe package now supports Python 3.13, but we need to ensure all dependencies are compatible with this version.
Let's check compatibility:
.github/workflows/test.yml (3)
9-13
: LGTM! Action versions and Python version are up-to-date.The updates to the latest versions of actions/checkout@v4 and actions/setup-python@v5 are appropriate. Using Python 3.13 for linting aligns with the PR objectives.
Line range hint
41-56
: LGTM! Security and version updates are well implemented.The changes demonstrate good practices:
- Updated to latest action versions
- Consistent Python version with other jobs
- Properly using secrets for the Codecov token
23-27
: Verify Python version compatibility with dependencies.The matrix strategy correctly includes all supported Python versions (3.9-3.13) as per PR objectives. The action versions are up-to-date.
Let's verify the Python version compatibility with project dependencies:
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #88 +/- ##
===========================================
+ Coverage 99.17% 100.00% +0.82%
===========================================
Files 14 14
Lines 241 241
Branches 24 22 -2
===========================================
+ Hits 239 241 +2
+ Misses 1 0 -1
+ Partials 1 0 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
en general lo veo bien, solo tengo una duda con un test
|
||
def test_mongo_to_dict_with_none(): | ||
result = mongo_to_dict(None) | ||
assert result == {} |
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.
este test es de alguna parte del código que no tiene coverage?
Description
This PR updates the testing setup and ensures compatibility with supported Python versions.
Key Changes
Motivation
These changes align with the project's goal of maintaining consistency across supported versions and ensuring up-to-date dependency management. Dropping Python 3.8 reflects the end of its upstream support and reduces maintenance overhead.
Summary by CodeRabbit
Release Notes v0.2.3.dev4
New Features
ssn
field in testsDependencies
black
,flake8
,isort
,mypy
,pytest
, and othersmongoengine
,dnspython
,pymongo
, andpymongocrypt
boto3
version constraintsTesting
None
input in helper functionMaintenance