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

Set Cache-Control: no-cache if cache buster check fails #28

Merged
merged 1 commit into from
Oct 2, 2023

Conversation

robertknight
Copy link
Member

@robertknight robertknight commented Oct 2, 2023

If an asset URL fetch fails due to version skew across multiple servers in a deployment, prevent the 4xx response from being cached by the CDN or browser.

The proper solution is for the downstream app to either deploy assets ahead of the app deployment, or use sticky sessions. This change reduces the downside if eg. stickiness fails for any reason.

Fixes #27


Testing:

  1. Check out this branch and install the local h-assets package in h (tox -e dev --run-command "pip install -e /path/to/h-assets")
  2. Get a valid asset URL, including the query string, from the HTML of http://localhost:5000/search
  3. Fetch that URL. It should return a 200 response with no Cache-Control header
  4. Change the query param to be different and fetch again. It should return a 404 response with the Cache-Control: no-cache header set

If an asset URL fetch fails due to version skew across multiple servers in a
deployment, prevent the 4xx response from being cached by the CDN or browser.

The proper solution is for the downstream app to either deploy assets ahead of
the app deployment, or use sticky sessions. This change reduces the downside if
eg. stickiness fails for any reason.

Fixes #27
Copy link

@acelaya acelaya left a comment

Choose a reason for hiding this comment

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

LGTM. Also followed testing steps, and working as expected.

@robertknight robertknight merged commit ab33fb4 into main Oct 2, 2023
@robertknight robertknight deleted the prevent-404-caching branch October 2, 2023 13:49
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

Successfully merging this pull request may close these issues.

Prevent caching of failed responses
2 participants