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

workaround tornado+py38+windows compatibility issue #456

Merged
merged 1 commit into from
Nov 24, 2019

Conversation

minrk
Copy link
Member

@minrk minrk commented Nov 12, 2019

set eventloop policy to the old default while tornado is not compatible with the new one

same as jupyter/notebook#5047

cross-ref: tornadoweb/tornado#2608

set eventloop policy to the old default while tornado is not compatible with the new one
Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

Thanks!

@bollwyvl
Copy link
Contributor

Since this patch would need to be in two places in notebook (in notebookapp.py, and in launchnotebook.py) and here in ipykernel, and very possibly in more places (anything with a threadpoolexecutor that also uses async... nbviewer, nbformat tests), should this not be hoisted up to jupyter_core?

It would likely also benefit us to get on github actions or azure pipelines so we are actually testing on windows... I wouldn't want to new start anything on appveyor at this point: the one-at-a-time free queue is just brutal, especially for windows excursions.

@minrk
Copy link
Member Author

minrk commented Nov 14, 2019

should this not be hoisted up to jupyter_core?

No, I don't think so. That would change the jupyter-core dependencies to include tornado, bump the minimum jupyter-core requirement in both packages, etc. It's a tiny patch and I think is better to duplicate than import from a common dependency, which separates the solution from where the problem is encountered.

@bollwyvl
Copy link
Contributor

bollwyvl commented Nov 14, 2019 via email

@minrk
Copy link
Member Author

minrk commented Nov 14, 2019

Quite true. Thanks for expanding the CI coverage!

@kevin-bates
Copy link

Thanks @minrk for the patch and @bollwyvl for the appveyor efforts - both have proven quite helpful to me. We're going to need the patch in jupyter_server and jupyter_kernel_mgmt and both projects will require an updated ipykernel for their tests as well.

Might there be an ETA on when an updated ipykernel can get released? I'm currently referencing Min's branch in jupyter_kernel_mgmt's appveyor.yml (PR 34), so even a merge into master would be helpful.

@jamesmyatt
Copy link

I assume this fix hasn't been released yet. Is there any timescale for this?

I'm hitting it now that notebook v6.0.3 has been released (jupyter/notebook#4613).

@kevin-bates
Copy link

Good point! The NB 6.0.3 drop is going to cause some major grief in new (and default) configurations on Windows/py38 systems. We probably should have ensured ipykernel was released prior to 6.0.3, but hindsight is 20/20 (hey, its the year of hindsight!).

@blink1073 - it looks like you've done the last few releases. Any chance you (or someone with applicable privs) could promote this closer to the top of your "todos"? If necessary, I could try to put together the change log entries if that helps. I'm assuming this release would be 5.1.4 rather than 5.2.0.

ericpre added a commit to ericpre/ipykernel that referenced this pull request Jan 26, 2020
). Set eventloop policy to the old default while tornado is not compatible with the new one.
@minrk minrk deleted the py38-win branch January 27, 2020 10:39
@minrk
Copy link
Member Author

minrk commented Jan 27, 2020

Apologies for the delay, 5.1.4 is out now with the fix.

ericpre added a commit to ericpre/ipykernel that referenced this pull request Jan 29, 2020
…python#456). Set eventloop policy to the old default while tornado is not compatible with the new one."

This reverts commit 06967ee.
@jamesmyatt
Copy link

I can confirm that the conda-forge release of 5.1.4 addresses my issues. Thanks

clrpackages pushed a commit to clearlinux-pkgs/ipykernel that referenced this pull request Mar 23, 2020
Benjamin Ragan-Kelley (1):
      back to dev

Dan Davison (1):
      Use shell.input_transformer_manager when available

David Brochart (2):
      Add control channel
      Update for v5.2

Eric Prestat (4):
      Workaround tornado+py38+windows compatibility issue (Same as ipython/ipykernel#456). Set eventloop policy to the old default while tornado is not compatible with the new one.
      Revert "Workaround tornado+py38+windows compatibility issue (Same as ipython/ipykernel#456). Set eventloop policy to the old default while tornado is not compatible with the new one."
      Fix example to work on windows with python3.8
      Update deprecated import in some examples.

L. Kärkkäinen (1):
      Suppress internal traceback when interrupting program and provide useful diagnostic message.

Matthias Bussonnier (2):
      try to fix tests on 3.8
      Don't indicate support for 3.4.

Nicholas Bollweg (3):
      add py38 and py35 to the appveyor matrix
      exclude pytest 5.3.4
      set pytest to fail fast in appveyor

Quentin Peter (2):
      Fix None eventloop
      Add blank line around nested function

Steven Silvester (1):
      Release 5.2.0

ossdev07 (1):
      Added arm64 jobs for Travis-CI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants