-
Notifications
You must be signed in to change notification settings - Fork 557
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
bump mongoengine, motor, and pymongo #4915
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughThe changes in this pull request involve updates to the version specifications of several dependencies in both Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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
Documentation and Community
|
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
🧹 Outside diff range and nitpick comments (1)
setup.py (1)
49-50
: Summary of dependency updatesThe updates to mongoengine, motor, and pymongo align with the PR objectives and should enable support for MongoDB version 8. However, these are significant version updates that require careful consideration:
- Verify compatibility: Ensure that the project is compatible with the new versions of all three dependencies. Review changelogs for breaking changes and update the codebase accordingly.
- Update documentation: Reflect any changes in usage or MongoDB version support in the project documentation.
- Consider upper bounds: For pymongo and potentially other dependencies, consider adding upper bounds to prevent automatic updates to potentially incompatible future versions.
- Comprehensive testing: Conduct thorough testing, especially for the scenarios mentioned in the PR description (connecting to existing and empty databases).
To ensure a smooth transition and maintain long-term stability:
- Implement a comprehensive test suite that covers all interactions with these dependencies.
- Set up continuous integration to regularly test against the latest compatible versions of these dependencies.
- Document the rationale behind these version changes and any known compatibility issues for future reference.
Also applies to: 58-58
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- requirements/common.txt (1 hunks)
- setup.py (1 hunks)
🧰 Additional context used
🔇 Additional comments (7)
requirements/common.txt (4)
12-19
: Summary: Dependency updates for mongoengine, motor, and pymongoThe updates to mongoengine, motor, and pymongo align with the PR objectives and follow good practices for version specification. The use of the tilde (~) notation allows for compatible minor version updates while maintaining stability.
Given the significance of these updates, especially the major version changes, it's crucial to conduct comprehensive testing:
- Verify that all features depending on these libraries work as expected.
- Pay special attention to database operations, ensuring compatibility with both existing and empty databases as mentioned in the PR objectives.
- Check for any deprecated features or syntax changes that might affect your codebase.
Consider running your full test suite and performing manual testing of critical database operations to ensure everything functions correctly with these updated dependencies.
13-13
: Approved: motor version updateThe update from >=2.5 to ~3.6.0 is significant and aligns with the PR objectives. The change in version specification style provides better control over updates.
As this is a major version update (2.x to 3.x), please verify that it doesn't introduce any breaking changes in your codebase. Run the following command to check for motor usage:
#!/bin/bash # Search for motor usage in the codebase rg --type python 'from motor import|import motor' -A 10
19-19
: Approved: pymongo version updateThe update from >=3.12,<4.9 to ~4.9.2 is significant and aligns with the PR objectives. The change in version specification style provides better control over updates and supports MongoDB version 8.
As this is a major version update (3.x to 4.x), please verify that it doesn't introduce any breaking changes in your codebase. Run the following command to check for pymongo usage:
#!/bin/bash # Search for pymongo usage in the codebase rg --type python 'from pymongo import|import pymongo' -A 10
12-12
: Approved: mongoengine version updateThe update from 0.24.2 to
0.29.1 is significant and aligns with the PR objectives. The use of the tilde () notation allows for compatible minor version updates, which is a good practice.Please ensure that this major version update (0.24 to 0.29) doesn't introduce any breaking changes in your codebase. Run the following command to check for any deprecated or removed features:
setup.py (3)
50-50
: Approved: motor version updateThe update from
motor>=2.5
tomotor~=3.6.0
aligns with the PR objective to update this dependency. This change narrows down the acceptable versions to the 3.6.x range, providing more control over the package version.As this is a major version update from 2.x to 3.x, please ensure that the project is compatible with motor 3.6.x. Review the changelog for any breaking changes and update the project documentation if necessary to reflect any changes in usage.
58-58
: Approved with suggestions: pymongo version updateThe update from
pymongo>=3.12,<4.9
topymongo~=4.9.2
aligns with the PR objective to update this dependency and enable support for MongoDB version 8. This change narrows down the acceptable versions to the 4.9.x range.Please consider the following points:
- Ensure that the project is compatible with pymongo 4.9.x, as this is a major version update from 3.x to 4.x. Review the changelog for any breaking changes.
- The removal of the upper bound (
<4.9
) could potentially lead to compatibility issues with future versions. Consider adding an upper bound to prevent automatic updates to potentially incompatible versions, e.g.,pymongo~=4.9.2,<5.0
.- Update the project documentation if necessary to reflect any changes in usage or MongoDB version support.
49-49
: Approved: mongoengine version updateThe update from
mongoengine==0.24.2
tomongoengine~=0.29.1
aligns with the PR objective to update this dependency. This change allows for more flexible updates within the 0.29.x range.Given the significant version jump, please ensure that the project is compatible with mongoengine 0.29.x. You may want to review the changelog for any breaking changes between 0.24.2 and 0.29.1.
The use of
and
are equivalent Let's hope that our upstream providers are better at semver than we are... |
Necessary but insufficient information: |
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.
@benjaminpkane did you thoroughly test this? You said you would not have time this week. |
I have not. To err on the side of caution, I propose we wait until after dev cut to merge |
definitely agree |
42b8fbc
to
5ea2521
Compare
What changes are proposed in this pull request?
Pin to newer versions of mongoengine, motor, and pymongo - the use of new versions needs to be consistent as variations result in pickle issues, so pymongo has to fit with motor has to fit with mongoengine
This also enables MongoDB 8 support.
How is this patch tested? If it is not, please explain why.
Release Notes
Is this a user-facing change that should be mentioned in the release notes?
notes for FiftyOne users.
(Details in 1-2 sentences. You can just refer to another PR with a description
if this PR is part of a larger change.)
What areas of FiftyOne does this PR affect?
fiftyone
Python library changesSummary by CodeRabbit