-
-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Make tests compatible with pymongo >= 4.14 #9968
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
Conversation
|
Ugh, I thought this would work with both old and new versions but CI clearly disagrees. Let me see if I can improve this ... |
a73ee00 to
e9c9cd2
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9968 +/- ##
=======================================
Coverage 78.68% 78.68%
=======================================
Files 153 153
Lines 19313 19313
Branches 2214 2214
=======================================
+ Hits 15196 15197 +1
Misses 3817 3817
+ Partials 300 299 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
e9c9cd2 to
c3336a4
Compare
mongodb/mongo-python-driver#2413 caused some test regressions here. This isn't currently a problem for the upstream test suite since it pins pymongo==4.10.1 via kombu, but we're running into it in Debian where we've already upgraded pymongo for other reasons. kombu already tried to upgrade pymongo but had to revert due to these test regressions (see celery/kombu#2384 and celery#9938). One of the test fixes (relating to `mongodb_backend_settings`) illustrates an incompatibility where I couldn't figure out a reasonable way to avoid passing it through to Celery users, so I added a note to the documentation about it. It may also be worth including a brief mention of it in the release notes. Using the canonical case for the option in question should work with both old and new versions of pymongo.
c3336a4 to
34ace3c
Compare
|
This isn't quite as clean as my original version, but I've checked that it works with both old and new versions of pymongo. |
|
we cam upgrade pymongo version in kombu as well |
|
Certainly, though I guess that would be after something like this lands in celery. |
|
yes |
auvipy
left a comment
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.
can you also incorporate changes from this pr #9665
I mean, I suppose, but why? That PR affects exactly one pymongo version, namely 4.12.0, as the pymongo bug that it's working around was fixed in 4.12.1; so it's completely disjoint with the versions that I'm dealing with in this PR. Why not just merge the two PRs separately? |
|
OK sure. |
Description
mongodb/mongo-python-driver#2413 caused some test regressions here. This isn't currently a problem for the upstream test suite since it pins pymongo==4.10.1 via kombu, but we're running into it in Debian where we've already upgraded pymongo for other reasons. kombu already tried to upgrade pymongo but had to revert due to these test regressions (see celery/kombu#2384 and #9938).
One of the test fixes (relating to
mongodb_backend_settings) illustrates an incompatibility where I couldn't figure out a reasonable way to avoid passing it through to Celery users, so I added a note to the documentation about it. It may also be worth including a brief mention of it in the release notes. Using the canonical case for the option in question should work with both old and new versions of pymongo.