Make nest_asyncio optional

[davidbrochart]

In nbclient, we chose to implement blocking methods such as execute by running their async counterparts (e.g. async_execute) in an event loop (with asyncio.run_until_complete), to avoid code duplication. This can be an issue in environments where an event loop is already running, such as a Jupyter Notebook. nest_asyncio solves this issue, but it doesn't play well with tornado (see tornadoweb/tornado#2753). This will impact applications such as Voila (which will use the async methods of nbclient and so won't need to nest event loops). We should for now make the use of nest_asyncio an opt-in.

[choldgraf]

Correct me if I'm wrong, but this is also removing one of the more-common user-facing methods execute, no?

[davidbrochart]

No, but I agree it looks like it. It's actually monkey-patched with the make_blocking function. Not sure this is acceptable though.

[davidbrochart]

I removed the monkey-patching of NotebookClient. Now execute and execute_cell are explicitly declared and call run_blocking on their async counterparts.

[davidbrochart]

Trigger CI.

[davidbrochart]

Trigger CI.

[choldgraf]

Another user-facing question from me - does it mean that running this from within Jupyter Notebooks will require using the next_asyncio flag? I feel like this will be not be obvious to people. Is there any way to check whether we need to nest based on environment variables etc? Or a way to catch the error message and display something like "try running with next_asyncio = True?

[davidbrochart]

Thanks for you comment @choldgraf, this last commit shows a meaningful message when nesting event loops seems to be needed.

[choldgraf]

Nice - I like that, I think it'll be helpful for people that aren't familiar with some of these issues (which unfortunately is probably exactly the kind of person that would be calling execute from within a notebook environment)

[davidbrochart]

I'm going to merge this PR soon, if nobody has objections.

[MSeal]

Sorry, was on vacation and dealing with COVID lockdown so I'm only now catching up on threads. Solution seems fine, just want to preserve comments :)

[davidbrochart]

Welcome back Matthew, I hope you are fine.
The last commit refactors the previous run_blocking into run_sync, which automatically inherits the docstring of the async method. Also, is_alive is now async in jupyter_client.

[davidbrochart]

The failing tests are related to #38 (comment), which will be solved when we have a jupyter_client release.
Also test_many_parallel_notebooks randomly fails, we need to investigate that.

[MSeal]

There was a minor conflict with your other PR I just merged, +1 to merge after the conflict is fixed. I'll see what's left after to keep from #34 and update that PR. Once this and 34 merges I think we can make a release.

test_parallel does fail randomly still ... as I currently understand it there's still a kernel socket race condition (I've fixed a number of them now) in how ports are registered. This might be a jupyter_client issue to track down tackle again.

[MSeal]

Welcome back Matthew, I hope you are fine.

Yep all good so far, went through some exposed places getting home but no symptoms as of 3 days in. Disconnected from my inbox for a bit was all.

[davidbrochart]

I will merge this PR soon, if no objection.

[MSeal]

merged