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

do not apply asyncio patch for tornado >=6.1 #5907

Merged
merged 1 commit into from
Dec 14, 2020

Conversation

bollwyvl
Copy link
Contributor

@bollwyvl bollwyvl commented Dec 11, 2020

Tornado 6.1 now no longer requires the asyncio patch on windows for python >=3.8. 🎉

On jupyter-server/jupyter_server#339, we went with a hard cut to require 6.1, and remove the patch, but this PR applies the lighter touch from ipython/ipykernel#564 of just not applying the patch.

Fixing this is good, as the error manifests itself as unkillable python hang on windows, timing out CI.

@bollwyvl
Copy link
Contributor Author

Ha, thanks @minrk! How you doing?

I made some progress on #5909 so this could be exercised a little harder (especially getting the selenium tests to run on windows under a representative deployment) but looks like nobody's ever done it before, so it's slow going. Will be doing some more looking!

@bollwyvl
Copy link
Contributor Author

Also, incidentally: can you think of any other places we had to do this stuff? I think i hit most of them...

@minrk
Copy link
Member

minrk commented Dec 15, 2020

can you think of any other places we had to do this stuff?

I did a search and it looks like:

  • ipykernel
  • notebook
  • qtconsole
  • jupyterhub
  • nbconvert
  • nbdime

@minrk
Copy link
Member

minrk commented Dec 15, 2020

How you doing?

I'm okay! Super busy and spread a little thin, but trying to keep up. How are you? :)

@bollwyvl
Copy link
Contributor Author

Same... it'll be great when all the stuff that's in the air lands, but it does seem like a lot right now!

@minrk
Copy link
Member

minrk commented May 9, 2021

Looking back at tornado docs, I think we should still be using the selector loop all the time. Performance will be better, as ~everything we do is built add/remove_reader, which are slower when proactor is used do to the extra thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants