[00:00:49] so, i have a patch for the page previews repo where i was thinking that all i needed to do was fix the indexing error but this won't actually work because https://upload.wikimedia.org/wikipedia/commons/a/aa/512px-Red_Giant_Earth_warm.jpg doesn't exist [00:01:16] niedzielski: what's happening is that the mw api is giving us the original URL because the original (https://commons.wikimedia.org/wiki/File:Red_Giant_Earth_warm.jpg) is smaller than the thumb size we're requesting [00:01:29] (1024px) [00:01:38] bearND: hehe ye it's hard to track where I am :) [00:01:44] i'm about to deploy a fix to drop that to 320px for the summary endpoint, which seems much more reasonable [00:02:03] and in practice should fix a lot of the problem cases [00:02:07] niedzielski: For most cases the MCS fix should do it because we would be requesting a much smaller thumbnail size (320px). The client side concern is mostly for cases where the original image is smaller than that. [00:02:09] (but not all) [00:03:58] niedzielski: if the `px-` string is not present (=`indexOf()` returns -1) then is should not try to add any `px-` inside the URL string. [00:04:19] mdholloway: bearND well i'm certainly grateful fast fix! i don't think there's much i can do in the client though except default to the original without kind of hacking something together that manually adjusts the url. in a future patchset, maybe it would be best if mcs requested a 0px width image so a thumbnail compatible url was nearly guaranteed [00:04:56] bearND: mdholloway sure i can do that [00:05:03] I thnk ideally we want a that thumbnail API [00:06:10] for sure [00:07:21] it's probably worth revisiting some of that logic next time we get some time to pay down tech debt... when that was written we weren't getting the original image size, so it was only safe to rewrite thumb size URL segments downward, not upward [00:07:25] mdholloway: would you ping P.chelolo once the deploy is done? I gotta run to an appointment in a bit. [00:07:33] now we're getting the original size, so we know the ceiling [00:07:38] bearND: sure [00:08:07] ty [00:08:09] (we get a 404 if we try to rewrite a thumb url for a size > the original size) [00:08:39] so maybe that 0px trick would work. worth playing with. [00:09:22] we also haven't been doing much improvement work in this area because of the promised thumb API that's coming along at some point, as bearND mentioned, which will supersede all of this [00:10:29] makes sense [00:11:40] mdholloway: i was just reading over the weekend about how wikipedia zero is going away and immediately thought of all that code we refactored back in the day! now that code is going to disappear and i can imagine some of this thumbnail code going away soon too [00:12:11] niedzielski: oh man, i'd actually forgotten how gnarly some of that code was! :) [00:12:54] * niedzielski it feels good to forget! :D [00:24:23] Pchelolo: MCS is deployed (with the version bump to 1.3.1). [00:28:10] ok guys, lemme start cs dump [00:28:47] brb, restarting [00:29:07] mdholloway: ty. Looking good so far. [00:29:11] https://www.irccloud.com/pastebin/Cc3uJCKe/ [00:30:52] strangely enough the thumbnail in this one actually has a 230px wide thumbnail (with the `px-`) [00:31:23] ok cswiki dump is on [00:32:48] bearND: it's because PageImages doesn't actually allow requesting specifically by width [00:33:03] it actually just uses the larger dimension [00:33:46] i put up a patch to fix that one time but did not succeed in convincing reviewers it was necessary [00:34:10] probably partly because the thumb api is coming [00:42:48] i guess it works out in our favor in the example above, since the original's width is < 320 [00:44:50] ok, good [00:45:12] Pchelolo: I guess you could start the enwiki dump before you go to bed [00:46:03] bearND: not sure if we're ok running 2 of them in parallel, I'll wait for the cs one to finish, last time it was fairly quick [01:08:25] everything seems good here. i'm off to buy some groceries. thanks Pchelolo for running the dumps! [01:38:28] fyi cswiki is done, I've started enwiki overnight [02:39:36] bearND|afk: i'm actually a bit surprised this didn't show up even with RESTBase summaries. the request is the same there, just with a pithumbsize of 320 (which we now do, too) [02:40:09] i guess that's a good sign that dropping to 320 should hopefully fix most cases [16:34:55] o/ bearND: olliv: hi all, i'm back from the dentist! i see T187955 is triaged as UBN but i'm not sure what's left to be done, exactly. i saw there were some apparent issues that turned out to be caused by bad content in the browser cache after last night's patch. [16:34:55] T187955: Page preview shows icon instead of thumbnail - https://phabricator.wikimedia.org/T187955 [16:35:27] basically, can i help unbreaking whatever still needs unbreaking? [16:35:47] sorry if i'm missing context, there's been lots of activity i'm still catching up on [16:37:05] * mdholloway goes back to reviewing patches for tgr|away in the meantime [18:58:16] mdholloway: sorry, that's ubn! from our side (merging stephen's change) [18:58:53] phuedx: makes sense, thanks! [19:23:44] jdlrobson: I'm already on the mixin, have spoken to Jan before [19:24:03] we use hyphens now in three places and will prob not end there