[02:16:08] 10Multimedia, 10Commons, 10MediaWiki-Page-deletion: Unable to delete file pages on commons: MWException/LocalFileLockError: "Could not acquire lock" - https://phabricator.wikimedia.org/T132921#2234762 (10Krinkle) [14:07:05] 10Multimedia, 10MediaWiki-extensions-MultimediaViewer: Could not load specific image - https://phabricator.wikimedia.org/T172230#3509352 (10Aklapper) 05Open>03stalled [14:09:11] gilles: So I'm looking at the state of Thumbor/3d2png now, and I have to say I'm feeling pulled in multiple directions. The triangle count check is a good idea, but it sounds like it's impossible in the lambda...and you seem to be nixing that in favor of a Swift hack? [14:10:23] marktraceur: you can do the triangle check, but it won't work for the custom loader we have (swift), only the stock file loader that the tests you've written use [14:10:39] the hack I've described is necessary to make it work with our custom loaders [14:11:10] essentially, our custom loaders don't pass the entire file in memory, because we deal with giant files [14:11:25] Oh, but the default loader does? [14:11:27] and it's impossible to pass context from the request to the mime check, only the file's buffer [14:11:31] I may have misunderstood your emails. [14:11:37] yes the default loader loads the entire file in memory [14:11:48] because most thumbor installs don't deal with giant files like we do [14:12:20] in our case when it's loaded from swift we stream it to a temp file and we pass only the beginning of the file to the mime check [14:12:53] that buffer is the only thing we can use for context [14:13:18] gilles: OK, so there's a hacky solution that we front-load, and backup solutions that should only happen in the case where we're not using our loader. What about the xvfb/xfra/suid discussion on the patch? Looks like you and Filippo disagreed on that [14:13:49] let's keep using xvfb-run, until filippo shows that we can't [14:14:00] but he had a valid concern about the display number [14:14:11] we probably want to use one specific to the process [14:14:20] using the pid as the display number for example [14:14:41] otherwise multiple thumbor processes would be sharing the same virtual screen` and we could run into race conditions [14:15:22] so don't use 99, but something unique to the process and you'll need an extra parameter for xvfb-run, because it uses display 99 by default [14:15:57] OK. [14:16:04] in fact the corresponding mediawiki code probably has the same problem [14:16:58] Fantastic [14:17:27] as for how to cheat from the swift loader, we can also make it look like a text STL (start with "solid", right?) this way the engine remains hack-free [14:17:46] the actual temp file will be used for the conversion anyway [14:17:59] the buffer passed to the mime check is only used for the mime check [14:19:32] thumbor itself is hard to modify to support passing more context than the buffer to the mime check. because why do you need more context when you have the entire file? [14:20:19] except in our case we don't. I should write about this, it's one of the few structural issues we've hit in thumbor, and it comes from the fundamental greater variety of files we deal with [14:21:30] gilles: That sounds like it will work. I'm glad I didn't do the full syntactic check (which includes, at the end of the file, "endsolid X") [14:22:33] yeah I mean, the point to verify that the file is correct. it's not a good predictor of whether the file will convert properly, regardless of which file format you're dealing with [14:22:43] it's more like "should we throw this at engine A or B?" [14:23:07] the point *isn't* [14:23:46] we convert a ton of files through IM that are invalid per their format's spec. the user doesn't care [14:24:51] but we can't make the STL engine the catch-all for anything we don't recognize either [14:25:12] Right. [14:48:04] 10Multimedia, 10ImageTweaks, 10Beta-Feature: Provide a beta feature of ImageTweaks - https://phabricator.wikimedia.org/T141317#3509507 (10Gilles) [14:48:07] 10Multimedia, 10ImageTweaks, 10Performance-Team, 10Thumbor, 10Beta-Feature: Expose Thumbor internally for ImageTweaks and similar use cases - https://phabricator.wikimedia.org/T151339#3509505 (10Gilles) 05Open>03declined Now that the project is finished, I think that the thumbor instances we have a h... [16:06:20] 10Multimedia, 103d, 10Design, 10Patch-For-Review: Improve 3D material - https://phabricator.wikimedia.org/T167748#3509740 (10ABorbaWMF) This looks good, but the thumbnails are still purple. I think that is cover here: https://phabricator.wikimedia.org/T170444 [16:22:26] 10Multimedia, 103d, 10Design, 10Patch-For-Review: Improve 3D material - https://phabricator.wikimedia.org/T167748#3343129 (10MarkTraceur) @ABorbaWMF negative, the 3d2png changes aren't deployed yet. I think any thumbnails currently present on beta are from old code, and cannot be trusted. T170444 will allo... [16:44:19] gilles: I couldn't name it 3D2PNG_PATH because variables can't start with numbers [16:44:25] I'll get the rest of them though. [16:44:36] THREED, then [16:46:07] Okie dokie [17:10:50] gilles: It looks like most of the commands don't use temporary files, just STDOUT...any reason to think that won't work here? [17:11:47] We'd need to expand 3d2png.js to support that. and currently the libraries output some log messages, not sure if they're in stdout or stderr though [17:12:03] Probably not worth it then. [17:13:40] the vips engine uses a temp file if you're looking for an example [17:13:46] cleanup is important as well [17:20:03] Oh, I'll double check that. [17:21:23] I think I went with a slightly more straightforward tempfile strategy but it all looks good [17:25:46] marktraceur: I think you missed https://phabricator.wikimedia.org/D732#14646 [17:26:00] Yeah, I was too quick [17:26:04] Fixing that now. [17:26:41] 10Multimedia, 103d, 10Design, 10Patch-For-Review: Separate View / Download action for 3d Filepage - https://phabricator.wikimedia.org/T167730#3509927 (10ABorbaWMF) Yes, I reproduced that on Chrome and Safari on Mac. I'm not sure how I missed that one. If you use the open mediaviewer button and then close (... [17:33:33] All caught up! [17:33:40] marktraceur: casing not fixed, I believe git can be annoying with case changes [17:33:51] you tried to rename the file, right? [17:34:42] Ugh, trying again. [17:35:06] gilles: I think it went through but I forgot one instance. [17:35:38] I wish my Vagrant would work and I could test this before flinging it at you [17:35:53] Not the ideal working environment on any level [17:47:53] marktraceur: just put it on deployment-imagescaler01, I pasted the commands [17:48:19] you can scp your changes there and test it before putting it up for review [18:29:18] gilles: The temp file shouldn't be extensionless...I passed a suffix of .stl in and that seems to work locally...is Thumbor creating another temp file somewhere? [18:30:53] you got that backwards, the one you've suffixed stl is the output [18:30:56] it's a png, in fact [18:31:04] Oh, hm. [18:31:05] the source temp file is managed by the thumbor plugins [18:31:19] Well, that'll fail anyway #brainfart [18:31:52] Oh, no, but...still pretty confusing, so I'll fix it and then submit a patch for 3d2png [19:28:35] marktraceur: have you patched 3d2png? [19:29:26] gilles: There's a patch up, yes [19:33:17] marktraceur: I don't have +2 on that repo [19:34:20] also once merged the new version will need to be deployed to deployment-imagescaler01 and I don't know how that's done (I doubt that it's automatic, is it?) [19:35:43] gilles: Seems unlikely. I may need to wait until later in the week to figure that out. [19:44:39] at least I can test on vagrant in the meantime, then [20:19:02] marktraceur: with the fixes I've just asked for, applied locally to my vagrant :) https://www.dropbox.com/s/kpofnx9lvzpavjn/Capture%20d%27%C3%A9cran%202017-08-08%2016.18.27.png?dl=0 [20:19:39] Hooray [20:20:19] gilles: Hopefully that means we'll be clear to push this forward next week, to beta and maybe testwikis. [20:20:44] I'll make more changes tomorrow, I have some things to accomplish this afternoon [20:20:48] yeah, if you can get the new 3d2png version deployed, it should be easy [20:20:58] 10Multimedia, 103d, 10Design, 10Patch-For-Review: Separate View / Download action for 3d Filepage - https://phabricator.wikimedia.org/T167730#3510403 (10dr0ptp4kt) Thanks, @ABorbaWMF. @MarkTraceur, moving this to "Next up" for fix on this same task for the repeat "eye" / raster click and Media Viewer X clo... [20:49:53] 10Multimedia, 10Beta-Cluster-Infrastructure, 10Thumbor: On beta commons, thumbnailing of 3D files is broken still - https://phabricator.wikimedia.org/T170444#3431671 (10ksmith) So is this waiting for {T161719} or is there something that needs to happen here? If so, by who? [21:33:51] (03PS1) 10Umherirrender: Improve some parameter docs [extensions/CommonsMetadata] - 10https://gerrit.wikimedia.org/r/370753