Add support for Python 3.8

[jrbourbeau]

See the corresponding dask/dask PR at dask/dask#5603

[TomAugspurger]

Seems like a slight issue with stacktrace:

UnsatisfiableError: The following specifications were found

to be incompatible with the existing python installation in your environment:

Specifications:

  - stacktrace -> python[version='>=3.5,<3.6.0a0|>=3.6,<3.7.0a0|>=3.7,<3.8.0a0']

Your python: python=3.8

If python is on the left-most side of the chain, that's the version you've asked for.

When python appears to the right, that indicates that the thing on the left is somehow

not available for the python version you are constrained to. Note that conda will not

change your python version to a different minor version unless you explicitly specify

that.

The following specifications were found to be incompatible with each other:

Package setuptools conflicts for:

stacktrace -> python[version='>=3.6,<3.7.0a0'] -> pip -> setuptools

python=3.8 -> pip -> setuptools

Package certifi conflicts for:

stacktrace -> python[version='>=3.6,<3.7.0a0'] -> pip -> setuptools -> certifi[version='>=2016.09|>=2016.9.26']

python=3.8 -> pip -> setuptools -> certifi[version='>=2016.09|>=2016.9.26']

Package pip conflicts for:

python=3.8 -> pip

stacktrace -> python[version='>=3.6,<3.7.0a0'] -> pip

Package wheel conflicts for:

stacktrace -> python[version='>=3.6,<3.7.0a0'] -> pip -> wheel

python=3.8 -> pip -> wheel

Package ca-certificates conflicts for:

stacktrace -> python[version='>=3.6,<3.7.0a0'] -> openssl=1.0 -> ca-certificates

python=3.8 -> openssl[version='>=1.1.1d,<1.1.2a'] -> ca-certificates

I think we can remove that from the 3.8 build.

[jrbourbeau]

Thanks for reviewing @TomAugspurger. Yeah, stacktrace isn't available for Python 3.8 yet. Just pushed a commit which skips the install for 3.8

[TomAugspurger]

Failure on 3.8 in

_________________________ test_cleanup_repeated_tasks __________________________

...
>       assert not list(ws)

E       AssertionError: assert not [<test_steal.test_cleanup_repeated_tasks.<locals>.Foo object at 0x7f60b5e8daf0>, <test_steal.test_cleanup_repeated_tasks.<locals>.Foo object at 0x7f606b665ca0>]

E        +  where [<test_steal.test_cleanup_repeated_tasks.<locals>.Foo object at 0x7f60b5e8daf0>, <test_steal.test_cleanup_repeated_tasks.<locals>.Foo object at 0x7f606b665ca0>] = list({<weakref at 0x7f6062ab9bd0; to 'Foo' at 0x7f60b5e8daf0>, <weakref at 0x7f6062ab9680; to 'Foo' at 0x7f606b665ca0>})

I don't recall seeing that before.

[TomAugspurger]

Restarted the 3.8 build to check, though I think we'll want to avoid adding new flaky tests :)

[jrbourbeau]

CI passed. Restarting again

[PMeira]

I noticed the AppVeyor/Windows tests only include Python 3.6, so this doesn't address the Tornado situation with Python 3.8 on Windows, right? Context: tornadoweb/tornado#2608

I'm testing Distributed with Python 3.8 on Windows (most packages from conda-forge) and it seems to be working fine for my usage (1 thread per worker) after adjusting the event loop policy. To do that, I added...

        if sys.platform == 'win32':
            asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy())

...right after:

distributed/distributed/utils.py

Lines 1194 to 1198 in b6fb541

	import tornado.platform.asyncio
	asyncio.set_event_loop_policy(
	tornado.platform.asyncio.AnyThreadEventLoopPolicy()
	)

(not sure if it's the best place for that or even if it's generic enough)

[jrbourbeau]

