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

Copy to non-existing S3 folder path creates a file with "/" instead of file name in that folder #451

Open
quantori-pokidovea opened this issue Jul 4, 2024 · 1 comment

Comments

@quantori-pokidovea
Copy link

Steps to reproduce:

# take an existing file
src = S3Path("s3://my-bucket/file.txt")
# and copy it into non-existent folder
dst = S3Path("s3://my-bucket/destination/")

src.copy(dst)

Current behavior

Creates a file with name / inside the folder s3://my-bucket/destination/

Expected behavior

The file s3://my-bucket/destination/file.txt is created

@pjbull
Copy link
Member

pjbull commented Jul 4, 2024

FWIW .copy is meant to mimic the behavior of the copy function from shutil, which has similarly potentially unintuitive behavior (it both strips the delimiter and writes dst as a file).

from pathlib import Path
from shutil import copy

src = Path("text_file.txt")
src.write_text("hello")
#> 5

dst = Path("new_target/")

copy(src, dst)
#> PosixPath('new_target')

print(dst.is_file())
#> True
print(dst.read_text())
#> hello

Since this is CloudPath only API that is not on a Path object, we do have some leeway on deciding if there is a better approach. However, there is some ambiguity in the right approach since s3://bucket/file-end-in-slash/ is a valid name for a blob/object on at least some of the providers.

Some options:

  • (as suggested above) Treat CloudPaths ending in / as directories in copy even if they don't exist
  • Add a kwarg/setting to treat CloudPaths ending in / as directories in copy explicitly
  • Add a warning if we ever write to a CloudPath ending in / (since this is likely user error)
  • Leave as-is and document

If we document, the workaround is to concat the paths yourself (since not all providers even support empty folders):

src = S3Path("s3://my-bucket/file.txt")
dst = S3Path("s3://my-bucket/destination/")

src.copy(dst / src.name)

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

No branches or pull requests

2 participants