-
Notifications
You must be signed in to change notification settings - Fork 814
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
[tokumx] Handle int64 and replica connections #2390
Conversation
bd001a8
to
035285b
Compare
@olivielpeau could you take a quick pass? it's small |
I just added an additional commit to this PR which fixes an issue around reading replica sets. As of pymongo version 3.0, the Also, database connections are created in a pool with MongoClient, and are automatically closed up for you. So there should be no need to close the connection ourselves before opening this new one, as pymongo should reuse connections. |
socketTimeoutMS=DEFAULT_TIMEOUT*1000, | ||
read_preference=ReadPreference.SECONDARY, | ||
**ssl_params) | ||
db = conn[db.name] |
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 we don't handle the authentication on this connection. We should maybe extract the actual connection creation logic from the _get_connection
method and use it here.
Thanks for the fix! Added in 2 comments. |
@olivielpeau okay this should be good |
Looks good, feel free to squash and merge! |
As of pymongo version 3.0, the 'read_preference' attribute of 'MongoClient' is read-only. We set `read_preference` to secondary when reading replicas, causing an AttributeError. This commit creates a new database connection instead.
d4dc949
to
280b9d6
Compare
Ran into a situation where some data points coming from mongo are of type int64 (ie
0L
, or2L
). We should take these into account and then coerce them to floats.Additionally, the integration panel for TokuMX mentions specifying a full MongoDB URI, but the yaml shows no examples.