-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Convert the typing handler to async/await. #7679
Conversation
ada08e3
to
600d412
Compare
@@ -163,7 +164,7 @@ def test_started_typing_local(self): | |||
|
|||
self.assertEquals(self.event_source.get_current_key(), 0) | |||
|
|||
self.successResultOf( | |||
self.get_success( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these aren't quite the same thing. successResultOf
asserts that the thing has already completed; get_success
does not do that. I think tests.test_utils.get_awaitable_result
should do what you want here.
That said, it probably doesn't matter here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_success
seems to just handle if it is an awaitable
, wrap it in a ensureDeferred
and then call successResultOf
:
Lines 443 to 449 in 0361932
def get_success(self, d, by=0.0): | |
if inspect.isawaitable(d): | |
d = ensureDeferred(d) | |
if not isinstance(d, Deferred): | |
return d | |
self.pump(by=by) | |
return self.successResultOf(d) |
So I think it does the same thing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The difference is the self.pump
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
* commit '363082561': Convert the typing handler to async/await. (#7679)
There's a couple of odd methods in here, in particular:
_stopped_typing
was yielded, but wasn't aDeferred
.get_new_events
doesn't need to be a coroutine, but I think it expects to be one based on the callers.get_success
need to be used in tests instead ofgetSuccessOf
for old deps, I think.