Skip to content
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

Fixed mongo instance locking bug #836

Closed
wants to merge 5 commits into from
Closed

Fixed mongo instance locking bug #836

wants to merge 5 commits into from

Conversation

ISauve
Copy link
Contributor

@ISauve ISauve commented Oct 31, 2017

What does this PR do?

We currently use PyMongo's database_names() command to fetch the names of all the databases; this in turn uses MongoDB's listDatabases command. For Mongo versions >= 3.2, the command supports an additional field nameOnly which returns only the names of the databases and does not require database locks ( documentation ). PyMongo does not currently support this option, but the next version is supposed to, so this is just a temporary workaround until that version is released ( PyMongo changelog ).

This PR checks which version of Mongo the user is running and calls the listDatabases command directly, using the nameOnly field if the version is high enough.

Motivation

#606

Testing Guidelines

An overview on testing
is available in our contribution guidelines.

Versioning

  • Bumped the version check in manifest.json
  • Updated CHANGELOG.md. Please use Unreleased as the date in the title
    for the new section.

Additional Notes

Anything else we should know when reviewing?

@ISauve ISauve changed the title Fix mongo instance locking bug Fixed mongo instance locking bug Oct 31, 2017
Added use of 'nameOnly' tag when fetching database names for mongo versions >= 3.2
Copy link
Contributor

@gmmeyer gmmeyer left a comment

Choose a reason for hiding this comment

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

I added a single comment here, after you fix that, it's good to go I think!

Thanks!

@@ -862,7 +865,21 @@ def total_seconds(td):
except KeyError:
pass

dbnames = cli.database_names()
cmd = SON([("listDatabases", 1)])
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a comment with a link to the code in the mongo driver here and a comment with a reminder to fix this after the upgrade is done!

@ISauve
Copy link
Contributor Author

ISauve commented Nov 2, 2017

PyMongo 3.5 already contains a fix for this issue - no need to wait for the next version to come out. This problem is fixed by #747

@ISauve ISauve closed this Nov 2, 2017
@ISauve ISauve deleted the isabelle/mongo branch November 2, 2017 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants