Remove gen.coroutine usage in scheduler
#3242
Conversation
Use `async`/`await` and `asyncio` idioms throughout.
| raise Return( | ||
| { | ||
| "status": "missing-data", | ||
| "keys": sum([r["keys"] for r in result if "keys" in r], []), |
There was a problem hiding this comment.
Was this wrong before? I would have expected this to iterate over result.values
There was a problem hiding this comment.
Yeah, this was wrong before.
There was a problem hiding this comment.
It's really good to have a second pair of eyes here
|
In principle everything here looks good to me. Thanks for doing this @jcrist I'm a little curious why I didn't change some of these over in the past. I wonder if there was some odd thing going on before. |
|
Looks like not all tests pass, so will have to dive deeper. Only ran a few checks locally since the test suite takes a while. In general what's your process with testing locally? Many things seem related, so its hard to know what tests are likely to fail. |
|
I usually run one of the files that I think is relevant, probably As context, before we got very stringent about checking for thread/process/fd leaks before and after every test (the |
File "/home/travis/build/dask/distributed/distributed/scheduler.py", line 2012, in cancel_key
lambda _: self.cancel_key(key, client, retries - 1),
File "/home/travis/miniconda/envs/test-environment/lib/python3.7/site-packages/tornado/ioloop.py", line 693, in add_future
assert is_future(future)It looks like this might be a relatively routine case of Tornado/Asyncio mismatch. This is probably why I left that one there. We could also try replacing |
|
Yeah, I'm going to keep pushing on this until we can remove this aspect of tornado from the codebase. Just need to go deeper. |
| self.loop.add_future( | ||
| gen.sleep(0.2), lambda _: self.cancel_key(key, client, retries - 1) | ||
| self.loop.call_later( | ||
| 0.2, lambda _: self.cancel_key(key, client, retries - 1) | ||
| ) |
There was a problem hiding this comment.
Fixed. There's no reason to manually call gen.sleep here, using IOLoop.call_later works just as well.
jcrist
force-pushed
the
no-gen-coroutine-scheduler
branch
from
86a66f9
to
e8eddd1
Compare
|
All tests pass. Good to merge? |
|
Woot. Thanks @jcrist ! |
Use
async/awaitandasyncioidioms throughout.