WIP: Remove nested async by copying code

[martindurant]

Major change to async code!

All nested calls to sync removed, by copying code such as walk() from spec to asyn. All async method only call async methods, and an attempt to do otherwise raises NotImplemented. In fact, _run_until_complete is gone. This is how it should have been written from the start (except that some copied code could clearly be factored out).

cc @hayesgb , this surely affects azure
s3fs and gcsfs already have conforming code locally I will push when we agree on this change.

[martindurant]

Documentation will need to be very clear about about to write an async implementation.

cf fsspec/gcsfs#363 , which still fails due to a nested sync call somewhere.

cc @mrocklin - this should make calling async things much more robust.

[hayesgb]

@martindurant -- Want to be sure I'm clear. No calls from async code to sync code, correct?

[TomAugspurger]

I'm happy to see this :) @hayesgb LMK if you need a hand on the adlfs side of things.

[martindurant]

No calls from async code to sync code

Specifically: async code cannot call sync code that calls async code.
For example, previously get was a sync function created by sync(_get), so that you can call it from normal code. However, it internally calls _expand_path which, in the spec implementation calls find, which also is sync(_find). That no longer happens, _expand_path's code is copied into AsyncFileSystem and calls await _find.

In other news...
#572 moved the loop into a threadlocal variable, so that you have one event loop per thread instead of a single event loop in its own thread. This required some objects deeply tied to the loop, like http session objects, to also be threadlocal. I don't know if this affects azure.

[martindurant]

@hayesgb @TomAugspurger - probably should include adlfs in the list of things that get tested here, as s3fs and gcsfs already are. That would be a separate PR.

[hayesgb]

Agreed. I’m covered up with other stuff at the moment, and I’m sure this was impacting memory usage when calling .close() on the File object, which prompted a direct call to asyncio in the current master branch. @TomAugspurger - if you have time to jump in that would be great. I was hoping to do a 0.7 release soon...

…

On Mar 24, 2021, at 7:49 AM, Martin Durant ***@***.***> wrote:  @hayesgb @TomAugspurger - probably should include adlfs in the list of things that get tested here, as s3fs and gcsfs already are. That would be a separate PR. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[TomAugspurger]

Sounds good. I'll take a look at this today.

[TomAugspurger]

Locally, I see three failures from the adlfs test suite, but those three failures are also present on fsspec master. At a glance, some look related to "directory" handling, which I think is discussed in #562.

If needed I can bisect fsspec to the commit where things started failing (they pass with 0.8.7) but I'm guessing it's already known.

[martindurant]

Sounds good, @TomAugspurger - let's cover that in the separate issue, then.

[hayesgb]

I’m going to execute a release of adlfs. Based on this discussion, I’ll pin fsspec<=0.8.7, with the intention to remove the pin once this is resolved.