There was a Python 3.8-related fix to tornado in version 6.0.3 (tornadoweb/tornado#2683). This causes only one of our tests, distributed/deploy/tests/test_local.py::test_silent_startup, to fail and I've xfailed it here.

However, when using tornado < 6.0.3 on Python 3.8 there are many asyncio.exceptions.CancelledError tracebacks logged to the screen:

Exception in callback with_timeout.<locals>.error_callback(<Future cancelled>) at /Users/jbourbeau/miniconda/envs/dist-3.8-tornado-5/lib/python3.8/site-packages/tornado/gen.py:968
handle: <Handle with_timeout.<locals>.error_callback(<Future cancelled>) at /Users/jbourbeau/miniconda/envs/dist-3.8-tornado-5/lib/python3.8/site-packages/tornado/gen.py:968>
Traceback (most recent call last):
  File "/Users/jbourbeau/miniconda/envs/dist-3.8-tornado-5/lib/python3.8/asyncio/events.py", line 81, in _run
    self._context.run(self._callback, *self._args)
  File "/Users/jbourbeau/miniconda/envs/dist-3.8-tornado-5/lib/python3.8/site-packages/tornado/gen.py", line 970, in error_callback
    future.result()
asyncio.exceptions.CancelledError

Are there any objections to bumping our minimum tornado version to 6.0.3 (for reference, this version was released on Jun 22, 2019)? Not sure if this is too aggressive a bump for most users cc @mrocklin

[TomAugspurger]

@jrbourbeau I think there are way to do version-specific requirements: https://setuptools.readthedocs.io/en/latest/setuptools.html#declaring-platform-specific-dependencies

It'll complicate our setup.py / conda recipe a bit, but is probably worth it for tornado, maybe?

[mrocklin]

I've run into some systems that still don't react well to Tornado 6. For example I think that our dask-examples binder deployment might not work due to some Jupyter/Tornado issues. I could be wrong though.

[mrocklin]

This might also be good motivation to replace gen.with_timeout with the asyncio equivalent as you suggested in #3367 ?

[mrocklin]

In general I find that test_silent_startup is valuable to have active if we can manage it. Seeing a bunch of warning messages pop up during usage is distracting, and gives a sense that we're less mature of a project than we are.

[jrbourbeau]

This might also be good motivation to replace gen.with_timeout with the asyncio equivalent as you suggested in #3367 ?

Yeah, that would help us avoid the tornado issue. I opened up #3372 to see how the test suite handles the gen.with_timeout -> asyncio.wait_for switch

In general I find that test_silent_startup is valuable to have active if we can manage it

Agreed

[jrbourbeau]

In Python 3.8, asyncio.CancelledError was changed from being a subclass of concurrent.futures.CancelledError to a subclass of BaseException (xref python/cpython#13528). This means that before when throwing and catching CancelledErrors, both asyncio.CancelledError and concurrent.futures.CancelledError would be caught by except concurrent.futures.CancelledError. With Python 3.8+ this is no longer the case so we need to be a little more careful about how we handle CancelledErrors.

Luckily, almost all the occurrences of CancelledError in the codebase are intended to be concurrent.futures.CancelledError. So I've added concurrent.futures.CancelledError to the distributed.utils namespace so we can standardize on it throughout the project (similar to the TimeoutError standardization in #3394). Moving forward, if a CancelledError from asyncio is needed we should explicitly use asyncio.CancelledError.

[mrocklin]

That seems sensible to me. Thank you for diving into this @jrbourbeau

[jrbourbeau]

Np, it's a subtle change that's good to know about

A few other notes:

• I've updated the title of this PR to be a WIP to indicate we should hold off on merging until the 2.10.0 release is out in the next few days. That'll give these changes some time to simmer in master

• This PR should be ready for a review

• I'm going to re-run the CI builds a few times to identify possible flaky tests (all green checks at the time of writing this comment)

[TomAugspurger]

I think we're quite close. I think just a bit of debug code, and one question on the windows event loop stuff.

[TomAugspurger]

From what I understand, tornado decided not to do this for users by default. Did Jupyter decided whether or not to do it?

Is appropriate for Dask to me making this decision?

[jrbourbeau]

Tornado decided not to set the event loop policy to asyncio.WindowsSelectorEventLoopPolicy() by default and instead recommends projects do that themselves. Jupyter and several other projects have done this (e.g. jupyter/notebook#5047).

Our situation is a little different because we want to set the event loop policy to tornado's AnyThreadEventLoopPolicy to let us create loops inside of any thread. Tornado has decided to set the base class for AnyThreadEventLoopPolicy to asyncio.WindowsSelectorEventLoopPolicy on Windows (tornadoweb/tornado@d2af6a6), but there hasn't been a release with the those changes yet so we're doing that ourselves here. I'll add a note that we can remove this once the next tornado release is out.

[TomAugspurger]

Thanks @jrbourbeau. I think we're good to merge this whenever (once the CI is passing 😄).

[jrbourbeau]

CI is passing 🎉

For reference, there was a patch made to the conda-forge tornado feedstock that fixed the tornado event loop policy issue with Python 3.8 on Windows. Unfortunately the patch also broke things for us (hence the most recent CI failure). Fortunately, other packages have fixed things internally and the patch to the conda-forge recipe was recently removed (xref conda-forge/tornado-feedstock#36). It's worth noting that the same conda recipe patch was also made to tornado in the default channel and is still in place, so that's why we're installing tornado from conda-forge in CI now

[mrocklin]

Woo!

…

On Thu, Feb 13, 2020 at 2:10 PM James Bourbeau ***@***.***> wrote: Merged #3249 <#3249> into master. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3249?email_source=notifications&email_token=AACKZTH2MYZUZKMPC5JJMDTRCXANXA5CNFSM4JPF3UK2YY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOWUAJ2EY#event-3036716307>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACKZTDEHWS4SRCAZESDXCDRCXANXANCNFSM4JPF3UKQ> .