Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consistent recursive cp, get and put into non-existent target directory #1148

Merged
merged 4 commits into from
Jan 20, 2023

Conversation

ianthomas23
Copy link
Collaborator

This is a WIP for #1062. Running the test script from that issue gives the following output:

==> cp
  loop 0: ['/tmp/target/a']
  loop 1: ['/tmp/target/a', '/tmp/target/source', '/tmp/target/source/a']
==> cp, slash
  loop 0: ['/tmp/target/a']
  loop 1: ['/tmp/target/a']
==> rsync
  loop 0: ['/tmp/target/source', '/tmp/target/source/a']
  loop 1: ['/tmp/target/source', '/tmp/target/source/a']
==> rsync, slash
  loop 0: ['/tmp/target/a']
  loop 1: ['/tmp/target/a']
==> scp
  loop 0: ['/tmp/target/a']
  loop 1: ['/tmp/target/a', '/tmp/target/source', '/tmp/target/source/a']
==> scp, slash
  loop 0: ['/tmp/target/a']
  loop 1: ['/tmp/target/a']
==> fs.cp
  loop 0: ['/tmp/target/a']
  loop 1: ['/tmp/target/a', '/tmp/target/source', '/tmp/target/source/a']
==> fs.cp, slash
  loop 0: ['/tmp/target/a']
  loop 1: ['/tmp/target/a']
==> fs.get
  loop 0: ['/tmp/target/a']
  loop 1: ['/tmp/target/a', '/tmp/target/source', '/tmp/target/source/a']
==> fs.get, slash
  loop 0: ['/tmp/target/a']
  loop 1: ['/tmp/target/a']
==> fs.put
  loop 0: ['/tmp/target/a']
  loop 1: ['/tmp/target/a', '/tmp/target/source', '/tmp/target/source/a']
==> fs.put, slash
  loop 0: ['/tmp/target/a']
  loop 1: ['/tmp/target/a']
==> mem.cp
  loop 0: ['/remote_target/a']
  loop 1: ['/remote_target/a', '/remote_target/remote_source', '/remote_target/remote_source/a']
==> mem.cp, slash
  loop 0: ['/remote_target/a']
  loop 1: ['/remote_target/a']
==> mem.get
  loop 0: ['/tmp/target/a']
  loop 1: ['/tmp/target/a', '/tmp/target/remote_source', '/tmp/target/remote_source/a']
==> mem.get, slash
  loop 0: ['/tmp/target/a']
  loop 1: ['/tmp/target/a']
==> mem.put
  loop 0: ['/remote_target/a']
  loop 1: ['/remote_target/a', '/remote_target/source', '/remote_target/source/a']
==> mem.put, slash
  loop 0: ['/remote_target/a']
  loop 1: ['/remote_target/a']

showing that the fsspec functions cp, get and put are consistent with command-line cp and scp.

The solution is mostly about passing the correct arguments to utils.other_paths() to obtain the required behaviour.

It is only a WIP for two reasons:

  1. I have broken the use case of fs.copy(['/tmp/source/a', '/tmp/source/b'], '/tmp/target') and don't yet have a fix for it.
  2. I have only fixed other_paths() calls in spec.py not in asyn.py, which presumably needs similar logic.

@ianthomas23 ianthomas23 force-pushed the consistent_cp_get_put branch 2 times, most recently from 7c86326 to d99a56c Compare January 17, 2023 15:17
@ianthomas23 ianthomas23 force-pushed the consistent_cp_get_put branch from d99a56c to dc83003 Compare January 17, 2023 16:08
@ianthomas23
Copy link
Collaborator Author

This now works for the synchronous case, including for copying 2+ named files. I still need to update and test the async side of things.

Copy link
Member

@martindurant martindurant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks good!
One question.

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