Skip to content

core.py: fix exclusive write for small files #974

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.

Sign up for GitHub

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

Jump to bottom
Merged
martindurant merged 5 commits into from
Jul 8, 2025
Merged

core.py: fix exclusive write for small files #974

martindurant merged 5 commits into from
Jul 8, 2025

Conversation

@JayThomason
Copy link
Contributor

@JayThomason JayThomason commented Jul 1, 2025 •
edited
Loading

Prior to this change the exclusive write mode "x" did not behave properly for small files, instead acting the same as "w".

This change solves that by passing the IfNotMatch header to put_object even when writing small files that don't require a multipart upload.

Fixes #973

Tested by running the test script linked in the issue as well as updating the unit tests.

@martindurant
Copy link
Member

martindurant commented Jul 1, 2025

Can we please have a test function added? Perhaps also a zero-length file would be nice.

@JayThomason
Copy link
Contributor Author

JayThomason commented Jul 1, 2025

Can we please have a test function added? Perhaps also a zero-length file would be nice.

Sure let me work on that.

@JayThomason
Copy link
Contributor Author

JayThomason commented Jul 1, 2025

@martindurant I updated the unit tests and also discovered a related bug in moto: getmoto/moto#9028

Once this bug is fixed in moto the xfail decorator can be removed from the remaining tests.

@martindurant
Copy link
Member

martindurant commented Jul 1, 2025

Thank you.
The xfail tests still work against AWS?

Thanks for filing the bug with moto - please let us know if something happens there.

@martindurant
Copy link
Member

martindurant commented Jul 1, 2025 

FAILED s3fs/tests/test_s3fs.py::test_pipe_exclusive_big_after_small - Failed: DID NOT RAISE <class 'FileExistsError'>
FAILED s3fs/tests/test_s3fs.py::test_put_exclusive_big_after_small - Failed: DID NOT RAISE <class 'FileExistsError'>
FAILED s3fs/tests/test_s3fs.py::test_put_exclusive_small - Failed: DID NOT RAISE <class 'FileExistsError'>
= 3 failed, 324 passed, 2 skipped, 6 xfailed, 11 warnings in 114.84s (0:01:54) =

@martindurant
Copy link
Member

martindurant commented Jul 1, 2025

(check the version of moto and botocore you have versus the one in CI)

@JayThomason
Copy link
Contributor Author

JayThomason commented Jul 1, 2025

It looks like the tests will fail if the botocore version is < 1.33.0 because that is when IfNoneMatch support for conditional writes was added to the s3 client. I'll see if we can conditionally mark the tests as xfail depending on the botocore version to ensure they continue passing on newer botocore versions moving forward.

@JayThomason
Copy link
Contributor Author

JayThomason commented Jul 2, 2025

@martindurant is there anything else you want to see in this PR?

@martindurant
Copy link
Member

martindurant commented Jul 2, 2025

Still catching aiobotocore versions >=2.8.0,<2.9.0

Also some formatting

@JayThomason
core.py: fix exclusive write for small files
946e1ea 
Prior to this change the exclusive write mode "x" did not behave
properly for small files, instead acting the same as "w".

This change solves that by passing the IfNotMatch header to put_object
even when writing small files that don't require a multipart upload.
@JayThomason
update exclusive write unit tests
676ef1c
@JayThomason
make exclusive tests xfail based on botocore version
05040fc
@JayThomason
black lint
d46144f
@JayThomason
fix botocore version check: IfNoneMatch was added in 1.36.0 for put_o…
6f6b46d 
…bject
@JayThomason
Copy link
Contributor Author

JayThomason commented Jul 7, 2025

Thanks @martindurant I just took another stab at it. If you can trigger Ci that would be great.

After manually searching through the botocore docs of different versions I found IfNoneMatch for put_object and complete_multipart_upload wasn't added to botocore until 1.36.0, not 1.33.0.

@martindurant martindurant merged commit f34a9f8 into fsspec:main Jul 8, 2025
21 checks passed
@martindurant
Copy link
Member

martindurant commented Jul 8, 2025

+1

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Exclusive write does not work for small files

2 participants

@JayThomason @martindurant