Allow MongoDB to be instantiated without mongomock library #11023
Conversation
|
Jenkins results:
|
There was a problem hiding this comment.
Thanks @amaltaro for your quick fix. I have left one comment inline, which I think is worth taking a look.
| import mongomock | ||
| except ImportError: | ||
| # this library should only be required by unit tests | ||
| mongomock = None |
There was a problem hiding this comment.
That is a good workaround, but if we go that way we should then walk to the end.
We do know that it is supposed to be used only in the unit tests. But, since the class itself provides the option for an object to be created with mockMongoDB=True, then if someone, who is missing the mongomock library, does try to benefit from the flag, this would generate the following error on line 53 :
In [1]: mongomock = None
In [2]: client = mongomock.MongoClient()
---------------------------------------------------------------------------
AttributeError Traceback (most recent call last)
<ipython-input-2-233abe3373bf> in <module>
----> 1 client = mongomock.MongoClient()
AttributeError: 'NoneType' object has no attribute 'MongoClient'
In this case I would suggest to add some extra protection on line in question, like:
In [7]: if mockMongoDB and mongomock is not None:
...: self.client = mongomock.MongoClient()
There was a problem hiding this comment.
I actually wanted it to crash indeed, since it would be a setup that people should never even try. Nothing as a big error in our face :)
But I think I can do something slightly better here, still along the same line of thought. New change coming up in a few minutes.
|
Jenkins results:
|
| if mockMongoDB and mongomock is None: | ||
| msg = "You are trying to mock MongoDB, but you do not have mongomock in the python path." | ||
| self.logger.critical(msg) | ||
| raise ImportError(msg) |
There was a problem hiding this comment.
ok, this is sound.
Agreed!
raise ImportError if requested to mock mongo without mongomock in the path
|
Thanks Todor. Code rebased and will get merged soon. |
|
Jenkins results:
|
Fixes #11022
Status
ready
Description
Don't crash the service that is importing MongoDB if it does not have the
mongomocklibrary in the path, which is actually only needed for unit tests.Is it backward compatible (if not, which system it affects?)
YES
Related PRs
Complement for: #11007
External dependencies / deployment changes
None