Support language variants for summary and mobile content #1010
Conversation
Pchelolo
changed the title
There was a problem hiding this comment.
Neatly done!
Now that we implement no-store on the bucket level, we also need to normalise the cache-control header for all incoming requests to RESTBase, though, as we don't want that a client sends it to us. Basically something like:
if (mwUtil.isNoCacheRequest(req)) {
req.headers['cache-request'] = 'no-cache';
} else {
delete req.headers['cache-request'];
}| @@ -145,15 +145,16 @@ paths: | |||
| request: | |||
| method: put | |||
| uri: /{domain}/sys/key_value/page_summary/{request.params.title} | |||
| headers: '{{merge({"if-none-hash-match": "*"}, extract.headers)}}' | |||
| headers: '{{merge({"if-none-hash-match": "*"}, extract.headers, "cache-control": request.headers.cache-control)}}' | |||
There was a problem hiding this comment.
Why is the request's cache-control being added here and below?
There was a problem hiding this comment.
@d00rman to pass through the no-store header.
Pchelolo
force-pushed
the
variants_for_everything
branch
from
70b6821
to
aef1901
Compare
Pchelolo
force-pushed
the
variants_for_everything
branch
from
e19656a
to
23e8698
Compare
|
Doesn't Varnish strip those headers? |
Change-Id: I4901a437444f3805bbca82df1b1a3c66c41a3d96
Change-Id: Iff004aefb21f43bc530a0ad2cf658baddebf5011
Change-Id: Ibca5c9c654a568348454aea6f1bed8aa10f087a4
Change-Id: Ibde9d24247ba202f64d6bb69ce7b08ad791b40e0
Change-Id: I1bb5fbf6edfb9524c6969efe23cde766be2c39e2
Pchelolo
force-pushed
the
variants_for_everything
branch
from
23e8698
to
193229d
Compare
|
Rebased |
|
All the dependencies were deployed, so we're ready to go fowrward with this. |
Yes, Varnish does, but what about internal clients? Better safe than sorry. EDIT: Also, what about installs that don't count on Varnish being there? The smaller the number of assumptions, the better IMHO |
v1/summary.yaml
Outdated
| catch: | ||
| status: 412 | ||
| status: [ 412, 202 ] |
There was a problem hiding this comment.
Nit: let's sort these numbers.
v1/summary.yaml
Outdated
| return_if: | ||
| status: 412 | ||
| status: [ 412, 202 ] |
Change-Id: I4c1e78be374de1f0723d870f09ca1cb98ee7a156
lib/normalize_headers_filter.js
Outdated
| module.exports = function normalizeHeaders(hyper, req, next) { | ||
| if (mwUtil.isNoCacheRequest(req)) { | ||
| req.headers['cache-control'] = 'no-cache'; | ||
| } else { |
There was a problem hiding this comment.
Nit: else if (req.headers && req.headers['cache-control']) (I wonder if this is needed, i.e. is there somewhere where we ensure the existence of the headers hash?)
Change-Id: I5de0e5b81521f64acee7bc2cab0fe4db2773ef44
Pchelolo
force-pushed
the
variants_for_everything
branch
from
3a72534
to
9d9ff96
Compare
cc @wikimedia/services @berndsi