[20:03:53] bearND: any change you would have some time to have a look into https://phabricator.wikimedia.org/T197792 ? We're trying to make variants work in beta till the end of the week so that we catch all the bugs and move all to prod in one chunk [20:05:20] James_F: might ^ be of interest to you since it's a related to the Accept-Language header work you did for Parsoid? [20:05:49] Oh, cool. [20:06:54] no super urgency there, but we want to move it all to prod as a big one chunk for serbian wiki, and that's one of the missing parts [20:07:43] btw, I've tested mcs `accept-language` forwarding in beta, seems to work correctly in semi-real environment [20:10:14] Pchelolo: James_F: ok, to get this in this week we only have until tomorrow morning for our backup MCS deploy window. [20:10:42] bearND: there's no prod deploys this week due to SRE offsite [20:10:47] Wait. We don't… yeah. [20:11:19] but we can deploy to beta whatever we want, and we're using this window to get everything for lang variants in order for beta [20:12:02] Oh, my bad. Forgot about the deploy stop, oops. Just tried to deploy MCS. [20:12:32] bearND: oh oh oh.... someone could be in trouble :))) [20:12:43] Still, if we want it in beta we need to get this merged, too. [20:12:59] Pchelolo: hehe, I had to roll it back anyways. :( [20:13:27] bearND: yeah. we're already past that point in RESTBase, we have a lot of code in master that's not in prod and only in beta.. [20:14:07] it would've been like that for lang variants anyway, it's a too big of a feature to get it correct right away [20:15:22] and for you it doesn't really matter, as long as we don't expose that mcs has changed regarding, for example, vary header, nobody will see that [20:20:16] btw, thanks James_F for getting mediawiki-title to modern state, I've never had time for getting to it.. [20:20:37] Pchelolo: Happy to help. :-) [20:21:30] text me when you think you're done with improvements, I'll release a new version [20:21:55] or I can just make you a maintainer on npmjs if you want and if you tell me your name there [20:22:56] Pchelolo: are RB/Parsoid already sending the Vary header? [20:23:11] bearND: in beta - yes [20:24:12] Pchelolo: any good example pages to test this? [20:24:41] bearND: curl -i -H 'Accept-Language: sr-ec' https://sr.wikipedia.beta.wmflabs.org/api/rest_v1/page/html/RESTBase_Testing_Page | grep vary [20:25:04] see how cyrillic changes to latin when you change sr-ec to sr-el [20:25:12] Ово је тестна страница - cyrillic [20:25:26] Ovo je testna stranica - latin [20:25:41] and then when you don't provide accept-language you have both [20:25:58] the Vary is only set if the page can possibly be affected by variants [20:26:07] so we don't split the cache without need [20:26:49] James_F: ^ [20:27:16] * James_F nods. [20:27:19] Pchelolo: Hmm, without the accept-language header I only see the cyrillic version [20:27:44] Never mind. The grep hid it [20:27:58] bearND: there's a list there https://sr.wikipedia.beta.wmflabs.org/api/rest_v1/page/html/RESTBase_Testing_Page one is cytillic and the second is latin [20:28:13] then with accept language both turn either cyrillic or latin [20:28:28] That's a nice test, hehe [20:28:44] Yeah. [20:28:53] cause the "natural" way in mediawiki is actually the mix of both [20:29:44] finally we will fix that so that people will not see mix of transcripts in the app [20:30:32] So, we get `Vary: accept-language, Accept-Encoding`. Should we be sending the Vary header along as is? [20:32:18] bearND: I think so ye [20:32:21] Does RB do anything with accept-encoding for normal content? [20:35:06] James_F: for the vary header it doesn't matter, it's for Varnish. normally clients will accept gzip so that's what varnish caches, but if client does not, it will be a cache miss and it will go to RB and it will give non-gzipped content [20:35:28] between backend services it doesn't really matter [20:36:15] Right. [20:36:29] you can just transfer whatever Parsoid gives and that's ok, Accept-Encoding in vary will be added by varnish anyway. [20:37:10] One note Parsoid now also provides Vary: content-type, but T197702 [20:37:10] T197702: Parsoid should not set Vary: Content-Type - https://phabricator.wikimedia.org/T197702 [20:37:45] Yeah, I'm just trying to work out how we'd actually do anything with a response request in the MCS codebase. :-) [20:38:04] Do you want us to whitelist acceptable (ha) Vary values? [20:38:53] by now - nope. lets RB/Parsoid be the source of truth on this [20:39:30] * James_F nods. [20:41:01] Pchelolo: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/251312 is going to be a headache if that ever lands. [20:43:04] James_F: hm... server-side - no, not really, at least for now as we don't store the variants... For the clients, especially apps, it will be a major painpoint [20:43:32] James_F: I'm not sure there is an as elegant solution as for there was for the request header since we don't pass along the response object to `getParsoidHtml()`, at least yet. So, either we start passing the response object to that one as well or add it to the callers in the respective `then()` block. [20:43:54] bearND: Yeah… bit of a breaking change either way. [20:44:42] E.g. for routes/page/summary.js somewhere starting line 30 or later, probably near 36/37. [20:45:38] James_F: is making a really good point btw, we never thought what to do if the variants get renamed. I will file a ticket to think about that. not a top priority, but we need to support some alias system there [20:46:34] Pchelolo: There's also the MediaWiki <-> reality issue (MediaWiki knows about zh-hans and zh-cn, but BCP47 says the latter should be zh-hans-cn etc.). [20:47:16] ye... zh-hans-cn will not even be properly supported by Varnish now.. [20:47:23] devil is in the details here :) [20:47:52] (Technically "zh-Hans-CN" for some reason.) [20:48:09] James_F: thankfully we lowcase everything :) [20:48:21] Pchelolo: Until we find a client that objects. :-) [20:50:07] hopefully nobody will differentiate zh-Hans-CN and zh-hans-cn as different variants [20:50:22] Yeah. [20:51:35] but you're rasing some very good points here, especially we need possibility to support old apps versions where sr-ec is hard-coded.. [20:52:33] or... make the apps fetch these constants from MW [20:52:51] Fetching from MW sounds messy. [20:53:12] Apps already have to translate from platform language to Web-world language. [20:53:45] hard-coding sounds real messy as well, imagine we add support for a new variants, like for Norwegian... how will the older app versions get to know about that [20:53:52] Pchelolo: The apps don't have sr-ec hard-coded, yet. [20:54:09] I guess you're talking about the future, right? [20:54:33] bearND: ye, but 2 weeks from now hopefully we will have all the infra set up for them to get proper support for this stuff [20:54:44] so Chineese ppl will be able to use the apps [20:55:32] For zhwiki the apps are using MW action=mobileview right now. So, technically they can already use the app, it's just not using RB. [20:55:43] For srwiki that might be another story. [20:56:07] There is not special handling in the apps for srwiki AFAIK [20:56:52] gotcha. Anyway, I've got ideas for a couple new tickets to think about, thank you [20:57:11] We used to have an In the news endpoint for zhwiki which we abandoned since RB doesn't handle variants. That one might be a good one to bring back/ [20:58:16] bearND: oh... you've just made me realize we need even more tickets for variants :) [21:03:11] James_F: since we probably want to pass along the Vary response header for pretty much all the things we ask for Parsoid content we might as well pass the response header there and have that done in `_getRestPageContent`: [21:03:19] https://www.irccloud.com/pastebin/tTmEiChu/ [21:03:54] Huh. Will that work? It seems to easy. :-) [21:04:35] James_F: You would still need to pass around the MCS response object. Hence me calling the above parsoidResponse as to not be too confusing. [21:10:33] bearND: hehe, I literally clicked your localhost links on the ticket several times before understanding why my browser doesn't show anything :) [21:14:01] bearND: my laptop will blow up if I run all of our stack constantly [21:14:25] and I will have not enough memory for my netflix!