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

put_file: default to overwrite=True #419

Merged
merged 3 commits into from
Jan 5, 2024

Conversation

pmrowla
Copy link
Contributor

@pmrowla pmrowla commented Jun 21, 2023

adlfs put_file() currently raises FileExistsError by default if the specified blob already exists. In version aware contexts put_file should default to succeeding even if the specified blob already exists, so that put_file creates a new version of the blob.


I should also note that as far as I can tell every other fsspec put_file implementation defaults to overwriting an existing file, so I'm also wondering whether the current adlfs overwrite=False default should be changed in all contexts (regardless of whether or not the fs is version aware)?

@TomAugspurger
Copy link
Contributor

Thanks for the PR.

I agree that we should match fsspec's default behavior here. I don't think this should depend on whether or not the filesystem is version aware or not.

And it's a bit of a hassle, but I think we should deprecate the current behavior first, by setting the default to None which means overwrite=False with a warning that it'll change to True in the future.

adlfs/spec.py Outdated Show resolved Hide resolved
Copy link
Contributor

@TomAugspurger TomAugspurger left a comment

Choose a reason for hiding this comment

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

We can clean this up if we're just changing stuff to overwrite=True.

We'll need a note in the changelog. Maybe a versionchanged in the docstring too.

adlfs/spec.py Outdated Show resolved Hide resolved
adlfs/spec.py Outdated Show resolved Hide resolved
adlfs/spec.py Outdated Show resolved Hide resolved
adlfs/spec.py Outdated Show resolved Hide resolved
@pmrowla pmrowla changed the title put_file: default to overwrite=True for version-aware fs put_file: default to overwrite=True Jan 3, 2024
@pmrowla
Copy link
Contributor Author

pmrowla commented Jan 3, 2024

Updated the PR as requested

@efiop efiop merged commit 488fdf0 into fsspec:main Jan 5, 2024
4 checks passed
@pmrowla pmrowla deleted the version-aware-overwrite branch January 5, 2024 08:45
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.

3 participants