-
Notifications
You must be signed in to change notification settings - Fork 1.1k
PYTHON-4933 - Allow drivers to set bypassDocumentValidation: false on… #2227
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
pymongo/asynchronous/bulk.py
Outdated
@@ -87,7 +87,7 @@ def __init__( | |||
self, | |||
collection: AsyncCollection[_DocumentType], | |||
ordered: bool, | |||
bypass_document_validation: bool, | |||
bypass_document_validation: Optional[bool] = None, |
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.
This should be bypass_document_validation: Optional[bool],
the arg should still be required.
pymongo/asynchronous/collection.py
Outdated
session: Optional[AsyncClientSession], | ||
bypass_doc_val: Optional[bool] = None, |
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.
This should be bypass_doc_val: Optional[bool],
. It should still be required positional arg in all our internal APIs since the default is never used anyway.
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.
Our current internal APIs expect it to be a keyword argument and not positional. Is changing it to a positional argument intentional here?
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.
Previously it was positional:
write_concern: WriteConcern,
op_id: Optional[int],
bypass_doc_val: bool,
session: Optional[AsyncClientSession],
There's no need to change it to have a default of None because we always pass a value anyway so the default will never be used.
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.
Looks like_insert_one
is positional, _update
and _update_retryable
are keyword arguments. We should unify those so they are consistent. I'd prefer them to all be keyword arguments for clarity.
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.
Let's just keep this one as positional and leave the rest.
pymongo/asynchronous/collection.py
Outdated
let=let, | ||
hint=hint, | ||
session=session, | ||
**kwargs, |
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.
Was this change intentional? It seems unneeded.
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.
Not intended, good catch.
… write commands