[07:24:13] hi marktraceur [07:49:13] hi legoktm [07:49:16] around? [08:04:22] mayankmadan: We'll be more around tomorrow; today was a holiday [08:10:15] hi [08:13:30] legoktm, you free? [08:13:39] possibly, what's up? [08:15:04] legoktm, how can i get all the possible values of [08:15:05] _this.setStatus( mw.message( 'mwe-upwiz-' + result.upload.stage ).text() ); [08:15:19] this is in mw.UploadWizardDetails.js [08:15:21] the message key? [08:15:26] line 1341 [08:15:31] you would want to figure out what result.upload.stage could be [08:15:38] legoktm, exactly [08:16:02] legoktm, i know one possible key is mwe-upwiz-undefined when result.upload.stage is undefined [08:30:15] legoktm, around? [08:48:19] somewhat [08:49:51] legoktm, then how can i get the possible values of result.upload.stage ? [08:50:03] figure out where its set? check the API code? [09:05:33] legoktm, result is an argument in the function, just wanted to ask , what type it is? [09:06:26] I have no clue, really haven't looked at the code [11:45:10] legoktm, where should i look up? [12:17:54] tgr, a little help here? [13:56:29] (03CR) 10Mayankmadan: Adding code comments of resulting keys (031 comment) [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/104782 (owner: 10Mayankmadan) [15:23:13] mayankmadan__: are you looking for what result.upload.stage can be? [15:23:23] tgr, yes [15:23:52] you have to trace back the result object to its source [15:24:03] see where it comes from [15:25:24] tgr, well, its an argument [15:25:36] tgr, how do i do that? [15:26:13] you have to look through the code, check where the function is called, see what is used as argument [15:26:31] ok is a local function, so it must be used close to where it is defined [15:26:54] a good IDE can probably help you there, but it's doable by hand [15:28:16] tgr, that is the problem [15:28:21] tgr, it is not called [15:28:56] judging by the name, it is used as a success callback [15:29:27] so you pass ok to some asynchronous function, and that function calls ok back when it is finished [16:45:32] hi marktraceur [16:45:44] Hello! [16:45:49] OK, now I'm in and working and can help more. [16:45:56] I think you're getting some good advice already, though [16:46:03] marktraceur, great [16:46:05] Have you stepped through the places the function is called? [16:46:58] the function is only called inside setTimeout which is in that function only [16:47:15] mayankmadan__: With what arguments? [16:47:35] Oh, the same [16:47:39] marktraceur, nothing. it is used as a variable [16:47:42] mayankmadan__: Does the function get passed anywhere? [16:47:46] nope [16:47:49] Huh. [16:47:54] OK, now I'll look at it. [16:47:59] Because that seems unlikely [16:48:14] Where is it again? [16:48:37] marktraceur, line 1341 mw.UploadWizardDetails [16:49:28] marktraceur, line 1341 is the line to inquire [16:49:31] OK, and the ok function must get called somewhere, right? [16:50:07] In particular, it calls out to an API call, with the success function being OK [16:50:22] In particular it's using the upload API [16:50:31] So let's look at the documentation for the upload API [16:50:41] Luckily it's everywhere - http://commons.wikimedia.org/w/api.php [16:50:56] You can text search for action=upload [16:51:20] It points us at https://www.mediawiki.org/wiki/API:Upload [16:51:49] And the sample data there says one value is "Continue" [16:51:58] Now let's see if we can't find the API code in core [16:52:11] mayankmadan__: Have you looked at the core code much? [16:55:30] marktraceur, not mych [17:02:03] mayankmadan__: Well, get ready to start up :) let's look at the mediawiki core directory [17:02:23] ok [17:02:34] There are a lot of directories there, but here you just sort of have to know that "includes" holds almost all of the code [17:02:47] okay [17:02:51] So cd into includes [17:03:08] Here are a lot more files - but there's also a directory called "api" [17:03:16] That's what we want, so cd there [17:03:30] Now we have a bunch of similarly-named files that define API modules [17:03:37] yes [17:03:55] ApiUpload.php is the one we want, obviously, because we're looking at the Upload API [17:04:18] Now let's open that file and search for "result" [17:04:25] * chrismcmahon follows along... [17:04:32] I find a function called getContextResult that seems interesting [17:05:05] mayankmadan__: Can you tell me where we should look next, based on the contents of getContextResult? [17:05:34] wait a minute let me catch up [17:05:36] :) [17:05:43] having tea [17:06:37] Ah, kay [17:07:13] marktraceur, we should have a look at performUpload() [17:07:27] Anything else? [17:08:26] the result functions [17:08:29] Right [17:08:34] So let's do those first [17:08:45] If you text search for each of them (or scroll down, they're nearby) [17:08:51] You can see what they return as results [17:09:15] I'd start writing them down in a list, by the way [17:09:32] Separate from whatever code you're writing that uses them [17:09:43] marktraceur, they all are returning an array [17:10:55] mayankmadan__: What's the structure of that array? What members does it have? [17:13:27] marktraceur, sorry i disconnected [17:13:29] did you send any messages [17:13:31] mayankmadan: What's the structure of that array? What members does it have? [17:15:41] marktraceur, it looks like a 2 dimensional array with 'result', 'warnings', 'filekey' etc [17:15:50] as keys [17:15:53] Right [17:16:57] To me, that looks like the 'upload' array at https://www.mediawiki.org/wiki/API:Upload#Uploading_directly [17:17:14] Which means the 'result' member of the array is probably the result string you're looking for [17:19:15] marktraceur, result member is always a string [17:19:32] mayankmadan: Knowing that, you can more easily figure out the values from the rest of the functions. [17:20:09] So if you go through the first three, you should have Success, Warning, and Continue on your list [17:20:48] marktraceur, but i need values of result.upload.stage [17:20:55] ...hm [17:21:03] Sorry, I'm wrong then [17:21:09] marktraceur, but how can a string have an upload method [17:21:31] What? [17:22:20] marktraceur, if result is a string then how can it have a function named upload inside it [17:22:39] We're talking about two different variables [17:22:41] They aren't the same [17:22:44] Patience, padawan [17:22:58] So, luckily we're in the right file, and 'stage' is a much easier thing to search for [17:23:39] marktraceur, whats padawan? [17:25:45] Heh, nothing, it's a Star wars thing [17:26:04] mayankmadan: So searching through that file, I only find the word "queued" used in that array key [17:26:31] yep [17:26:55] So I'm making sure that's all I see, but I suspect there'll be another one somewhere [17:29:39] mayankmadan: This is strange, but I'm going to say "queued" is basically the only place [17:29:42] er, value [17:29:58] Ah, no! [17:30:23] mayankmadan: 'stage' gets updated, later, by the backend things responsible for assembling chunked uploads. [17:30:39] marktraceur: there are a bunch of values via the job queue i think [17:30:39] If you cd back to includes/ and do git grep stage, it shows you the relevant lines (and values!) [17:30:51] But I think the only value you ever see from the API is 'queued' [17:33:02] Especially since I don't see $result variables being set to anything but empty arrays, and I don't see them passed into any functions (much less by reference) [17:34:23] BRB [17:35:28] mayankmadan: So, based on what we've discovered here, I'd suggest you write an explanation of the upload.stage return values on [[mw:API:Upload]] [17:36:35] In particular that it's used mostly internally to signify what stage of uploading a chunked job is on [17:36:48] But that the API returns it for the initial chunk upload attempt [17:37:08] So really the only valid value of it from the API's perspective is 'queued', except when it's undefined [17:37:16] But undefined shouldn't mean anything [17:42:54] marktraceur, got it [17:43:10] marktraceur, so should i write it on mw:api:upload too ? [17:43:14] wiki page? [17:43:27] mayankmadan: Yes please! I would expect in this section: https://www.mediawiki.org/wiki/API:Upload#Chunked_uploading [17:43:53] marktraceur, i always wanted to edit an important document like that :) [17:44:04] We sure make it easy :) [17:50:25] marktraceur, have a look at this https://gerrit.wikimedia.org/r/#/c/104782/1/resources/mw.FormDataTransport.js [17:50:33] in this case, ok is never being called [17:51:18] im still thinking of looking into api dir cause its inside the api.post function [17:51:48] Well [17:55:51] marktraceur, well? [17:56:34] mayankmadan: I'm not sure what you mean by "ok is never being called" [17:57:23] marktraceur, the ok function in that file is never passed anywhere [17:57:35] tgr: Is it still just us for standups? [17:58:00] i think fabrice is back today [17:58:09] mayankmadan: It's passed into the api call, as the success callback [17:58:15] tgr: Ah, 'kay, need headset then [17:58:49] marktraceur, its not [18:00:05] Um, yeah, it is [18:00:10] As part of the configuration object [18:08:30] mayankmadan: Let me put it this way [18:09:05] mayankmadan: If you assume there's a function "foo", and I write the JavaScript "foo( { bar: 2 } );", am I passing the value 2 to the function? [18:09:25] no [18:09:31] What am I passing it? [18:09:40] you are passing an object with 'bar' as a key [18:09:49] And 2 as the value [18:09:56] So the function has access to the value 2 [18:10:08] yes [18:10:30] Similarly, if I write "foo( { bar: function ( baz ) { ... } } );", the function has access to the function bar. [18:10:32] It's the same with the API call [18:11:05] If foo is defined as "function foo( quux ) {", quux is not the function, but quux.bar is. [18:11:27] Similarly, in the call to the API, the OK function is called from the config object literal that's passed in [18:12:40] It's defined as "mw.api.post = function ( config ) {" and we're calling it as "mw.api.post( { ok: function ( data ) { ... } } );", and somewhere in the mw.api.post function it says "config.ok( data );" [18:12:46] I guarantee you that it happens. [18:13:13] If it doesn't, that's a bug in our code and mw.api, but unless you have hard evidence that it's never called, I'm not sure I believe it [18:37:21] mayankmadan: Making sense? [18:37:30] marktraceur, was away [18:37:35] will take a look now [18:55:51] I guess I'll work on the metrics scripts/dashboards [18:55:52] Sigh [18:56:00] mayankmadan: Any luck? [18:56:30] marktraceur, i still dont understand where it is passed part [18:57:05] mayankmadan: Start reading at line 219 [18:57:57] marktraceur, so you are saying that api.post will be calling ok [18:58:02] Yes. [19:12:15] where is the api.post function defined? [19:14:00] (03PS1) 10Apsdehal: Added No description message [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/105005 [19:14:09] marktraceur, i cant find it anywhere [19:14:35] mayankmadan: It's a core library [19:14:50] You shouldn't need to worry about that [19:15:39] marktraceur, then how am i supposed to see what are the possible values of response [19:17:22] It's an API call [19:17:29] mw.api wouldn't have possible values anyway [19:17:48] mayankmadan: Actually it's the *same* API call, so the same values are possible [19:18:00] okay [19:18:12] same as result? [19:18:34] Both api calls are to the upload api [19:18:41] Oh, wait [19:19:31] I think that's still the case [19:19:34] But I'm not totally sure [19:19:41] Let me look for something to do with chunked uploads [19:20:22] Hm, no, I don't get it [19:21:35] AHA [19:22:05] mayankmadan: We can get every single stage possible from the checkstatus module [19:22:25] mayankmadan: http://commons.wikimedia.org/w/api.php?action=help&modules=upload see the last parameter [19:22:32] It will send you to a totally different code path [19:22:52] And that can return multiple different stages [19:22:52] OK, now we're getting somewhere [19:23:12] mayankmadan: From the core includes/ directory, do "git grep stage" [19:23:13] That should give you all/most of the values [19:25:16] yep [19:25:30] Sorry I missed that [19:26:12] mayankmadan: You should reconsider the nature of your patch, though [19:26:32] mayankmadan: The comments shouldn't mention -undefined - the only reason it's ever returned that way is because of a bug [19:26:43] You should fix the bug and add the comments at the same time [19:26:47] marktraceur, and the bug report asks to do that [19:27:04] marktraceur, comment 4 [19:27:12] I see that [19:27:26] mayankmadan: Nemo was talking about all *valid* keys - -undefined is not valid [19:27:55] marktraceur, undefined is valid too, if response.upload.stage is not defined [19:28:11] It's possible, but not a valid thing logically [19:28:21] mayankmadan: We don't want to return *any* message if the stage is undefined [19:28:33] So you should check for that, and take a different path in that case. [19:28:48] Probably throwing an error about being unable to check the file's status [19:28:55] Depending on where you are [19:29:03] ok a try catch block? [19:29:57] mayankmadan: There's not *currently* any error, so no [19:30:20] marktraceur, so just an if statement [19:30:32] marktraceur, and that error should go into the console right? [19:30:34] I think so, yeah [19:31:05] You should probably fail the upload too, maybe after a few attempts at checking the status [19:31:49] marktraceur, fail the upload? [19:32:40] Mark it as failed [19:32:57] i dont understand [19:33:59] mayankmadan: Never mind, maybe just avoid putting the message anywhere [19:34:14] I'm not totally sure what a failed checkStatus would mean [19:34:15] oh [19:34:17] now i get this [19:34:25] get what you were trying to say [19:38:48] marktraceur, so the values of all the occurences of stage in includes can be the returned value? [19:40:10] I believe so [19:40:36] I see queued, assembling, and publish [19:40:57] yep [19:49:19] (03PS2) 10Mayankmadan: Adding code comments of resulting keys [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/104782 [19:49:32] marktraceur, can you have a look at the patchset [20:05:05] (03CR) 10MarkTraceur: [C: 04-1] Adding code comments of resulting keys (033 comments) [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/104782 (owner: 10Mayankmadan) [20:05:08] mayankmadan: Style issues mostly [20:06:27] marktraceur, what comments? [20:06:44] Read both files [20:10:12] marktraceur, what is wrong with spacing [20:10:55] mayankmadan: https://www.mediawiki.org/wiki/CC/JS [20:16:02] (03PS3) 10Mayankmadan: Adding code comments of resulting keys [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/104782 [20:16:09] marktraceur, bettter? [20:16:41] mayankmadan: Almost - you need a space between if and the left-paren [20:16:48] "if (" instead of "if(" [20:16:56] Because if is not a function, it's a keyword [20:18:24] (03PS4) 10Mayankmadan: Adding code comments of resulting keys [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/104782 [20:18:33] marktraceur, now? [20:20:21] (03CR) 10MarkTraceur: [C: 032] "'kay, I assent!" [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/104782 (owner: 10Mayankmadan) [20:20:27] Gratz, mayankmadan [20:20:49] marktraceur, oh that feels good [20:20:54] (03Merged) 10jenkins-bot: Adding code comments of resulting keys [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/104782 (owner: 10Mayankmadan) [20:21:02] marktraceur, just one patch left before the task is completed [20:21:08] Yup [20:27:48] marktraceur, i dont know about the second bug, i dont think there is anything to do there [20:28:02] marktraceur, https://bugzilla.wikimedia.org/show_bug.cgi?id=54460 [20:29:04] Hm [20:29:46] marktraceur, the message key should be defined in the i18n file [20:30:44] Well [20:31:20] well? [20:31:50] A stash error means something went wrong in stashing the file [20:32:05] There's nothing more specific [20:32:11] So probably just add a message that says that [20:32:18] i was just saying that [20:32:36] :) [20:32:44] marktraceur, says that? [20:34:39] Says "There was an error when we tried to upload the file." [20:35:17] in the i18n file right [20:35:57] Yeah [20:36:53] marktraceur, can i my name in the file as one of authors too? :) [20:37:10] Oh, sure! [20:37:22] We tend to do that after significant contribution, but there's no hard and fast rule [20:37:26] Mostly it's when we notice it [20:37:50] marktraceur, then i wont add it [20:39:40] Oh, in the i18n file you can [20:39:43] Sorry, I misunderstood [20:46:09] (03PS1) 10Mayankmadan: Adding a message for api-error-stasherror [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/105040 [20:47:44] marktraceur, https://gerrit.wikimedia.org/r/#/c/105040/ [20:47:48] how does this look [20:51:59] marktraceur, and i cant edit https://www.mediawiki.org/wiki/API:Upload [20:52:04] its in read only mode [20:52:51] Oh, hm [20:52:54] I'll do it then [20:54:33] marktraceur, how about the patch, i was going to ask andre to close it [20:54:53] mayankmadan: Interesting, there's a message for these in core, but not for stasherror [20:54:57] I'm confused [20:55:08] Let me ask yurik something [20:55:12] okay [21:27:46] (03Abandoned) 10Mayankmadan: Adding a message for api-error-stasherror [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/105040 (owner: 10Mayankmadan) [21:37:30] (03PS1) 10Mayankmadan: Including api-error-stasherror from core [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/105097 [21:48:18] (03PS2) 10MarkTraceur: Including api-error-stasherror from core [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/105097 (owner: 10Mayankmadan) [21:49:38] (03CR) 10MarkTraceur: [C: 04-1] Including api-error-stasherror in frontend messages (031 comment) [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/105097 (owner: 10Mayankmadan) [21:51:10] (03PS3) 10Mayankmadan: Including api-error-stasherror from core [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/105097 [22:02:58] (03CR) 10jenkins-bot: [V: 04-1] Including api-error-stasherror from core [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/105097 (owner: 10Mayankmadan) [22:04:55] (03CR) 10jenkins-bot: [V: 04-1] Including api-error-stasherror from core [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/105097 (owner: 10Mayankmadan) [22:07:44] marktraceur, can i get a +1 now :) [22:08:12] Jenkins still says you're failing [22:08:22] Oh, no, never mind [22:09:22] mayankmadan: Hm, maybe we should say something about stashing in the message [22:09:32] "There was an error while uploading the file to the stash." maybe [22:09:50] okay [22:15:06] (03CR) 10MarkTraceur: [C: 031] "Once I2895fe77d147a9065710912b5c2673ab833dd29f is merged this is good to go." [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/105097 (owner: 10Mayankmadan) [22:19:40] (03CR) 10Mattflaschen: [C: 032] Replace usage of SpecialPage::getTitle with getPageTitle [extensions/PronunciationRecording] - 10https://gerrit.wikimedia.org/r/103972 (owner: 10Legoktm) [22:19:47] (03CR) 10Mattflaschen: [V: 032] Replace usage of SpecialPage::getTitle with getPageTitle [extensions/PronunciationRecording] - 10https://gerrit.wikimedia.org/r/103972 (owner: 10Legoktm) [22:30:50] (03CR) 10Siebrand: [C: 04-1] "i18n/L10n reviewed. Inline issue with qqq." (031 comment) [extensions/MultimediaViewer] - 10https://gerrit.wikimedia.org/r/105005 (owner: 10Apsdehal) [22:35:45] marktraceur, the other one is merged [22:36:16] (03CR) 10MarkTraceur: [C: 032] Including api-error-stasherror from core [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/105097 (owner: 10Mayankmadan) [22:36:27] Super [22:37:39] (03Merged) 10jenkins-bot: Including api-error-stasherror from core [extensions/UploadWizard] - 10https://gerrit.wikimedia.org/r/105097 (owner: 10Mayankmadan) [22:41:39] andre__, around? [22:51:05] mayankmadan: I think he's still on vacation [22:51:18] robla, thanks [22:53:24] mayankmadan: I'm half-around, yeah. How can I help? [22:53:47] (first day back at work, catching up) [22:53:59] andre__, can you have a look at http://www.google-melange.com/gci/task/view/google/gci2013/5854794258317312 [22:54:15] all the patches are merged so i think it can be closed [22:54:30] mayankmadan: qgil closed it 7 min ago [22:54:39] sorry [22:54:40] I'm too late :D [22:54:44] :) [22:54:54] sorry to bother you [22:54:59] Are the "File:File:..." links a known bug, or should I file one? [22:57:38] quiddity: I'm not aware of any: https://bugzilla.wikimedia.org/buglist.cgi?short_desc=file&query_format=advanced&short_desc_type=allwordssubstr&component=MultimediaViewer [22:58:17] I haven't heard of it [22:58:26] Oh, did a new version go out!? Oh man. [22:58:53] Looks like quim already noted it here: https://bugzilla.wikimedia.org/show_bug.cgi?id=57842#c6 [22:59:12] so yah, that problem's live in Enwiki now. :/ [22:59:19] Ooh, this is image bucketing [23:00:44] ok, I'll leave it with you. Sorry to be the bearer of bad news. [23:00:58] Oh, crap [23:01:02] Thanks [23:02:55] marktraceur, if you are not busy, can we start working on https://gerrit.wikimedia.org/r/#/c/104012/ [23:03:22] mayankmadan: It sounds like after my half hour meeting I'm going to get to fix a bug and (hopefully) backport it [23:03:25] So maybe tomorrow [23:03:41] marktraceur, okey dokey [23:37:39] legoktm, can you have a look at https://gerrit.wikimedia.org/r/#/c/105111/ [23:38:12] * legoktm clicks [23:41:04] mayankmadan_: I left two comments, but I really don't know how that code works [23:41:53] legoktm, look at bryan's comments on https://gerrit.wikimedia.org/r/#/c/104012/