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

Upgrade to support jupyter_client 8 #1778

Merged
merged 1 commit into from
Jun 15, 2023
Merged

Conversation

shreve
Copy link
Contributor

@shreve shreve commented Apr 13, 2023

When upgrading the dependency spec to include v8, some python tests broke (see #1728). The tests were failing with a timeout due to indefinitely blocked IO.

This originates in an infinitely-looping notebook and is caused by a misconfiguration of the client connection. Currently using nbclient.NotebookClient with a jupyter_client.manager.KernelManager results in a deadlock. nbclient doesn't seem aware of this issue because there is no test covering it. NotebookClient uses AsyncKernelManager by default.

So why isn't nbgrader using it? nbgrader uses the NotebookClient via nbconvert which just realized the same issue. Up until v7.3.1, nbconvert has been hard-coding the default kernel manager to the sync version. (see jupyter/nbconvert#1964)

Rather than require nbconvert>=7.3.1, this commit sets our own default value to the async kernel manager.

When upgrading the dependency spec to include v8, some python tests
broke (see jupyter#1728). The tests
were failing with a timeout due to indefinitely blocked IO.

This originates in an infinitely-looping notebook and is caused by a
misconfiguration of the client connection. Currently using
`nbclient.NotebookClient` with a `jupyter_client.manager.KernelManager`
results in a deadlock. nbclient doesn't seem aware of this issue because
there is no test covering it. `NotebookClient` uses `AsyncKernelManager`
by default.

So why isn't nbgrader using it? nbgrader uses the NotebookClient via
nbconvert which just realized the same issue. Up until v7.3.1, nbconvert
has been hard-coding the default kernel manager to the sync version.
(see jupyter/nbconvert#1964)

Rather than require nbconvert>=7.3.1, this commit sets our own default
value to the async kernel manager.
@github-actions
Copy link
Contributor

Binder 👈 Launch a Binder on branch shreve/nbgrader/jupyter_client

@shreve
Copy link
Contributor Author

shreve commented Apr 13, 2023

I'm not familiar but it looks like the codecov dependency got pulled from pypi.

https://github.com/codecov/codecov-python

@tuncbkose
Copy link
Contributor

tuncbkose commented Apr 13, 2023

opened #1780 for codecov this was a bit careless so I closed it, but it should be fine to rerun the tests now

@brichet
Copy link
Contributor

brichet commented Jun 13, 2023

Kicking the CI

@brichet brichet closed this Jun 13, 2023
@brichet brichet reopened this Jun 13, 2023
@brichet
Copy link
Contributor

brichet commented Jun 15, 2023

@shreve can you rebase to check the tests ?

Copy link
Contributor

@brichet brichet left a comment

Choose a reason for hiding this comment

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

Thanks @shreve

@brichet brichet merged commit 6b0ec9f into jupyter:main Jun 15, 2023
@shreve shreve deleted the jupyter_client branch June 15, 2023 19:26
AlirezaT99 pushed a commit to AaltoSciComp/nbgrader that referenced this pull request Aug 6, 2024
When upgrading the dependency spec to include v8, some python tests
broke (see jupyter#1728). The tests
were failing with a timeout due to indefinitely blocked IO.

This originates in an infinitely-looping notebook and is caused by a
misconfiguration of the client connection. Currently using
`nbclient.NotebookClient` with a `jupyter_client.manager.KernelManager`
results in a deadlock. nbclient doesn't seem aware of this issue because
there is no test covering it. `NotebookClient` uses `AsyncKernelManager`
by default.

So why isn't nbgrader using it? nbgrader uses the NotebookClient via
nbconvert which just realized the same issue. Up until v7.3.1, nbconvert
has been hard-coding the default kernel manager to the sync version.
(see jupyter/nbconvert#1964)

Rather than require nbconvert>=7.3.1, this commit sets our own default
value to the async kernel manager.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants