Skip to content

PYTHON-2859 Add BSON Binary subtype 7 #763

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

Merged
merged 3 commits into from
Oct 26, 2021

Conversation

juliusgeo
Copy link
Contributor

Turns out the spec changes have already been added into pymongo. So all that is left is to add the constant.

bson/binary.py Outdated
@@ -23,6 +23,8 @@

This is the default subtype for binary data.
"""
COLUMN_SUBTYPE = 0x07
"""BSON binary subtype for columns"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments:

  • The constants in this file are defined in increasing order. Please move this down below MD5_SUBTYPE.
  • please use 7 instead of 0x07 since none of the other constants use hex.
  • For the docstring, it should end in a period. Please also add a versionadded.
  • Please check doc/api/bson/binary.rst to see if we need to update the docs as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@juliusgeo juliusgeo requested a review from ShaneHarvey October 26, 2021 01:38
@@ -169,6 +169,11 @@ class UuidRepresentation:
"""BSON binary subtype for an MD5 hash.
"""

COLUMN_SUBTYPE = 7
"""BSON binary subtype for columns.
.. versionadded:: 4.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check that the docs render correctly: https://pymongo--763.org.readthedocs.build/en/763/api/bson/binary.html

In this case there is a missing new line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Member

@ShaneHarvey ShaneHarvey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@juliusgeo juliusgeo merged commit eabd223 into mongodb:master Oct 26, 2021
juliusgeo added a commit to juliusgeo/mongo-python-driver that referenced this pull request Apr 5, 2022
juliusgeo added a commit to juliusgeo/mongo-python-driver that referenced this pull request Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants