Skip to content

Add better caching instruction to headers#3000

Merged
chrisknoll merged 1 commit intoOHDSI:masterfrom
uc-cdis:feat/better_cache_instructions_in_nginx
Apr 22, 2025
Merged

Add better caching instruction to headers#3000
chrisknoll merged 1 commit intoOHDSI:masterfrom
uc-cdis:feat/better_cache_instructions_in_nginx

Conversation

@pieterlukasse
Copy link
Contributor

Addresses #2999

@pieterlukasse pieterlukasse changed the title Feat: add better caching instruction to headers Add better caching instruction to headers Mar 19, 2025
@pieterlukasse
Copy link
Contributor Author

@anthonysena any interest in this?

@chrisknoll
Copy link
Collaborator

I'm good with the idea of adding cache headers, anything that reduces bandwidth and speeds up load times.

One thing I was looking into several years back was adding 'etag' and other HTTP headers. I thought this was a function of our IIS servers that serve HTML, but also could be related to REST requests from WebAPI.

I took a quick peek at the cache header your PR suggests, but I don't know enough about that particular one. Can you summarize what the intended result of including those headers would be? (ie: which requests would be impacted is it javascript, html, images, or JSON from WebAPI?)

@pieterlukasse
Copy link
Contributor Author

@chrisknoll the idea of this change is to ensure clients always get the latest version of a resource, which is crucial when you're deploying updates frequently.
From our testing / experience, this change seems to have solved our problems. So I thought I'd share it back upstream.
Here is some documentation on no-cache and must-revalidate:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Cache-Control
The naming is confusing. From the docs:

Note that no-cache does not mean "don't cache". no-cache allows caches to store a response but requires them to revalidate it before reuse. If the sense of "don't cache" that you want is actually "don't store", then no-store is the directive to use.

@chrisknoll
Copy link
Collaborator

Ok! I'll give it a shot. thanks for the explanation.

@chrisknoll chrisknoll merged commit 061e9c3 into OHDSI:master Apr 22, 2025
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.

2 participants