Page MenuHomePhabricator
Create Task
Maniphest T197792

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

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.

Details

SubjectRepoBranchLines +/-
Pass through Parsoid's "Vary" and "Content-Language" headersmediawiki/services/mobileappsmaster+93 -5
Customize query in gerrit

Related Objects
Search...

Event Timeline

• Pchelolo triaged this task as Medium priority.Jun 20 2018, 12:36 PM
• Pchelolo created this task.
Restricted Application added a project: Product-Infrastructure-Team-Backlog-Deprecated. · View Herald TranscriptJun 20 2018, 12:36 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
• Pchelolo updated the task description. (Show Details)Jun 20 2018, 2:54 PM
• bearND moved this task from Needs triage to Kanban on the Product-Infrastructure-Team-Backlog-Deprecated board.Jun 20 2018, 3:32 PM
• bearND edited projects, added Product-Infrastructure-Team-Backlog-Deprecated (Kanban); removed Product-Infrastructure-Team-Backlog-Deprecated.
Jdforrester-WMF claimed this task.Jun 20 2018, 8:06 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/...

gerritbot added a comment.Jun 20 2018, 11:19 PM

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

gerritbot added a project: Patch-For-Review.Jun 20 2018, 11:19 PM
cscott mentioned this in T197702: Parsoid should not set Vary: Content-Type.Jun 21 2018, 8:08 PM
• 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 Product-Infrastructure-Team-Backlog-Deprecated (Kanban) board.
Jdforrester-WMF moved this task from Doing to Code Review on the Product-Infrastructure-Team-Backlog-Deprecated (Kanban) board.Jun 25 2018, 5:42 PM
Jdforrester-WMF mentioned this in T198134: Improve per-endpoint response header testing in MCS.Jun 25 2018, 6:41 PM
gerritbot added a comment.Jun 25 2018, 6:41 PM

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

Jdforrester-WMF moved this task from Code Review to To Deploy on the Product-Infrastructure-Team-Backlog-Deprecated (Kanban) board.Jun 25 2018, 6:50 PM
• Mholloway moved this task from To Deploy to Sign off on the Product-Infrastructure-Team-Backlog-Deprecated (Kanban) board.Jun 25 2018, 9:06 PM
Jdforrester-WMF removed a project: Patch-For-Review.Jun 27 2018, 4:15 PM
• Jhernandez subscribed.Jun 29 2018, 5:13 PM

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 Medium to High.
• Vvjjkkii added projects: CheckUser, Connected-Open-Heritage-Batch-uploads (RAÄ-KMB_1_2017-02), Tamil-Sites, Gamepress, Hashtags, Jade, KartoEditor, Language-2018-Apr-June, New-Editor-Experiences, Mail, TCB-Team (now WMDE-TechWish).
• 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 removed projects: TCB-Team (now WMDE-TechWish), Mail, New-Editor-Experiences, Language-2018-Apr-June, KartoEditor, Jade, Hashtags, Gamepress, Tamil-Sites, Connected-Open-Heritage-Batch-uploads (RAÄ-KMB_1_2017-02), CheckUser.
• bearND updated the task description. (Show Details)
• bearND added a subscriber: Aklapper.
• Pchelolo added a comment.Jul 2 2018, 7:39 AM

@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.

phuedx mentioned this in T188164: Popups don‘t support language variant conversion and {{lang}} template.Jul 2 2018, 9:07 AM
• Jhernandez added a comment.Jul 5 2018, 7:10 PM

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

• Pchelolo added a comment.Jul 10 2018, 9:53 PM
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
Jdforrester-WMF added a comment.Jul 10 2018, 10:01 PM

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.

• Pchelolo added a comment.Jul 10 2018, 10:05 PM

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

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL