Add documentation for cache control headers on cloud-backed media #4599
Conversation
WalkthroughThe recent update enhances the documentation by adding a section on "Browser Caching." This new content highlights the significance of enabling built-in browser caching for cloud-backed media, providing specific guidance for various cloud platforms. Users are equipped with actionable insights to optimize their data handling and improve performance through effective cache-control header configurations. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- docs/source/teams/installation.rst (5 hunks)
Additional comments not posted (5)
docs/source/teams/installation.rst (5)
180-186: Clear and informative introduction to browser caching.The section provides a good overview of the importance of enabling browser caching for cloud-backed media.
274-276: Accurate and complete instructions for Amazon S3.The section provides clear instructions for specifying cache-control headers on S3 objects.
342-344: Accurate and complete instructions for Google Cloud Storage.The section provides clear instructions for specifying cache-control headers on GCP content.
456-458: Accurate and complete instructions for Microsoft Azure.The section provides clear instructions for specifying cache-control headers on Azure blobs.
530-532: Accurate and complete instructions for MinIO.The section provides clear instructions for specifying cache-control headers on MinIO content using the metadata field of the put_object API.
docs/source/teams/installation.rst
Outdated
| If you would like to take advantage of browser caching you can | ||
| `specify cache-control headers on S3 objects <https://docs.aws.amazon.com/whitepapers/latest/build-static-websites-aws/controlling-how-long-amazon-s3-content-is-cached-by-amazon-cloudfront.html#specify-cache-control-headers>`_. | ||
|
|
There was a problem hiding this comment.
Should we mention that S3 by-default doesn't send anything for cache-controls header and that if customers are unsure, they can leave it as-is and up to the browser heuristics engine? (which uses other metadata like mime-type, etc to determine a reasonable default)
docs/source/teams/installation.rst
Outdated
| If you would like to take advantage of browser caching you can | ||
| `specify cache-control headers on GCP content <https://cloud.google.com/storage/docs/metadata#cache-control>`_. | ||
|
|
There was a problem hiding this comment.
I think we should mention the max-age=0 default here and how it is pernicious.
docs/source/teams/installation.rst
Outdated
| If you would like to take advantage of browser caching you can | ||
| `specify cache-control headers on Azure blobs <https://learn.microsoft.com/en-us/azure/cdn/cdn-manage-expiration-of-blob-content#setting-cache-control-headers-by-using-azure-powershell>`_. | ||
|
|
There was a problem hiding this comment.
IDK what Azure's default is, but if it's not too hard of an info to get, I suggest including it.
docs/source/teams/installation.rst
Outdated
| If your datasets include cloud-backed media, we recommend configuring your data | ||
| sources to allow for built in browser caching. This will cache signed URL responses |
There was a problem hiding this comment.
I'd go as far as imploring them to do it 😄
Maybe ...we strongly recommend...?
There was a problem hiding this comment.
+1 - for the other comments I can include the default but I feel like in general we should just tell them to do it instead of hoping that the browser or storage solution handles it properly. Like Stuart mentioned on the ticket some of these "defaults" change based on how you've setup your storage permissions so it's possible our default is different from theirs just based on how we have configured our data to begin with. I'm afraid to say (as an example) "S3 doesn't provide cache headers by default" but if their bucket has X configuration it starts some default value.
There was a problem hiding this comment.
Valid. I'm not aware of anything you can configure bucket-level that affects that particular metadata in either S3 or GCP - which is why I insisted on including it - but I defer to you - what you're saying makes sense.
There was a problem hiding this comment.
I was looking around online and seems like GCP is the only one providing a default value (added these in a commit) but probably best to also strongly recommend them to configure it for themselves instead of taking the gamble.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- docs/source/teams/installation.rst (5 hunks)
Files skipped from review as they are similar to previous changes (1)
- docs/source/teams/installation.rst
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- docs/source/teams/installation.rst (5 hunks)
Files skipped from review as they are similar to previous changes (1)
- docs/source/teams/installation.rst
|
LGTM! 🎸 |
|
Be sure the docs formatting looks correct before merging. Link here 😄 |
|
@benjaminpkane downloaded and reviewed locally it looks correct merging now! |
What changes are proposed in this pull request?
Updating documentation to inform customers about how to enable browser caching for our supported cloud media types.
https://voxel51.atlassian.net/browse/TEAMS-3180
How is this patch tested? If it is not, please explain why.
n/a
Release Notes
Is this a user-facing change that should be mentioned in the release notes?
notes for FiftyOne users.
(Details in 1-2 sentences. You can just refer to another PR with a description
if this PR is part of a larger change.)
What areas of FiftyOne does this PR affect?
fiftyonePython library changesSummary by CodeRabbit