python-trio / trio-asyncio

a re-implementation of the asyncio mainloop on top of Trio
Other
188 stars 37 forks source link

Bring trio-asyncio into the next decade #66

Closed oremanj closed 4 years ago

oremanj commented 4 years ago

A collection of changes to make CI pass, including on 3.8.

Fix use of deprecated Trio features:

Collect Python stdlib asyncio tests from the installed stdlib, rather than trying to vendor them. This lets us run the tests on far more platforms.

Ignore some asyncio deprecation warnings on 3.8 for now.

Clean up the monkeypatching of asyncio event loop and event loop policy accessors. The primary functional changes are:

Improve SyncTrioEventLoop to not leak threads so much.

oremanj commented 4 years ago

Sigh, and of course the CI python versions are too old for these tests now.

Seems like we should probably try to collect tests from the installed Python, rather than vendoring them. We can have a list of which need to add skip/xfail markers for which Python version. I'll try that approach next.

codecov[bot] commented 4 years ago

Codecov Report

Merging #66 into next will decrease coverage by 0.73%. The diff coverage is 89.53%.

@@            Coverage Diff            @@
##            next      #66      +/-   ##
=========================================
- Coverage   73.4%   72.66%   -0.74%     
=========================================
  Files         11       11              
  Lines       1267     1284      +17     
  Branches     166      177      +11     
=========================================
+ Hits         930      933       +3     
+ Misses       282      281       -1     
- Partials      55       70      +15
Impacted Files Coverage Δ
trio_asyncio/adapter.py 75.93% <ø> (-0.22%) :arrow_down:
trio_asyncio/sync.py 71.42% <100%> (-9.68%) :arrow_down:
trio_asyncio/base.py 81.9% <100%> (-3.52%) :arrow_down:
trio_asyncio/async_.py 81.03% <33.33%> (-4.45%) :arrow_down:
trio_asyncio/loop.py 68.07% <90.27%> (+11.7%) :arrow_up:
trio_asyncio/handles.py 77.57% <0%> (-3.74%) :arrow_down:
trio_asyncio/child.py 66.66% <0%> (+0.33%) :arrow_up:
... and 2 more
oremanj commented 4 years ago

Down to 7 failing tests on py3.7, and one on py3.6. (This is without copying the skip/xfail markers from the previous set of vendored tests, except for one.)

The greenlet-based SyncTrioEventLoop is kind of evil, to be sure, but I found it to work a lot more reliably than the threading-based one.

smurfix commented 4 years ago

Using greenlet instead of thread is a great idea, should help a lot.

oremanj commented 4 years ago

I think the greenlet direction is a good one to explore, but it was cluttering this diff too much, and ideally we'd have Trio support for it so we don't have to muck with Trio internals so much. Will revisit at a later time.

This passes 3.8 for me now locally.

dhirschfeld commented 4 years ago

Just tested trio-async master against trio master on py38/win-64 and it's looking pretty good - just a single error:

=================== FAILURES ====================
_______________________________ ProactorLoopCtrlC.test_ctrl_c _______________________________

self = <test.test_asyncio.test_windows_events.ProactorLoopCtrlC testMethod=test_ctrl_c>

    def test_ctrl_c(self):

        def SIGINT_after_delay():
            time.sleep(0.1)
            signal.raise_signal(signal.SIGINT)

        thread = threading.Thread(target=SIGINT_after_delay)
>       loop = asyncio.get_event_loop()

..\..\..\..\envs\test-trio\lib\test\test_asyncio\test_windows_events.py:48:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
trio_asyncio\loop.py:274: in _new_loop_get
    return _trio_policy.get_event_loop()
trio_asyncio\loop.py:182: in get_event_loop
    return super().get_event_loop()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <trio_asyncio.loop.TrioPolicy object at 0x000002563F576C40>

    def get_event_loop(self):
        """Get the event loop for the current context.

        Returns an instance of EventLoop or raises an exception.
        """
        if (self._local._loop is None and
                not self._local._set_called and
                isinstance(threading.current_thread(), threading._MainThread)):
            self.set_event_loop(self.new_event_loop())

        if self._local._loop is None:
>           raise RuntimeError('There is no current event loop in thread %r.'
                               % threading.current_thread().name)
E           RuntimeError: There is no current event loop in thread 'MainThread'.

..\..\..\..\envs\test-trio\lib\asyncio\events.py:639: RuntimeError
======================== 1 failed, 1886 passed, 78 skipped, 3 xfailed in 163.66s (0:02:43) ========================
parity3 commented 4 years ago

In the pre-merged version, as noted by https://github.com/python-trio/trio-asyncio/issues/63 I was able to work around this by running:

asyncio._set_running_loop(asyncio.get_event_loop())

In the top trio_asyncio.run function.

dhirschfeld commented 4 years ago

Thanks for the heads-up @parity3! I'll still be on py37 for the next several months but I'll keep this in mind for when I make the transition to py38!