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

tornado 6 compatibility #4449

Merged
merged 9 commits into from Mar 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 8 additions & 2 deletions .travis.yml
Expand Up @@ -50,7 +50,8 @@ before_install:
- pip install "attrs>=17.4.0"

install:
- pip install --pre .[test]
- pip install --pre .[test] $EXTRA_PIP
- pip freeze
- wget https://github.com/jgm/pandoc/releases/download/1.19.1/pandoc-1.19.1-1-amd64.deb && sudo dpkg -i pandoc-1.19.1-1-amd64.deb


Expand Down Expand Up @@ -97,10 +98,15 @@ matrix:
env: GROUP=python
- python: 3.5
env: GROUP=python
- python: "3.7-dev"
- python: 3.7
dist: xenial
env: GROUP=python
- python: 3.6
env: GROUP=docs
- python: 3.6
env:
- GROUP=python
- EXTRA_PIP="tornado<5"

after_success:
- codecov
9 changes: 9 additions & 0 deletions docs/source/changelog.rst
Expand Up @@ -21,6 +21,15 @@ We strongly recommend that you upgrade pip to version 9+ of pip before upgrading
Use ``pip install pip --upgrade`` to upgrade pip. Check pip version with
``pip --version``.

.. _release-5.7.5:

5.7.5
-----

- Fix compatibility with tornado 6 (:ghpull:`4392`, :ghpull:`4449`).
- Fix opening integer filedescriptor during startup on Python 2 (:ghpull:`4349`)
- Fix compatibility with asynchronous `KernelManager.restart_kernel` methods (:ghpull:`4412`)

.. _release-5.7.4:

5.7.4
Expand Down
4 changes: 2 additions & 2 deletions notebook/base/zmqhandlers.py
Expand Up @@ -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:
Copy link
Member

Choose a reason for hiding this comment

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

FWIW - I had found the similar change was necessary via ws_connection.

self.ping_callback.stop()
return

Expand Down Expand Up @@ -237,7 +237,7 @@ def _reserialize_reply(self, msg_or_list, channel=None):
def _on_zmq_reply(self, stream, msg_list):
# Sometimes this gets triggered when the on_close method is scheduled in the
# eventloop but hasn't been called.
if self.stream.closed() or stream.closed():
if self.ws_connection is None or stream.closed():
self.log.warning("zmq message arrived on closed channel")
self.close()
return
Expand Down
5 changes: 2 additions & 3 deletions notebook/gateway/managers.py
Expand Up @@ -8,7 +8,6 @@
from tornado import gen, web
from tornado.escape import json_encode, json_decode, url_escape
from tornado.httpclient import HTTPClient, AsyncHTTPClient, HTTPError
from tornado.simple_httpclient import HTTPTimeoutError

from ..services.kernels.kernelmanager import MappingKernelManager
from ..services.sessions.sessionmanager import SessionManager
Expand Down Expand Up @@ -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:
Copy link
Member

Choose a reason for hiding this comment

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

Thank you.

# This can occur if the host is valid (e.g., foo.com) but there's nothing there.
raise web.HTTPError(504, "Timeout error attempting to connect to Gateway server url '{}'. " \
raise web.HTTPError(504, "Error attempting to connect to Gateway server url '{}'. " \
"Ensure gateway url is valid and the Gateway instance is running.".format(
GatewayClient.instance().url))
except gaierror as e:
Expand Down
49 changes: 49 additions & 0 deletions notebook/utils.py
Expand Up @@ -12,13 +12,31 @@
import sys
from distutils.version import LooseVersion

try:
from inspect import isawaitable
except ImportError:
def isawaitable(f):
"""If isawaitable is undefined, nothing is awaitable"""
return False

try:
from concurrent.futures import Future as ConcurrentFuture
except ImportError:
class ConcurrentFuture:
"""If concurrent.futures isn't importable, nothing will be a c.f.Future"""
pass

try:
from urllib.parse import quote, unquote, urlparse, urljoin
from urllib.request import pathname2url
except ImportError:
from urllib import quote, unquote, pathname2url
from urlparse import urlparse, urljoin

# tornado.concurrent.Future is asyncio.Future
# in tornado >=5 with Python 3
from tornado.concurrent import Future as TornadoFuture
from tornado import gen
from ipython_genutils import py3compat

# UF_HIDDEN is a stat flag not defined in the stat module.
Expand Down Expand Up @@ -306,3 +324,34 @@ def _check_pid_posix(pid):
check_pid = _check_pid_win32
else:
check_pid = _check_pid_posix


def maybe_future(obj):
"""Like tornado's gen.maybe_future

but more compatible with asyncio for recent versions
of tornado
"""
if isinstance(obj, TornadoFuture):
return obj
elif isawaitable(obj):
return asyncio.ensure_future(obj)
elif isinstance(obj, ConcurrentFuture):
return asyncio.wrap_future(obj)
else:
# not awaitable, wrap scalar in future
f = TornadoFuture()
f.set_result(obj)
return f

# monkeypatch tornado gen.maybe_future
# on Python 3
# TODO: remove monkeypatch after backporting smaller fix to 5.x
try:
import asyncio
except ImportError:
pass
else:
import tornado.gen
tornado.gen.maybe_future = maybe_future

Copy link
Member

Choose a reason for hiding this comment

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

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.

2 changes: 1 addition & 1 deletion setup.py
Expand Up @@ -98,7 +98,7 @@
zip_safe = False,
install_requires = [
'jinja2',
'tornado>=4, <6',
'tornado>=4.1',
# pyzmq>=17 is not technically necessary,
# but hopefully avoids incompatibilities with Tornado 5. April 2018
'pyzmq>=17',
Expand Down