-
Notifications
You must be signed in to change notification settings - Fork 1.1k
PYTHON-5310 - Fix uri_parser AttributeError when used directly #2283
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
def test_pymongo_submodule_attributes(self): | ||
import pymongo | ||
|
||
self.assertTrue(hasattr(pymongo, "uri_parser")) |
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.
Does this test fail before this fix? Also can we add:
self.assertTrue(pymongo.uri_parser)
self.assertTrue(pymongo.uri_parser.parse_uri))
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.
Yes, this assertion fails before this fix.
pymongo/__init__.py
Outdated
@@ -45,6 +45,7 @@ | |||
"WriteConcern", | |||
"has_c", | |||
"timeout", | |||
"uri_parser", |
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.
Is this the right fix? Back in 4.7 uri_parser wasn't part of __all__
and this bug didn't exist. Perhaps we shouldn't include uri_parser here and instead we should import pymongo.uri_parser
like we did before.
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.
I'm worried about unintentional side effects of including it in __all__
.
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.
Good catch, we don't actually need to include uri_parser
in __all__
.
No description provided.