[15:00:28] <wm-bot>	 Technical Advice IRC meeting starting in 60 minutes in channel #wikimedia-tech, hosts: @nuria - all questions welcome, more infos: https://www.mediawiki.org/wiki/Technical_Advice_IRC_Meeting
[15:28:14] <apergos>	 heh, another meeting and I just happen to have a question :-D see folks in half an hour!
[15:50:12] <wm-bot>	 Technical Advice IRC meeting starting in 10 minutes in channel #wikimedia-tech, hosts:  - all questions welcome, more infos: https://www.mediawiki.org/wiki/Technical_Advice_IRC_Meeting
[16:03:58] <bd808>	 apergos: apparently we ended up with no hosts for TAIM this week, but !ask and maybe I or someone else who is lurking can help
[16:04:05] <apergos>	 heh
[16:05:04] <apergos>	 oh I was going to keep slugging away at it; I want formatversion 2 for mediawiki api login so I don't have to dig 'failed' out of the json but that doesn't seem to be happening
[16:08:05] <bd808>	 interesting. So action=login&formatversion=2 is not returning in the expected format?
[16:08:48] <mainframe98>	 Hi, I've created https://gerrit.wikimedia.org/r/c/mediawiki/core/+/544296 to make link batches use injected services through a factory, but there's one test (from CirrusSearch) failing. This is because the LinkBatch constructor now call the service locator, something it previously did not. The CirrusSearch tests however are Unit tests, so there is
[16:08:49] <mainframe98>	 no service locator available. I also can't just turn them into Integration tests, as there's a test inheritance structure I'd have to break.
[16:09:29] <mainframe98>	 So, my question is: what should I fix? CirrusSearch? That can't be changed to use the introduced service until the patch is merged, and the patch doesn't merge until CirrusSearch is fixed
[16:09:59] <James_F>	 I can help out with TAIM too.
[16:10:11] <mainframe98>	 I can't make the tests inject a dummy service because the LinkBatch construction is buried somewhere in CirrusSearch
[16:10:30] <bd808>	 mainframe98: that sounds like a "fun" dependency loop to break and I was going to suggest you see what James_F has to say so ... :)
[16:10:41] * James_F laughs.
[16:11:28] <apergos>	 {"login":{"result":"Failed","reason":{"code":"wrongpassword","text":"Incorrect username or password entered. Please try again."}}}
[16:11:44] <James_F>	 mainframe98: As the breakage shows, you're making a breaking change. Normally that has to go through a forwards-/backwards-compatibility cycle.
[16:11:57] <apergos>	 that' what I get, rather than a nice 'errors' :  and a list
[16:12:21] <apergos>	 and that's with formatversion=2 and errorformat=plaintext and all the rest
[16:12:41] <apergos>	 (sorry, I was off in another window trying and fialing to get anything different/better)
[16:12:47] <James_F>	 In this case that's probably not feasible. For these edge cases, make the CirrusSearch patch to fix how it works with a "Depends-On:" line on both the MW and CirrusSearch repos (and make a patch for every other important repo), and then someone like me will have to force-merge one of them to cut the Gordian knot.
[16:14:52] <mainframe98>	 James_F: That would work. Should both patches depend on each other?
[16:15:36] <bd808>	 apergos: just playing around with Special:ApiSandbox, it looks to me like the login.result response is the same for both formatversion=1 and formatversion=2. Maybe this is a bug in the implementation of the response format for that action?
[16:15:51] <James_F>	 Yes. Normally that makes it impossible for CI to land (tree, not a cyclic graph), but in this case we're force-merging so it works.
[16:15:56] <bd808>	 or a poorly documented 'feature'
[16:16:05] <apergos>	 yeah i went to the sandbox and didn't get anything better there either
[16:16:10] <James_F>	 The two-way dependency makes CI actually test both together in both repos and so prove that post-merge the test will pass.
[16:16:18] <mainframe98>	 Oh, that's devious. Thanks!
[16:16:24] <apergos>	 maybe we should ask anomie 
[16:16:35] <apergos>	 who did a lot of the error return cleanup for the api
[16:18:05] * anomie looks at backscroll
[16:19:14] <apergos>	 ah sorry
[16:19:38] <apergos>	 yeah I can't get format version 2 (a nice errors list) from mw api 'login' action
[16:19:46] <apergos>	 I get {"login":{"result":"Failed","reason":{"code":"wrongpassword","text":"Incorrect username or password entered. Please try again."}}}
[16:19:49] <apergos>	 which is suboptimal
[16:20:22] <anomie>	 apergos: formatversion=2 doesn't change the format of the response from action=login, you'd still have to dig out the "Failed". Changing the semantics of the response rather than just the way they're converted to JSON is beyond what formatversion is for.
[16:21:02] <apergos>	 is there any hope for it to return its errors as 'errors' someday, or is that an outlier?
[16:25:01] <anomie>	 If someone were to write a patch adding a parameter to ApiLogin to have it report failures as errors rather than in the legacy format it uses, I'd review it.
[16:25:10] <apergos>	 heh
[16:25:33] <apergos>	 I'm too full up to be nerd-sniped into it but that is a good outlook for the future, thanks!
[16:26:04] <apergos>	 I've never really cared about this before in other scripts, just was trying to make everything look and behave nice...
[16:26:43] <James_F>	 To make the nerd-snipe easier, MatmaRex just did some patches converting API errors to real errors for saving hooks via TitleBlacklist and others.
[16:28:56] <apergos>	 anybody tempted?? ^^
[16:29:30] * apergos goes to start a new small batch of generate/upload/add caption/add item/add depicts to beta commons and wikidata
[16:29:42] <apergos>	 after testing the cleaned up script one more time
[21:11:07] <ottomata>	 hm, any MW API person here who knows what this means?
[21:11:08] <ottomata>	 https://www.mediawiki.org/wiki/API:Data_formats#JSON_parameters
[21:11:16] <ottomata>	 callback: The function in which the result will be wrapped. 
[21:11:23] <ottomata>	 is that refering to a php callback function name?
[21:11:50] <bawolff>	 ottomata: no, its referring to JSONP mode
[21:11:58] <bawolff>	 basically, don't use it, CORS is much better
[21:12:22] <bawolff>	 but if specified, it wraps the entire thing in a json function, so that people can include it as a <script> tag and bypass the same origin policy
[21:13:05] <ottomata>	 ok not what i need then
[21:13:24] <ottomata>	 bawolff: does my API result always have to be keyed with an integer?
[21:13:26] <ottomata>	 i'm getting
[21:13:34] <ottomata>	 {
[21:13:34] <ottomata>	   "0": {
[21:13:34] <ottomata>	     "button-click1": {
[21:13:34] <ottomata>	       "stream": "button-click1",
[21:13:34] <ottomata>	 ...
[21:13:45] <bawolff>	 I think if you do formatversion=2 it is much nicer
[21:13:47] <ottomata>	 i want to avoid the nesting 
[21:14:17] <ottomata>	 hm formatversion=2 still gives me nesting
[21:15:34] <bawolff>	 It might just be the particular api endpoint doing that
[21:15:43] <bawolff>	 what api endpoint is this?
[21:15:47] <ottomata>	 well its my own i'm develeoping it :)
[21:15:49] <ottomata>	 but i'm just doing
[21:15:58] <ottomata>	 $this->getResult()->addValue(
[21:16:12] <ottomata>	 $this->getResult()->addValue(null, null, $value)
[21:16:15] <ottomata>	 and i just want $value as the result
[21:18:21] <bawolff>	 I think normally, the second arg is the name of your api module, so maybe null is being turned into 0
[21:18:31] <ottomata>	 oh i guess i have to call addValue for each of my entries
[21:18:32] <ottomata>	 hmmm
[21:18:38] <ottomata>	 so i'm returning an assoc array
[21:18:46] <ottomata>	 i want that as the top level ret value
[21:18:55] <ottomata>	 i can call add value for each entry in my assoc array maybe
[21:19:40] <bawolff>	 I think normally stuff isn't totally top level, as the api wants somewhere to return warnings and errors
[21:19:46] <ottomata>	 hm
[21:20:34] <bawolff>	 Its also been quite a while since I did api stuff, so I could potentially be telling you all lies :)
[21:24:15] <bawolff>	 If you were doing something where you needed absolute control over the result (because it has to follow some standard or something) you might consider looking into getCustomPrinter()
[21:25:08] <bd808>	 ottomata: anomie is the super ninja wizard for this topic. If you can explain what you would like the output to look like he can probably tell you how close you can get.
[21:25:45] <ottomata>	 bd808:  bawolff  calling addValue multiple times worked!
[21:26:06] <ottomata>	 hopefully ilt is allowed... we will see in review i guessssss
[21:26:07] <ottomata>	 :)
[21:26:10] <Reedy>	 you mean in a loop?
[21:26:12] <ottomata>	 yeah
[21:26:17] <Reedy>	 rather than once after a loop of building an array?
[21:26:21] <ottomata>	 yeah
[21:26:26] <Reedy>	 That's vaguely standard practice, so should be all good :)
[21:26:37] <ottomata>	 https://www.irccloud.com/pastebin/hf1fpwR8/
[21:27:00] <Reedy>	 yeah, looks like usual api code
[21:31:52] <ottomata>	 ok great thank you all
[21:37:50] <anomie>	 ottomata: Usually you have all your actual data under some key (->addValue( null, 'somekey', $value )), which makes it less likely that you'll accidentally collide with the API's standard mechanism of warning and error reporting. But as you found, it's possible to call addValue() for each key in $value if you want.
[21:39:28] <ottomata>	 it'd be nice to do it as I am, as eventgate could use this result value from the API or from a static config file
[21:39:38] <ottomata>	 the config works with a URI path either way
[21:39:45] <ottomata>	 either local file:/// or remote http:// uri
[21:39:54] <anomie>	 Also, do be aware that addValue() can return false if the result size limit is exceeded. You should either handle that (good), or use NO_SIZE_CHECK (much less good, and really bad for ApiQueryBase subclasses).
[21:40:13] <ottomata>	 changing the format of the config for the file is a bit annoying, and adding conditional logic for MW api stuff is too
[21:40:26] <ottomata>	 ok i can check for false
[21:40:29] <ottomata>	 what's the limit?
[21:41:40] <anomie>	 The limit is $wgAPIMaxResultSize
[21:42:14] <ottomata>	 12MB currently it looks like
[21:44:40] <anomie>	 ottomata: Watch out that you don't run into problems when your config fetched from the API suddenly contains a "warning" element added by the API though.
[21:47:18] <ottomata>	 yeah...
[21:47:23] <ottomata>	 anomie:  in what case would I get that?
[21:47:29] <ottomata>	 some exception thrown?
[21:47:33] <ottomata>	 or size limit reached?
[21:48:05] <anomie>	 ottomata: A warning, more likely if there's some deprecation that affects all API modules. It happened to ZeroConfig at one point, although I don't recall the details.
[21:48:14] <ottomata>	 hm ok
[21:48:48] <anomie>	 ottomata: Size limit reached would do it, though. An uncaught exception would result in a "config" consisting of just an "error" key.
[21:49:09] <ottomata>	 ok, i will see if i can make those happen and see if i can deal with it or not
[21:49:10] <ottomata>	 thank you.