Skip to content

Async cp/get/put the same as sync behaviour#1163

Merged
martindurant merged 1 commit into
fsspec:masterfrom
ianthomas23:async_cp_get_put
Jan 24, 2023
Merged

Async cp/get/put the same as sync behaviour#1163
martindurant merged 1 commit into
fsspec:masterfrom
ianthomas23:async_cp_get_put

Conversation

@ianthomas23
Copy link
Copy Markdown
Collaborator

This PR implements the correct _cp, _get and _put behaviour for AsyncFileSystem that PR #1148 implemented for the synchronous AbstractFileSystem. The changes are all in asyn.py and are almost identical to those made in spec.py except where a check for a trailing slash needs to be made earlier on as a subsequent call to _strip_protocol also strips the trailing slash.

There are no tests added here, instead I will shortly submit PRs to both s3fs and gcsfs to add them in line with the similar tests made in PR #1148. The tests I have added to those s3fs and gcsfs branches all fail when run against the latest commit of fsspec and pass when run against this PR. We will need to coordinate the CI/merging of the three PRs before we can be completely sure that everything consistently works together.

@martindurant
Copy link
Copy Markdown
Member

The CIs pull from the other repos main/master branches, but could be set temporarily to the incoming branch. Im not actually worried if CI shows red for a day or more while the different components land, so maybe best not to change the CI settings.

Having made all these changes, it would be interesting to compare with what generic.rsync does for similar situations.

@martindurant
Copy link
Copy Markdown
Member

Also, the green ✅ suggests that gcsfs needs more tests, we should have expected something to break!

@ianthomas23
Copy link
Copy Markdown
Collaborator Author

Having made all these changes, it would be interesting to compare with what generic.rsync does for similar situations.

I'll take a look, and also check what command-line rsync does too.

@martindurant
Copy link
Copy Markdown
Member

:) our rsync doesn't really get close to the many options of gnu rsync

@martindurant
Copy link
Copy Markdown
Member

martindurant commented Jan 24, 2023

The code here looks good, I am happy to merge when you say you are done, and the changes in gcsfs and s3fs can follow.

@hayesgb @TomAugspurger - this may affect adlfs

@ianthomas23
Copy link
Copy Markdown
Collaborator Author

The code here looks good, I am happy to merge when you say you are done, and the changes in gcsfs and s3fs can follow.

I am done here, and happy for this to be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants