Forward CodecOptions to BSON Encoder. #793
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #793 +/- ##
========================================
Coverage 95.29% 95.30%
========================================
Files 20 20
Lines 3849 3853 +4
========================================
+ Hits 3668 3672 +4
Misses 181 181 ☔ View full report in Codecov by Sentry. |
|
@pcorpet, does this seem to be an acceptable way of getting UUID support in pymongo4 ??? |
|
Is setting |
Here's the test I used Via kwargs to the collection. |
|
@pcorpet We are waiting for your opinion. Uuid representation is a great feature that will unblock many teams |
|
Thanks a lot for taking a look at this one. I think the solution is on the right track, and thanks for adding the value in the BSON check. I'll comment in the review for smaller stuffs. The major point missing to me is comparing UUIDs in the same server from two different client objects using different UUID representations. I think the easiest way is to prevent any legacy Java / legacy C encoding as the python & standard encodings are the same. By prevent I mean raise a NotImplementedError. |
| except ImportError: | ||
| json_utils = SON = BSON = None | ||
| CodecOptions = type(None) |
There was a problem hiding this comment.
If it's possible, I'd rather have it to be None, and that you check that it's not None before using it.
| @@ -74,6 +76,14 @@ class ReturnDocument(object): | |||
| } | |||
|
|
|||
|
|
|||
| def _bson_encode(document, codec_options): | |||
| if isinstance(codec_options, CodecOptions): | |||
| BSON.encode(document, check_keys=True, | |||
There was a problem hiding this comment.
Time to add ruff format lint stage to the CI? :)
| @@ -74,6 +76,14 @@ class ReturnDocument(object): | |||
| } | |||
|
|
|||
|
|
|||
| def _bson_encode(document, codec_options): | |||
| if isinstance(codec_options, CodecOptions): | |||
| BSON.encode(document, check_keys=True, | |||
There was a problem hiding this comment.
Can you add a test where this fails because of the encoding?
| with self.assertRaises(NotImplementedError): | ||
| self.database.with_options(custom_uuid_representation) | ||
| db = self.database | ||
| db.get_collection('yes_hello', |
There was a problem hiding this comment.
Please break after get_collection(
| db = self.database | ||
| db.get_collection('yes_hello', | ||
| codec_options=custom_uuid_representation).insert_one({'_id': uuid4()}) | ||
| # db.yes_hello.insert_one({'_id': uuid4()}) |
There was a problem hiding this comment.
Either drop this line or remove the #
|
@pcorpet |
|
Any updates on this? |
|
Any news on when this fix is coming? Thanks |
|
PyMongo 4 was released one year ago. |
Fixes #745.
This allows the use of UUID in pymongo4.
Passes unit tests.