-
Notifications
You must be signed in to change notification settings - Fork 286
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
Use tornado 6.2's PeriodicCallback in restarter #822
Conversation
Since it will now await any callbacks that are awaitable, we no longer need to wrap it in a `run_sync` call.
for more information, see https://pre-commit.ci
Note that this by itself will likely not fix the issue in jupyterlab/jupyterlab#11934 for all users. Anyone who uses any of our sync classes might still be experiencing issues. Or it could be that the issue only appears for To clarify that issue: If we call |
|
Let's see if we can get all green without xeus-cling. |
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, thanks for working on this!
Is it okay to release this as a patch release when it bumps the tornado dependency version? |
Yeah, let's try that, releasing now. |
Since tornado 6.2, the
PeriodicCallback
will now await any callbacks that are awaitable (tornadoweb/tornado#2924). So we no longer need to wrap the async restarter'spoll
method in arun_sync
call. This allows us to not depend onnest_asyncio
with the default configuration of jupyter client, so we can avoid some of its pitfalls (e.g. jupyterlab/jupyterlab#11934).