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

object_store: Cache-Control header usage #4498

Closed
Turbo87 opened this issue Jul 10, 2023 · 4 comments · Fixed by #5650
Closed

object_store: Cache-Control header usage #4498

Turbo87 opened this issue Jul 10, 2023 · 4 comments · Fixed by #5650
Labels
question Further information is requested

Comments

@Turbo87
Copy link
Contributor

Turbo87 commented Jul 10, 2023

Which part is this question about

object_store

Describe your question

https://docs.aws.amazon.com/whitepapers/latest/build-static-websites-aws/controlling-how-long-amazon-s3-content-is-cached-by-amazon-cloudfront.html and https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutObject.html describe that the Cache-Control header can be used to set the caching behavior for individual files.

Is there a way in object_store to use this header?

It appears to be possible to influence the default_headers of the underlying reqwest client, but that would presumably then set the header for all requests instead of just the PutObject requests.

Ideally it would be possible to set this value on a per-file basis, but for our use-case it would be sufficient to have this as an optional per-store setting.

@Turbo87 Turbo87 added the question Further information is requested label Jul 10, 2023
@Turbo87
Copy link
Contributor Author

Turbo87 commented Jul 10, 2023

@tustvold would it make sense to have put_opts() similar to get_opts()? I guess that would be a breaking change though

@tustvold
Copy link
Contributor

but that would presumably then set the header for all requests instead of just the PutObject requests

Is this actually an issue, I would have perhaps expected S3 to just ignore headers it doesn't recognise? We could also definitely consider adding headers for specific HTTP methods perhaps 🤔

put_opts

I think I would rather avoid this if possible, as not all stores will have consistent support for these options.

@Turbo87
Copy link
Contributor Author

Turbo87 commented Jul 10, 2023

Is this actually an issue, I would have perhaps expected S3 to just ignore headers it doesn't recognise?

that is a good question. since it's a regular HTTP header I was assuming that some of the other requests might assign unintentional meaning to it.

We could also definitely consider adding headers for specific HTTP methods perhaps 🤔

I guess that would solve the issue to some degree, although it'd be nice to use the same S3 store for files in the same bucket but with different caching headers.

I think I would rather avoid this if possible, as not all stores will have consistent support for these options.

yeah, I'm not sure how exactly it would work either. we could just have PutOptions { extra_headers: HeaderMap } to make it implementation specific, or do PutOptions { cache_control: Something } and ignore it for implementations that don't support anything like that.

@Turbo87
Copy link
Contributor Author

Turbo87 commented Jul 12, 2023

FWIW https://github.com/rust-lang/crates.io/blob/5418db15823b6726a4b8e6dd292b01de37c227e1/src/storage.rs#L69-L71 is what we're doing as a workaround for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants