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

rename (move) file over different file system #234

Open
petrus-v opened this issue Jul 9, 2024 · 3 comments
Open

rename (move) file over different file system #234

petrus-v opened this issue Jul 9, 2024 · 3 comments
Labels
enhancement 🚀 New feature or request

Comments

@petrus-v
Copy link
Contributor

petrus-v commented Jul 9, 2024

I would like to be able to be able to rename/move a file over different file system.

Does it make sense, in this library to support such feature ?

This would let developers write something like this:

UPath("local:///tmp/local_file").rename(UPath("s3://my-bucket/destination"))

which would:

  • copy the source file from /tmp/local_file to the s3 bucket: s3://my-bucket/destination
  • remove the local file once file is write on the other file system
@petrus-v
Copy link
Contributor Author

Self comments from yesterday tests:

  • We should be able to do something similar if UPath("s3://my-bucket/destination").fs != UPath("s3://other-s3-instance/bucket/destination").fs, but at least for the first version support between local and remote filesystem would be great.

  • For the time being only support local:// and file:// (not PosixUPath nor WindowsUPath) because of inheritance loop between classes make it hard to refactor and choose an mro order suit to all caseses

@ap--
Copy link
Collaborator

ap-- commented Jul 10, 2024

Hi @petrus-v

Thank you for opening the issue. This is very much in scope for universal_pathlib. There is an open PR #225 that I still need to verify and merge, which provides better errors for the currently unsupported rename across filesystems.

Please also see #227 for future additions once we rely on Python3.14+ upstream implementations in pathlib.

That being said, cross-filesystem functionality could be added to UPath.rename. An implementation for your feature request would preferably be opt-in (.i.e. something like the code example below) to avoid users shooting themselves in the foot.

# example how the feature could look like
>>> src = UPath("/tmp/somefile")
>>> src.write_text('hello world')
>>> dst = UPath("s3://mybucket/path/somefile")
>>> src.rename(dst)
Traceback (most recent call last):
   ...
ValueError("cross-filesystem rename is not permitted by default. Use `allow_protocols=['s3']`")
>>> src.rename(dst, allow_protocols=['s3'])
>>> dst.read_text()
'hello world'

argument names are of course up for debate.

(not PosixUPath nor WindowsUPath) because of inheritance loop between classes make it hard to refactor and choose an mro order suit to all caseses

These might go away in the future once there is a reasonably well working implementation for relative UPaths.

@ap-- ap-- added the enhancement 🚀 New feature or request label Jul 10, 2024
@petrus-v
Copy link
Contributor Author

Hi !

Thanks taking time to reply, very appreciated.

Hi @petrus-v

Thank you for opening the issue. This is very much in scope for universal_pathlib. There is an open PR #225 that I still need to verify and merge, which provides better errors for the currently unsupported rename across filesystems.

Please also see #227 for future additions once we rely on Python3.14+ upstream implementations in pathlib.

That's nice upstream improvements 😍

That being said, cross-filesystem functionality could be added to UPath.rename. An implementation for your feature request would preferably be opt-in (.i.e. something like) to avoid users shooting themselves in the foot.

# example how the feature could look like
>>> src = UPath("/tmp/somefile")
>>> src.write_text('hello world')
>>> dst = UPath("s3://mybucket/path/somefile")
>>> src.rename(dst)
Traceback (most recent call last):
   ...
ValueError("cross-filesystem rename is not permitted by default. Use `allow_protocols=['s3']`")
>>> src.rename(dst, allow_protocols=['s3'])
>>> dst.read_text()
'hello world'

argument names are of course up for debate.

I'm fine to implement the opt-in and will probably allow to set it globally from environment variables if you agree, something like this:

def rename(dst, allow_protocols=None):
    if allow_protocols is None:
        allow_protocols = os.environ.get("UPATH_RENAME_ALLOW_PROTOCOLS", "").split(",")
    ...

I like the idea that source code know about where are store data and separate configuration and usages.

(not PosixUPath nor WindowsUPath) because of inheritance loop between classes make it hard to refactor and choose an mro order suit to all caseses

These might go away in the future once there is a reasonably well working implementation for relative UPaths.

sounds great 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🚀 New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants