Add content negotiation filter #1052
Conversation
Pchelolo
requested review from
d00rman and
jdforrester
and removed request for
jdforrester
Pchelolo
force-pushed
the
content_negotiation
branch
from
368856c
to
cfd53bc
Compare
Pchelolo
changed the title
Pchelolo
force-pushed
the
content_negotiation
branch
from
782f5dd
to
992a19b
Compare
Change-Id: Iaa3bcee0b2f6b5b59b8b8716e861e76fc405137f
Change-Id: I8d6e0bc299ff2aeeb3e0e3d218fe766a2bbeefff
Change-Id: I9ebbeab5bc3e2180b59f9a13f735a39171b20a63
Pchelolo
force-pushed
the
content_negotiation
branch
from
992a19b
to
fe69a68
Compare
|
This is completely completed, was tested in Vagrant and seems to work OK. Next step is merging it and deploying to Labs to test VE some more. After - coordinated deploy together with @wikimedia/parsing as it's the first time we do anything like this and a lot of caution is needed. |
| return htmlRes; | ||
| } | ||
|
|
||
| if (semver.satisfies(storedVersion, requestedVersion)) { |
There was a problem hiding this comment.
This might be clearer as eq since the requestedVersion isn't a range.
But maybe you meant to use a partial or x-range (to ignore the patch)?
There was a problem hiding this comment.
We ignore the patch both in Varnish (by basically resetting the patch to 0) and in RESTBase
There was a problem hiding this comment.
You're saying that storedVersion will also always have a sanitized patch level? In which case, use eq. If not, you probably want to express requestVersion as a range using partial or x-range.
There was a problem hiding this comment.
Right.. will make another pr
Implements the protocol we've all agreed on in https://phabricator.wikimedia.org/T128040#4528109
Now we're supplying dummy
data-parsoidto Parsoid to bypass validation, but @arlolra promised to fix that in Parsoid.cc @wikimedia/services @wikimedia/parsing