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

refactor: make put-if-absent default for S3 stores #3091

Merged
merged 5 commits into from
Jan 1, 2025

Conversation

ion-elgreco
Copy link
Collaborator

@github-actions github-actions bot added binding/python Issues for the Python package binding/rust Issues for the Rust crate labels Dec 29, 2024
@ion-elgreco ion-elgreco force-pushed the feat/s3_put_if_absent branch from 681c7b8 to d7d0eab Compare December 29, 2024 16:40
@rtyler rtyler marked this pull request as draft December 30, 2024 22:32
@rtyler rtyler self-assigned this Dec 30, 2024
@rtyler
Copy link
Member

rtyler commented Dec 30, 2024

This is a good change overall and I don't think it should be a probably to slyly slip this in as a default when the AWS_LOCKING_PROVIDER environment variable is not set.

In my setups for example I require the S3DynamoDbLogStore for concurrent coordination with Spark writers, so I will want to do a little more testing with this to make sure that the right log store is chosen in the right circumstances

@rtyler rtyler added this to the v0.23 milestone Dec 30, 2024
@rtyler rtyler marked this pull request as ready for review December 31, 2024 23:38
@rtyler rtyler enabled auto-merge December 31, 2024 23:38
@rtyler rtyler force-pushed the feat/s3_put_if_absent branch from 1849be3 to 0c833d6 Compare December 31, 2024 23:48
ion-elgreco and others added 5 commits December 31, 2024 23:53
The options handling must occur before the ObjectStore is actually
created in order for the object_store crate to properly inherit the
conditional_put configuration setting.

Otherwise an OSError is returned (object_store NotImplemented error)

Signed-off-by: R. Tyler Croy <[email protected]>
Copy link

codecov bot commented Dec 31, 2024

Codecov Report

Attention: Patch coverage is 84.78261% with 7 lines in your changes missing coverage. Please review.

Project coverage is 72.52%. Comparing base (cc863c0) to head (9323c97).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
crates/aws/src/storage.rs 82.60% 3 Missing and 1 partial ⚠️
crates/aws/src/lib.rs 86.95% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3091      +/-   ##
==========================================
+ Coverage   72.30%   72.52%   +0.21%     
==========================================
  Files         128      128              
  Lines       41173    41182       +9     
  Branches    41173    41182       +9     
==========================================
+ Hits        29770    29867      +97     
+ Misses       9498     9406      -92     
- Partials     1905     1909       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rtyler rtyler force-pushed the feat/s3_put_if_absent branch from 0c833d6 to 9323c97 Compare January 1, 2025 00:00
Copy link
Member

@rtyler rtyler left a comment

Choose a reason for hiding this comment

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

I've reviewed and updated this one 👍

@rtyler rtyler added this pull request to the merge queue Jan 1, 2025
Merged via the queue into delta-io:main with commit 64c1b1a Jan 1, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
binding/python Issues for the Python package binding/rust Issues for the Rust crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Utilize Amazon S3 condition write to support concurrent write
2 participants