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

Better handling of deadline and timeout #132

Merged
merged 4 commits into from
Sep 29, 2015
Merged

Conversation

psi29a
Copy link
Contributor

@psi29a psi29a commented Sep 28, 2015

No description provided.

all tests pass

adding more @Timeouts

added more timeouts and fixed tests
@@ -290,13 +293,13 @@ def test_LimitAtBatchThresholdEdge(self):
res = yield self.coll.find(limit=100)
self.assertEqual(len(res), 100)

yield self.coll.drop(safe=True)
yield self.coll.drop()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In case anyone is wondering, pymongo never supported safe in drop and drop_collection... neither did we, it was just sucked up into **kwargs and ignored later on.
http://api.mongodb.org/python/current/api/pymongo/collection.html#pymongo.collection.Collection.drop

@IlyaSkriblovsky
Copy link
Contributor

< grumbling >
Actually, I don't much like idea of adding some sort of hidden arguments to each query method.

This way it would be hard to write new method that would receive **kwargs and do something with it as with dict (like command() or distinct() create DB query from kwargs) without hidden knowledge that some sort of underscored arguments may appear in kwargs and needs to be removed aforehead.

Or if someone will introduce new query method and will fail to follow guideline of receiving **kwargs and mandatory passing it to all subcalls, the deadline feature will be broken. And this guideline isn't obvious from the code without studying internals of @timeout. This looks like a sign of fragile design that is easy to break.

You changes for supporting deadline is pretty broad already. Isn't it better to remove inlineCallbacks from query methods and get benefit from Twisted's builtin cancellation mechanism? This would be a massive change too, but it will allow us to keep deadline feature separate from internals of each query method.
</ grumbling >

sorry :)

@psi29a psi29a added this to the 15.3 - Full of Cancel milestone Sep 29, 2015
@psi29a psi29a self-assigned this Sep 29, 2015
psi29a added a commit that referenced this pull request Sep 29, 2015
Better handling of deadline and timeout
@psi29a psi29a merged commit 6970ffa into twisted:master Sep 29, 2015
@psi29a psi29a deleted the timeout_fixes branch September 29, 2015 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants