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

Enable conditional_put for s3 by default #221

Open
ion-elgreco opened this issue Feb 5, 2025 · 7 comments
Open

Enable conditional_put for s3 by default #221

ion-elgreco opened this issue Feb 5, 2025 · 7 comments

Comments

@ion-elgreco
Copy link

This is something we did in delta-rs, since all known S3 stores have this support now.

Let me know what you think @kylebarron

@kylebarron
Copy link
Member

kylebarron commented Feb 5, 2025

Can you link to the delta-rs PR? I'm not the most knowledgeable about conditional puts and I don't know what would change.

@ion-elgreco
Copy link
Author

delta-io/delta-rs#3091

Enabling it means, you can do a put request with PutMode create without it throwing errors of not supported file operation.

@kylebarron
Copy link
Member

kylebarron commented Feb 5, 2025

I glanced through that PR but still don't know what would change in obstore. This is what I currently expose in the config: https://developmentseed.org/obstore/latest/api/store/aws/#obstore.store.S3Config.conditional_put

so to enable conditional put currently you'd use

S3Store(..., conditional_put="etag")

is that correct?

@kylebarron
Copy link
Member

Is there upstream discussion about making that the default behavior for AmazonS3?

@ion-elgreco
Copy link
Author

I glanced through that PR but still don't know what would change in obstore. This is what I currently expose in the config: https://developmentseed.org/obstore/latest/api/store/aws/#obstore.store.S3Config.conditional_put

so to enable conditional put currently you'd use

S3Store(..., conditional_put="etag")
is that correct?

Correct, we opted to insert these flags by default.

Is there upstream discussion about making that the default behavior for AmazonS3?

I think in the original PR when support was added for this, they left it off by default since it wasn't fully supported, but to my understanding this fully support by AWS was added couple months later? I think we should check with @tustvold

@kylebarron
Copy link
Member

kylebarron commented Feb 5, 2025

I'm inclined to wait and follow whatever defaults object_store chooses. object_store v0.12 is expected to be released this month, right? Would you like to create an upstream issue asking/confirming the status of the default behavior of conditional put?

ref apache/arrow-rs#7080

@kylebarron
Copy link
Member

https://github.com/apache/arrow-rs/blob/1019f5b27d3596077bcdd7e10b67e2c6d4cfbf02/object_store/src/aws/precondition.rs#L133-L138 this docstring should probably be updated to include native S3 as a store that supports it?

Maybe we should discuss assigning #[default] on the ETagMatch variant?

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