tornado 6 compatibility #4449
tornado 6 compatibility #4449
Conversation
to get testing 6.0 again
for debugging test env
stream is None in tornado 6
tornado gen.maybe_future is deprecated in >= 5.0 and doesn't accept asyncio coroutine objects or awaitables in general causing failures with tornado 6 on asyncio monkeypatch gen.maybe_future for easier backport to 5.x later, we can update to use our maybe_future throughout
instead of out-of-date dev version
HTTPTimeoutError is new in tornado 5.1
|
Can release 5.7.5 from 5.7.x after landing this |
|
OK, I think this is ready to go and we can release 5.7.5 once it's backported. |
|
I think it would be appropriate to pin |
| @@ -172,7 +172,7 @@ def open(self, *args, **kwargs): | |||
|
|
|||
| def send_ping(self): | |||
| """send a ping to keep the websocket alive""" | |||
| if self.stream.closed() and self.ping_callback is not None: | |||
| if self.ws_connection is None and self.ping_callback is not None: | |||
There was a problem hiding this comment.
FWIW - I had found the similar change was necessary via ws_connection.
| @@ -259,9 +258,9 @@ def gateway_request(endpoint, **kwargs): | |||
| except ConnectionRefusedError: | |||
| raise web.HTTPError(503, "Connection refused from Gateway server url '{}'. " | |||
| "Check to be sure the Gateway instance is running.".format(GatewayClient.instance().url)) | |||
| except HTTPTimeoutError: | |||
| except HTTPError: | |||
| else: | ||
| import tornado.gen | ||
| tornado.gen.maybe_future = maybe_future | ||
|
|
There was a problem hiding this comment.
Very nice. I had also found that gen.maybe_future wasn't happy with tornado 6, this is a much more elegant solution for B/C purposes.
|
Thank you Min! FWIW - I pull your branch and took things for a quick spin against tornado 6.0.1. Looks good! 👍 |
|
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon! If these instruction are inaccurate, feel free to suggest an improvement. |
tornado 6 compatibility
Broken package problem mentioned at e6a3066 now resolved, and we can safely unpin tornado from the Pipfile. Actual resolve-ment may have had to do with notebook coincidentally being updated from 5.7.4 to 5.7.6 at 7e91dbc. See jupyter/notebook#4449 and jupyter/notebook#4439. To be honest, I can't quite remember if it's actually notebook that broke in the first place, might have been some DeprecationWarning in some other package being printed that broke one of the tests in test_ipynb.ipynb...
to get testing 6.0 again
We usually try to test prereleases to catch this kind of issue, but stopped testing tornado 6 when we found out about a compatibility problem. Whenever we merge a PR to pin dependencies as a band-aid to get tests passing (AOK to get things unblocked), let's try to open a WIP PR unpinning the dependency immediately so we can fix the issue, because it's always a real thing we need to fix, ideally with a release before the upstream package gets its final release.
once this works:
closes #4437
closes #4439