Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add immutable directive to Cache-Control header #787

Merged
merged 3 commits into from
Dec 4, 2020

Conversation

lpsmith
Copy link
Member

@lpsmith lpsmith commented Jul 10, 2020

When serving up a Content-Addressed static asset, include immutable on the cache-control header

However, I did notice that when serving up a asset that is Content-Addressed and redirects are disabled, we are disabling all caching. We could modify Obelisk.Asset.Serve.Snap.serveAsset' to be less conservative and more precise on this regard: in doing so, we'd need to remember the whether or not the first call to serveAsset' before it recurses is an immutable or a redirect, and then use that instead of doRedirect on line 94.

I have:

  • Based work on latest develop branch
  • Looked for lint in my changes with hlint . (lint found code you did not write can be left alone)
  • Run the test suite: $(nix-build -A selftest --no-out-link)
  • (Optional) Run CI tests locally: nix-build release.nix -A build.x86_64-linux --no-out-link (or x86_64-darwin on macOS)

@lpsmith lpsmith changed the base branch from master to develop July 10, 2020 16:32
@3noch
Copy link
Collaborator

3noch commented Jul 10, 2020

and redirects are disabled

How does this happen?

@lpsmith
Copy link
Member Author

lpsmith commented Jul 10, 2020

and redirects are disabled

How does this happen?

By calling either serveAssetsInPlace or serveAssetInPlace instead of serveAsset(s). I didn't look at how those particular routines are used (or not) in typical codebases, though.

@ali-abrar ali-abrar merged commit 354795d into develop Dec 4, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants