Page MenuHomePhabricator

MCS should proxy Vary and Content-Language headers header it gets from Parsoid
Closed, ResolvedPublic

Description

MCS should return the same vary header as it got when requesting Parsoid HTML.

For language variants support Parsoid will be the ultimate source of truth regarding whether a certain page can or can not be affected by language variant, so for those pages that can possibly be transformed, Parsoid will set vary: accept-language in its response.

RESTBase will proxy and store that for HTML content, but for MCS RESTBase doesn't want to also fetch in Parsoid HTML in order to determine whether to set the header.

So MCS, when requesting HTML content for transformation should just proxy and return the Vary header it got together with HTML.

Event Timeline

Pchelolo triaged this task as Normal priority.Jun 20 2018, 12:36 PM
Pchelolo created this task.
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Pchelolo updated the task description. (Show Details)Jun 20 2018, 2:54 PM
bearND added a comment.EditedJun 20 2018, 9:08 PM

Here are some test request for the Parsoid case (beta cluster only for now)

curl -sI -H 'Accept-Language: sr-el' https://sr.wikipedia.beta.wmflabs.org/api/rest_v1/page/html/RESTBase_Testing_Page | grep -i vary
Vary: accept-language, Accept-Encoding

OR:

curl -sI -H 'Accept-Language: sr-ec' https://sr.wikipedia.beta.wmflabs.org/api/rest_v1/page/html/RESTBase_Testing_Page | grep -i vary

And here for a few MCS endpoints:

... similar for metadata/media/references/...

Change 441320 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[mediawiki/services/mobileapps@master] [WIP] Pass through Parsoid's "Vary" header

https://gerrit.wikimedia.org/r/441320

bearND renamed this task from MCS should proxy Vary header it gets from Parsoid to MCS should proxy Vary and Content-Language headers header it gets from Parsoid.Jun 22 2018, 5:45 PM
bearND moved this task from To Do to Doing on the Reading-Infrastructure-Team-Backlog (Kanban) board.

Change 441320 merged by jenkins-bot:
[mediawiki/services/mobileapps@master] Pass through Parsoid's "Vary" and "Content-Language" headers

https://gerrit.wikimedia.org/r/441320

Is this deployed to the beta cluster already?

I'm trying the test cases and they don't seem to be working:

~ => curl -sI -H 'Accept-Language: sr-ec' https://sr.wikipedia.beta.wmflabs.org/api/rest_v1/page/html/RESTBase_Testing_Page | grep -i vary
vary: accept-language, Accept-Encoding
# ☝️ works fine with restbase

# 👇 doesn't seem to work with mcs endpoints
~ => function test() {
> curl -sI -H 'Accept-Language: sr-ec' $1 | grep -i vary
> }
~ => test https://sr.wikipedia.beta.wmflabs.org/v1/page/summary/RESTBase_Testing_Page
vary: Accept-Encoding
~ => test https://sr.wikipedia.beta.wmflabs.org/v1/page/mobile-sections/RESTBase_Testing_Page
vary: Accept-Encoding
~ => test https://sr.wikipedia.beta.wmflabs.org/v1/page/content-html/RESTBase_Testing_Page
vary: Accept-Encoding

Let me know if I'm doing something stupid.

bearND added a comment.EditedJun 30 2018, 1:30 AM

@Jhernandez The MCS URI is missing api/rest_. It was returning a 404 earlier. Here's the full one:
https://sr.wikipedia.beta.wmflabs.org/api/rest_v1/page/summary/RESTBase_Testing_Page

Still it doesn't seem to work with the corrected URIs either.

function test() {
  curl -sI -H 'Accept-Language: sr-ec' $1 | grep -i lang
}

test https://sr.wikipedia.beta.wmflabs.org/api/rest_v1/page/summary/RESTBase_Testing_Page
access-control-allow-headers: accept, content-type, content-length, cache-control, accept-language, api-user-agent, if-match, if-modified-since, if-none-match, dnt, accept-encoding

My local MCS instance does have it, though:

test http://0.0.0.0:6927/sr.wikipedia.beta.wmflabs.org/v1/page/summary/RESTBase_Testing_Page
Vary: accept-language, Accept-Encoding
Content-Language: sr-ec

I think the language response headers are missing from the RESTBase requests because https://github.com/wikimedia/restbase/pull/1010 is not merged/deployed to beta cluster yet.

Vvjjkkii renamed this task from MCS should proxy Vary and Content-Language headers header it gets from Parsoid to 3kaaaaaaaa.Jul 1 2018, 1:02 AM
Vvjjkkii removed Jdforrester-WMF as the assignee of this task.
Vvjjkkii raised the priority of this task from Normal to High.
Vvjjkkii updated the task description. (Show Details)
Vvjjkkii edited subscribers, added: Jdforrester-WMF; removed: Aklapper, gerritbot.
bearND renamed this task from 3kaaaaaaaa to MCS should proxy Vary and Content-Language headers header it gets from Parsoid.Jul 1 2018, 4:27 AM
bearND assigned this task to Jdforrester-WMF.
bearND raised the priority of this task from High to Needs Triage.
bearND updated the task description. (Show Details)
bearND added a subscriber: Aklapper.

