BF: Save and restore existing asyncio event-loop

[mih]

This approach is copied from IPython. It prevents the crash reported in
gh-5064 by not leaving behind a closed event-loop, but rather any
previously existing one, or None.

Fixes gh-5064

[kyleam]

That brings back the BlockingIOError messages :/

BlockingIOError: [Errno 11] Resource temporarily unavailable
Exception ignored when trying to write to the signal wakeup fd:

[kyleam]

I revisited some of the python bug reports about BlockingIOError. I didn't gain much clarity, but somewhere in the followup searching I came across a commit that seems to work around the same issue.

It involves cleaning up with set_event_loop(None), so I doubt it would resolve gh-5064 because I don't think passing None has the effect of restoring the previous loop. But, in case someone that can trigger the issue (*) thinks it's worth testing, below is the change on top of this PR's base commit.

(*) I tried each recipe in gh-5064 with python 3.7.3 and ipython 7.19.0 installed in the virtualenv, not system wide, but was unable to trigger the failure.

patch

diff --git a/datalad/cmd.py b/datalad/cmd.py
index d861bcd8c..74df00f41 100644
--- a/datalad/cmd.py
+++ b/datalad/cmd.py
@@ -476,21 +476,24 @@ def run(self, cmd, protocol=None, stdin=None, cwd=None, env=None, **kwargs):
         else:
             event_loop = asyncio.SelectorEventLoop()
         asyncio.set_event_loop(event_loop)
-        # include the subprocess manager in the asyncio event loop
-        results = event_loop.run_until_complete(
-            run_async_cmd(
-                event_loop,
-                cmd,
-                protocol,
-                stdin,
-                protocol_kwargs=kwargs,
-                cwd=cwd,
-                env=env,
+        try:
+            # include the subprocess manager in the asyncio event loop
+            results = event_loop.run_until_complete(
+                run_async_cmd(
+                    event_loop,
+                    cmd,
+                    protocol,
+                    stdin,
+                    protocol_kwargs=kwargs,
+                    cwd=cwd,
+                    env=env,
+                )
             )
-        )
-        # terminate the event loop, cannot be undone, hence we start a fresh
-        # one each time (see BlockingIOError notes above)
-        event_loop.close()
+        finally:
+            asyncio.set_event_loop(None)
+            # terminate the event loop, cannot be undone, hence we start a fresh
+            # one each time (see BlockingIOError notes above)
+            event_loop.close()
 
         # log before any exception is raised
         lgr.log(8, "Finished running %r with status %s", cmd, results['code'])

[codecov]

Codecov Report

Merging #5106 (1770acf) into maint (295c967) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##            maint    #5106      +/-   ##
==========================================
+ Coverage   89.90%   89.92%   +0.02%     
==========================================
  Files         294      294              
  Lines       40953    40955       +2     
==========================================
+ Hits        36817    36829      +12     
+ Misses       4136     4126      -10     

Impacted Files	Coverage Δ
datalad/cmd.py	93.73% <100.00%> (+0.02%)	⬆️
datalad/downloaders/tests/test_http.py	90.49% <0.00%> (+1.90%)	⬆️
datalad/interface/tests/test_unlock.py	97.93% <0.00%> (+2.06%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 295c967...1770acf. Read the comment docs.

[mih]

Even though I still don't understand this properly, it looks good now:

% git co master
Switched to branch 'master'
Your branch is up to date with 'origin/master'.
% ipython
Python 3.7.3 (default, Apr  3 2019, 05:39:12) 
Type 'copyright', 'credits' or 'license' for more information
IPython 7.18.1 -- An enhanced Interactive Python. Type '?' for help.

In [1]: import datalad.api as dl

In [2]: dl.__version__
Traceback (most recent call last):
 ...
RuntimeError: Event loop is closed

If you suspect this is an IPython 7.18.1 bug, please report it at:
    https://github.com/ipython/ipython/issues
or send an email to the mailing list at ipython-dev@python.org

You can print a more detailed traceback right now with "%tb", or use "%debug"
to interactively debug it.

Extra-detailed tracebacks for bug-reporting purposes can be enabled via:
    %config Application.verbose_crash=True

% git co bf-5064
Switched to branch 'bf-5064'
Your branch is up to date with 'origin/bf-5064'.
(datalad) mih@juseless ~/code/datalad (git)-[bf-5064] % git log -n1 | cat
commit 1770acf58ffc0abe4669e5a5ecb8bbd769da6f1f
Author: Kyle Meyer <kyle@kyleam.com>
Date:   Fri Dec 4 16:26:39 2020 +0100

    BF: Fixup of the previous attempt to prevent gh-5064
    
    without flooding the terminal with messages.
    
    https://github.com/datalad/datalad/pull/5106#issuecomment-729863717
    
    @mih cannot trigger the ipython crash with this change anymore
    (but replicated on plain `maint` before)
(datalad) mih@juseless ~/code/datalad (git)-[bf-5064] % ipython
Python 3.7.3 (default, Apr  3 2019, 05:39:12) 
Type 'copyright', 'credits' or 'license' for more information
IPython 7.18.1 -- An enhanced Interactive Python. Type '?' for help.

In [1]: import datalad.api as dl

In [2]: dl.__version__
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-2-d3eff33a724a> in <module>
----> 1 dl.__version__

AttributeError: module 'datalad.api' has no attribute '__version__'

I will merge! Thx @kyleam !