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

Handle Warnings #760

Merged
merged 12 commits into from
Mar 30, 2022
Merged

Handle Warnings #760

merged 12 commits into from
Mar 30, 2022

Conversation

blink1073
Copy link
Member

Reboot of #751 without any changes to the KernelManager.

@blink1073
Copy link
Member Author

Downstream test failure is due to jupyterlab/maintainer-tools#74

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.

This looks good Steve - really nice to have such clean CI results!

Looking at the original attempts in #751, would it be possible to sneak in this particular change? I've run into this during race-condition testing on shutdown and it makes sense to silence the exception on shutdown that otherwise should be raised.

@blink1073
Copy link
Member Author

would it be possible to sneak in #751 (comment)?

Good call, done!

@blink1073 blink1073 merged commit 683e8dd into jupyter:main Mar 30, 2022
@blink1073 blink1073 deleted the filter-warnings-2 branch March 30, 2022 15:33
@@ -11,14 +11,14 @@
def run_sync(coro):
def wrapped(*args, **kwargs):
try:
loop = asyncio.get_event_loop()
loop = asyncio.get_running_loop()
Copy link
Member

Choose a reason for hiding this comment

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

This and only this line seem to be breaking some projects.
in particular https://github.com/Carreau/inplace_restarter.

It seem to be because asyncio.get_event_loop() can return the current event loop even if it's not (yet) running.

I'm not sure what a proper fix coulc be.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the heads up, I put a workaround in 0a2c228 (#772)

Copy link
Member

Choose a reason for hiding this comment

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

Thanks , I also fixed my project upstream, I'll try to have a look at #772

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.

None yet

3 participants