@bearND the MCS part is deployed in beta nad works good:

curl -i https://appservice.wmflabs.org/sr.wikipedia.beta.wmflabs.org/v1/page/summary/RESTBase_Tsting_Page | grep vary

vary: accept-language, Accept-Encoding

But the RESTBase counterpart is not deployed yet even to beta, it lives in https://github.com/wikimedia/restbase/pull/1010

We're being very gradual with public rollout if this, so don't want to merge it in master yet.

Ok, it is clear that it works locally, but lets keep this open until we can sign off on it at least on beta cluster to check that all the pieces fit together properly. 👍

Jhernandez changed the task status from Open to Stalled.Jul 5 2018, 7:10 PM
Jhernandez triaged this task as High priority.

Stalling until it can be checked.

Pchelolo closed this task as Resolved.Jul 10 2018, 11:25 AM
Pchelolo edited projects, added Services (done); removed Services (blocked).

I believe this is all good now.

bearND reopened this task as Open.EditedJul 10 2018, 9:45 PM

Still don't see accept-language in the vary header.

curl -sI -H 'Accept-Language: sr-ec' https://sr.wikipedia.beta.wmflabs.org/api/rest_v1/page/summary/RESTBase_Testing_Page | grep -i vary
vary: Accept-Encoding

The Parsoid response has it:

curl -sI -H 'Accept-Language: sr-ec' https://sr.wikipedia.beta.wmflabs.org/api/rest_v1/page/html/RESTBase_Testing_Page | grep -i vary
vary: accept-language, Accept-Encoding

Also I don't see the content-language header in the summary response.

Pchelolo closed this task as Resolved.Jul 10 2018, 9:49 PM

That's RESTBase, in RB we indeed didn't deploy it, but for MCS all should be done here

curl -i -H 'Accept-Language: sr-ec' http://localhost:8888/sr.wikipedia.org/v1/page/summary/%D0%9D%D0%B8%D0%BA%D0%BE%D0%BB%D0%B0_%D0%A2%D0%B5%D1%81%D0%BB%D0%B0

Vary: accept-language, Accept-Encoding

Hmm, yes, in beta cluster and prod it's working for some routes but not others:

page/summary

curl -sI -H 'Accept-Language: sr-el' https://sr.wikipedia.org/api/rest_v1/page/summary/%D0%93%D0%BB%D0%B0%D0%B2%D0%BD%D0%B0_%D1%81%D1%82%D1%80%D0%B0%D0%BD%D0%B0 | grep -i -E 'Vary|Content-Language'
vary: Accept-Encoding

page/metadata (non-variant Content-Language specified)

curl -sI -H 'Accept-Language: sr-el' https://sr.wikipedia.org/api/rest_v1/page/metadata/%D0%93%D0%BB%D0%B0%D0%B2%D0%BD%D0%B0_%D1%81%D1%82%D1%80%D0%B0%D0%BD%D0%B0 | grep -i -E 'Vary|Content-Language'
vary: accept-language, Accept-Encoding
content-language: sr

page/references (non-variant Content-Language specified)

curl -sI -H 'Accept-Language: sr-el' https://sr.wikipedia.org/api/rest_v1/page/references/%D0%93%D0%BB%D0%B0%D0%B2%D0%BD%D0%B0_%D1%81%D1%82%D1%80%D0%B0%D0%BD%D0%B0 | grep -i -E 'Vary|Content-Language'
vary: accept-language, Accept-Encoding
content-language: sr

page/media (non-variant Content-Language specified)

curl -sI -H 'Accept-Language: sr-el' https://sr.wikipedia.org/api/rest_v1/page/media/%D0%93%D0%BB%D0%B0%D0%B2%D0%BD%D0%B0_%D1%81%D1%82%D1%80%D0%B0%D0%BD%D0%B0 | grep -i -E 'Vary|Content-Language'
vary: accept-language, Accept-Encoding
content-language: sr

page/html

curl -sI -H 'Accept-Language: sr-el' https://sr.wikipedia.org/api/rest_v1/page/html/%D0%93%D0%BB%D0%B0%D0%B2%D0%BD%D0%B0_%D1%81%D1%82%D1%80%D0%B0%D0%BD%D0%B0 | grep -i -E 'Vary|Content-Language'
content-language: sr-el
vary: accept-language, Accept-Encoding

Sadly definition isn't really testable.

This ticket is for MCS and MCS only! In MCS it works fabulously. Those URIs are for RESTBase - that will not work until https://github.com/wikimedia/restbase/pull/1010 is deployed, which depends on Mediawiki train to prepare page previews for this.

For summary, we have a special ticket: T198465