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

Subprocess kill is not async #535

Merged
merged 1 commit into from
Mar 25, 2020
Merged

Conversation

davidbrochart
Copy link
Member

Copy link
Member

@kevin-bates kevin-bates left a comment

Choose a reason for hiding this comment

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

Thanks David. This is also consistent with what we did in JKM.

@MSeal MSeal merged commit 3ab5e5a into jupyter:master Mar 25, 2020
@MSeal
Copy link
Contributor

MSeal commented Mar 25, 2020

Do we need a patch release to get the change available for those other PRs?

@davidbrochart davidbrochart deleted the fix_kill branch March 25, 2020 17:04
@kevin-bates
Copy link
Member

I think this would be good as the jupyter_server PR produces intermittent failures during CI. I don't know about the nbclient behavior.

@MSeal
Copy link
Contributor

MSeal commented Mar 25, 2020

This could explain some hanging tests we see in python 3.5 for nbclient, I'll kick off a release now

@MSeal
Copy link
Contributor

MSeal commented Mar 25, 2020

6.1.1 is released

@kevin-bates
Copy link
Member

Thank you. I started drilling into the odd CI failure we've been seeing in the Notebook repo since about a month ago, and believe it is associated with JC 6.0. The tests are failing in kernel.js for its auto-restart test.

This theory is based on taking two of the failing PRs - of completely unrelated areas - and pinning their JC versions to 5.3.4 and both succeed. Because 6.0 consists of numerous fixes from the master branch - some 2 years old - this might be difficult, but I'm going to focus on some of the PRs I added since they could be related to restarts in some manner (as most of my PRs are in the areas of kernel management).

At any rate, we might need a 6.1.2 sometime in the near future.

@MSeal
Copy link
Contributor

MSeal commented Mar 25, 2020

Sounds great thanks for digging into it. I was going to start candidate testing 6.0 here but the virus has prompted delaying new changes for a couple weeks on our stacks.

@kevin-bates
Copy link
Member

PR #536 has been submitted to address the NB CI failures.

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.

3 participants