Add ability to configure explain command options #49
Conversation
test/test_collection.py
Outdated
| self.collection.watch(pipeline=[{"$project": {"tags": 1}}], | ||
| batch_size=10, full_document="updateLookup") | ||
| last_logger_payload = self.logger.cmd_payload | ||
| res = self.explain.watch(pipeline=[{"$project": {"tags": 1}}], | ||
| batch_size=10, full_document="updateLookup") | ||
| self.assertIn("queryPlanner", res["stages"][0]["$cursor"]) | ||
| self.assertTrue(self._recursiveIn("queryPlanner", res)) |
There was a problem hiding this comment.
I actually prefer the previous behavior here because res["stages"][0]["$cursor"] helps us understand the structure of the explain response. _recursiveIn makes the command response more of a black box. Can you revert these changes?
| self.assertTrue(self._recursiveIn("executionStats", res)) | ||
| self.assertTrue(self._recursiveIn("allPlansExecution", res)) | ||
|
|
||
| def test_comment(self): |
There was a problem hiding this comment.
Instead of checking the explain result, this test should probably assert that we sent the comment field with the explain comment using a CommandListener.
|
|
||
| def _explain_command(self, command): | ||
| command_son = command.get_SON() | ||
| explain_command = SON([("explain", command_son)]) | ||
| explain_command["verbosity"] = "queryPlanner" | ||
| explain_command["verbosity"] = self.verbosity or "queryPlanner" |
There was a problem hiding this comment.
I suggest setting the default verbosity in __init__.
| self.last_cmd_payload = command_son | ||
| return self.collection.database.command(explain_command) | ||
|
|
||
| def update_settings(self, verbosity=None, comment=None): |
There was a problem hiding this comment.
In PyMongo the Collection object is immutable (the internal settings are configured once at creation and never changed). ExplainableCollection should probably be immutable too to match pymongo.
test/test_collection.py
Outdated
| @@ -114,14 +114,15 @@ def test_delete_many(self): | |||
| last_cmd_payload = self.explain.last_cmd_payload | |||
| self._compare_command_dicts(last_cmd_payload, last_logger_payload) | |||
|
|
|||
| @unittest.skip("Travis does not have replica sets set up yet") | |||
There was a problem hiding this comment.
This is no longer true.
|
|
||
| explain = ExplainableCollection(collection, verbosity="queryPlanner", | ||
| comment="I'm a comment") | ||
| For more information see the documentation for the explain_ command. |
There was a problem hiding this comment.
I believe there needs to be a blank line after the code block.
test/test_collection.py
Outdated
| @@ -118,10 +119,10 @@ def test_watch(self): | |||
| res = self.explain.watch() | |||
| self.assertIn("queryPlanner", res["stages"][0]["$cursor"]) | |||
| self.collection.watch(pipeline=[{"$project": {"tags": 1}}], | |||
| batch_size=10, full_document="updateLookup") | |||
| batch_size=10, full_document="updateLookup") | |||
There was a problem hiding this comment.
This change seems off batch_size should align with pipeline.
test/test_collection.py
Outdated
| last_logger_payload = self.logger.cmd_payload | ||
| res = self.explain.watch(pipeline=[{"$project": {"tags": 1}}], | ||
| batch_size=10, full_document="updateLookup") | ||
| batch_size=10, full_document="updateLookup") |
test/test_collection.py
Outdated
| self.assertNotIn("comment", self.logger.cmd_payload) | ||
| self.explain = ExplainCollection(self.collection, comment="hey, " | ||
| "I'm a " | ||
| "comment") |
There was a problem hiding this comment.
Please fix the string format here. 3 short strings on new lines makes this harder to read. How about:
self.explain = ExplainCollection(self.collection, comment="comment")
No description provided.