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

Non Local Mongo #150

Merged
merged 5 commits into from Jan 30, 2020
Merged

Non Local Mongo #150

merged 5 commits into from Jan 30, 2020

Conversation

shyamd
Copy link
Contributor

@shyamd shyamd commented Jan 30, 2020

This patch enables using a non-local mongo server by supplying a URI string in the config

CasperWA
CasperWA previously approved these changes Jan 30, 2020
Copy link
Member

@CasperWA CasperWA left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks @shyamd! :)

else:
from mongomock import MongoClient

client = MongoClient()
client = MongoClient()
Copy link
Member

Choose a reason for hiding this comment

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

Is this not a possibility in mongomock? Or maybe it just doesn't make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It actually works fine as far as code goes, but it's probably not guaranteed behavior, so it doesn't hurt to separate the two unless we're getting issues elsewhere like linting.

Copy link
Member

Choose a reason for hiding this comment

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

It was merely for the elegance of the code as it was - only having one line for the instantiation of the client - but never mind.

@CasperWA
Copy link
Member

Ah, I see there's a test failing for blackening. Could you possibly install pre-commit (run pre-commit install in the root of the git repo folder)? And then run it for all files and commit (pre-commit run --all).

Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

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

This LGTM now the checks are passing, thanks @shyamd.

@CasperWA CasperWA merged commit eee3651 into Materials-Consortia:master Jan 30, 2020
@CasperWA CasperWA mentioned this pull request Feb 4, 2020
@shyamd shyamd deleted the non_local_mongo branch April 8, 2020 15:00
@CasperWA CasperWA mentioned this pull request Apr 22, 2020
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.

None yet

3 participants