[21:01:06] allright, RFC discussion about to start! [21:01:12] wee [21:01:30] #startmeeting TechCom RFC discussion [21:01:30] Meeting started Wed Oct 11 21:01:30 2017 UTC and is due to finish in 60 minutes. The chair is DanielK_WMDE. Information about MeetBot at http://wiki.debian.org/MeetBot. [21:01:30] Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. [21:01:30] The meeting name has been set to 'techcom_rfc_discussion' [21:01:35] #topic Move RunJobs.php to the mediawiki (core) repository [21:01:37] \o [21:01:43] #link https://phabricator.wikimedia.org/T175146 [21:01:51] hello [21:01:53] hey mobrovac! Glad you are here :) [21:02:03] :) [21:02:29] before we start, i'd like to encourage everyone to make liberal use of the #info command, so we get nice minutes. [21:02:31] o/ [21:02:33] ok. so... [21:02:52] mobrovac: what are the main questions you'd like to see answered in today's session? [21:03:51] as we are now working on this new back-end for the job queue (the transport for now), we kind of noticed that mediawiki-config might not be the right place for runJobs.php (and its new sibling runSingleJob.php) [21:04:11] in our opinion, it should be part of the code, instead of considered "config" [21:04:29] I see the scripts require multiversion - which seems to be uncommon for core code [21:04:47] the main selling point is that while executing jobs that are backed by redis might be wmf-specific, running jobs in general is not [21:04:57] it should probably be an API action [21:05:09] oh, except that it has a wiki switch... [21:05:15] SMalyshev: and the comment I had prepared ws that it would be nice to integrate more of wikifarm stuff into vanilla MW [21:05:17] And also that the runJobs.php in mw-config is mostly a clone of Special:RunJobs [21:05:38] Krinkle pointed out on the task that special:runjobs is already in core [21:05:57] MaxSem: *some* wikifarm stuff, yea. not necessarily the wikifarm stuff we currently have... [21:06:01] going off what Pchelolo said about the similarity of special:RunJobs and runjobs.php why not merge the two into one single thing? [21:06:02] multiversion is all in MW config, so RunSingleJob.php, which use multiversion, therefore has to be in some WMF-specific place [21:06:13] <_joe_> well runJobs.php is that, but runSingleJob.php is a first very rudimentary attempt at a real RPC endpoint [21:06:39] #info in our opinion, [RunJobs.php] should be part of the code, instead of considered "config" [21:06:47] #info I see the scripts require multiversion - which seems to be uncommon for core code [21:07:02] <_joe_> that's qualitatively different than what Special:RunJobs does [21:07:04] so what prevents us from converting those to special: ? [21:07:10] Multiversion in RPC in config is a red herring [21:07:26] whoops [21:07:35] It's only used because it's a new entry point. If we use a core special page, this would still happen, but through our main index.php multiversion entry point. [21:07:54] E.g. more sharing, less duplication. [21:08:09] the first few lines in mw-config/rpc/*.php are a copy of mw-config/w/*.php [21:08:18] #info Tim said: this should probably be na API action (but maybe not, since it has a wiki switch) [21:08:37] runJob.php needs a wiki to be used anyway, which effectively makes it non-multiversion [21:08:47] I am sorry if this was stated in the task, but is there a reason that we have both special:RunJobs and runjobs.php? [21:08:55] Krinkle: so if it was an api action, it wouldn't *need* to be multiversion? [21:08:59] no, Krinkle is right, it only uses multiversion to start up [21:09:02] mobrovac: Not exactly. multiversion means map 1 wiki to 1 version. That still needs to happen. [21:09:11] <_joe_> Zppix: yes, the latter is an entrypoint for the jobrunner in the WMF setup in production [21:09:26] <_joe_> Zppix: operations/mediawiki-config/rpc [21:09:38] mobrovac: In production we invoke maintennace script thoruhg mwscript instead of php for this reason, mwscript = multiversion, maps --wiki to the right mediawiki code base. [21:09:39] thanks joe [21:10:00] Krinkle: right, but that is used only to select the wiki, and since a full MW is deployed on the job runners, you could just set the host header instead to achieve the same thing [21:10:35] So the idea, as I proposed as alternate to moving rpc to core, is to use Special:RunJobs as starting point. From WMF perspective this would mean switching from calling over curl-to-HHVM /wiki/Special:RunJobs instead of localhost/rpc/?wiki= [21:11:01] _joe_: now I'm confused. Special:RunJobs is the production entry point? I thought rpc/RunJobs.php is? [21:11:24] <_joe_> DanielK_WMDE: I think I said that rpc/RunJobs.php is [21:11:49] DanielK_WMDE: they both exist in production and they're doing the same thing pretty much [21:11:55] <_joe_> "the latter" in Zppix question was indeed rpc/RunJobs.php :) [21:11:55] yea, but Zppix was asking about maintenance/runjobs.php vs Special:RunJobs, I think. [21:11:56] DanielK_WMDE: Krinkle has a really nice table over at https://phabricator.wikimedia.org/T175146#3618503 [21:12:11] At WMF we used to run jobs via core:maintenance/runJobs.php, but Aaron created wmf-config:rpc/runJobs.php (as a copy of it) so that it could be called over HTTP with HHVM caching. [21:12:14] DanielK_WMDE: ah no i meant rpc my bad for not being specfic [21:12:23] <_joe_> mobrovac: will Special:RunJobs be able to replace RunSingleJob.php as well? [21:12:26] ok, my bad then for being confused :) [21:12:41] the special page does not have JSON result formatting, the action API gives you that [21:12:41] <_joe_> I don't think so? [21:12:50] As far as I know there was no strong reason for that not to have used Special:RunJobs, except that we weren't using the Special Page so in spirit of least-possible-change, a new entry point was created instead. [21:12:53] _joe_: we can make it so, but imo we should first create speical:runsinglejob and then fold these two together out of caution [21:13:02] #info Krinkl's overview of Special:RunJobs vs rpc/RunJobs: https://phabricator.wikimedia.org/T175146#3618503 [21:13:08] <_joe_> also, waht about authorization/authentication? [21:13:15] RunSingleJob, in the way it takes a JSON specification and produces a JSON result, is almost identical to ParsoidBatchAPI [21:13:20] _joe_: mobrovac: Special:RunJobs would be equiv to rpc/runJobs.php. RunSingleJob would become its own new special page, the same way the current one is. [21:13:44] _joe_: we will reuse most of the code from Special:RunJobs and get to message signatures right away [21:13:50] TimStarling: I suppose considering an API action instead. although that seems somewhat orthogonal to whether or not WMF migrates to the "core" end point (be it special page or API). [21:14:07] as for authorization, ParsoidBatchAPI has a client IP filter, like RunSingleJob.php [21:14:16] TimStarling: are you opposed to moving the scripts into core as-is? [21:14:19] I do note however that the special page (unused by WMF right now, used by third-parties by default in the loopback mechanism for async jobs), did used to be an API and was moved to be a special job for some reason. [21:14:23] <_joe_> TimStarling: ok you answered my next question [21:14:26] how Special:RunJobs would be authenticated? secret password? [21:14:31] <_joe_> :) [21:14:46] #info RunSingleJob, in the way it takes a JSON specification and produces a JSON result, is almost identical to ParsoidBatchAPI [21:15:08] SMalyshev: https://phabricator.wikimedia.org/T175146#3618503 says its secrect key [21:15:08] <_joe_> #info < TimStarling> as for authorization, ParsoidBatchAPI has a client IP filter, like RunSingleJob.php [21:15:13] hehe, that's a low bar [21:15:17] SMalyshev: just like right jow - with secret key signing of the payload going to it [21:15:20] almost the same as "it's a POST API end point" [21:15:22] #info As far as I know there was no strong reason for that not to have used Special:RunJobs [21:15:29] TimStarling: do you think a client ip filter is better than the secret key apporach employed by special:runjobs? [21:15:41] SMalyshev: Right now RPC uses IP filter, and the special page uses a secret key. If we prefer one over hte other, we can easily add logic for that to the special page. [21:15:45] <_joe_> mobrovac: for our production, it is [21:16:12] a secret key is probably easier to configure, if the client and server get it from the same configuration [21:16:17] Pchelolo: ah ok. we just need to ensure there's no default mode where it doesn't do the right thing then, if we move it to core... [21:16:31] _joe_: wouldnt that mean needing to update something each time you need to allow another ip (for some reason) access? [21:17:08] #info <_joe_> waht about authorization/authentication? a secret key is probably easier to configure, if the client and server get it from the same configuration [21:17:31] what SMalyshev just said makes me think usgin the secret key approach might be better, but we can always make it work as we want based on config [21:18:28] Zppix: No, because the IP whitelist isn't the internal IP, but 127.0.0.1 [21:18:36] it's called from the same server, not cross server [21:18:52] Krinkle: I see okay [21:19:00] Krinkle: that will change though, as we are putting an lvs in front of the job runners [21:19:03] I will note that secret key may be non-trivial, assuming the key is not currently in puppet but in wmf-config on the deployment host locally? [21:19:18] For job runners to use it, the key would also need to be available in puppet's secret store. [21:19:43] this sounds like a minor config annoyance, but doable ^ [21:19:55] it's just $wgSecretKey, it's in the MW config [21:20:16] <_joe_> and you need to have it in puppet as well, of course, but that's an implementation detail [21:20:23] it should be in the private repo already, shouldn't it? [21:20:26] Sure, but if the comparison is about ease, then verifying remote_addr (which is what we do now) would be easiest. But if we want to use a token not because its easier but because its more secure or otherwise preferred, then by all means. [21:20:38] mobrovac: why would it be? [21:20:56] Krinkle: because it's a secret? :P [21:21:05] oh it's on tin [21:21:06] nm [21:21:19] last I checked, private MW settings are not version controlled [21:21:36] I just wanna make sure switching from remote_addr to a secret token isn't going to be considered a negative thign or a blocker to moving away from our custom RPC endpoint. [21:21:38] shouldn't they be? (but we are diverging with this) [21:21:45] I think it's a git repo now on tin, but just locally there. [21:22:02] yeah, checking now, there is a local git repo [21:22:06] Krinkle: in the new kafka-based JQ we're working on it's impossible-ish to verify access by IP, we're getting all the jobrunners under LVS [21:22:25] <_joe_> Pchelolo: you are not correct [21:22:28] Aside from the authentication difference, another difference between RPC and SpecialPage (or API), is that it will need to be invoked with the wikis' canonical hostname. [21:22:55] That information should already be available to jobchron/jobrunner service, but might be more difficult for the Kafka-based service. [21:22:57] that's fine, we know the domain for each wiki [21:23:02] given it's not in the job meta data afaik [21:23:04] Okay [21:23:09] we can totally verify access by IP, but doing it by secret key is simpler for core [21:23:09] the dbname is [21:23:22] #info I just wanna make sure switching from remote_addr to a secret token isn't going to be considered a negative thign or a blocker to moving away from our custom RPC endpoint. [21:23:25] <_joe_> secret key it is then :) [21:23:35] secret key +1 [21:23:56] ok, have we settled on using special:runjobs instead of mw-config/rpc/runJobs? [21:24:14] IP seems to be more dangerous - i.e. if we check for localhost, every "make code access URL" vulnerability is now automatically RCE [21:24:16] TimStarling: Ah, I see. simpler because of third parties? [21:24:26] #info Aside from the authentication difference, another difference between RPC and SpecialPage (or API), is that it will need to be invoked with the wikis' canonical hostname. [21:24:33] I misunderstood earlier, thinking it'd be trivial to swap the token logic in core with remote_addr. [21:24:42] yeah, same reason it uses a secret key in Special:RunJobs [21:24:45] <_joe_> I want to stress the security implications of a special page/api action allowing to submit the data of the job, as RunSingleJob.php does [21:24:49] cool [21:24:52] token it is :) [21:25:35] #info consensus on using a secret key achieved [21:25:36] simpler for WMF too really, do you trust ops to be able to maintain a list of internal networks? ;) [21:25:58] #info <_joe_> I want to stress the security implications of a special page/api action allowing to submit the data of the job, as RunSingleJob.php does [21:26:07] #info <_joe_> I want to stress the security implications of a special page/api action allowing to submit the data of the job, as RunSingleJob.php does [21:26:11] eh [21:26:26] ;) [21:26:52] ok, have we settled on using special:runjobs instead of mw-config/rpc/runJobs? [21:26:53] _joe_: do you think that changes whether we use remote_addr or a secret key? [21:27:08] <_joe_> mobrovac: nope, I suggested to digitally sign the message, is such a mechanism already in MW? [21:27:09] my preference would be to introduce a new action API module to core which is very similar to Special:RunJobs [21:27:15] TimStarling: right now it only needs 127.0.0.1 though, but yeah, for SingleJob it would need wider - assuming the kafka consumer is not on the same server. Although I don't see why it would need to be on a different server? Is that the plan? [21:27:26] #info my preference would be to introduce a new action API module to core which is very similar to Special:RunJobs [21:27:47] maybe even deprecate Special:RunJobs, why do we have APIs that are special pages? [21:27:56] <_joe_> I agree that the API seems like the natural place for something like this [21:28:03] Krinkle: we will be using lvs, which has the anti-feature of not being able to handle correctly requests that come from the server it is supposed to load-balance [21:28:14] #info Special:RunJobs actually used to be an API action, and was converted to a special page. [21:28:14] <_joe_> mobrovac: wat? [21:28:28] _joe_: what what/ [21:28:44] TimStarling: i was thinking the same thing... [21:28:45] yeah, I don't think that would work with LVS-DR [21:28:54] <_joe_> mobrovac: you don't want to make requests from the jobrunner to itself to be load-balanced, right? [21:29:02] hmm [21:29:10] yeah, that's what i'm trying to explain _joe_ :) [21:29:11] <_joe_> sorry, we're falling off track [21:29:45] Krinkle: do you know why? [21:29:47] TimStarling: i'd be in favour of an api too, but do we even know if/how special:runjobs is used? [21:29:59] mobrovac: Are the kafka-subscribers central (e.g. one or two, and on its own server separately and submit over hTTP to the jobrunner cluster), or would they replace the jobrunner service and can sit on each jobrunner server locally. [21:30:05] Kafka can fan out the objects right? [21:30:24] mobrovac: TimStarling maybe slowly deprec special page then encourage API before just deprec it completely [21:30:30] Krinkle: the set of hosts that can send a job is limited, yes [21:30:34] mobrovac: It is only used internally by MW itself within page view requests on third-party wikis that don't use crons or separate services. [21:30:35] mobrovac: in my mind, both the special page and the api module should be thin wrappers around a class that implements the actual logic [21:30:43] _joe_: Session:_get/setSecret does data signing [21:30:48] it is a special page instead of an internal PHP class so that it can be asynchronous. [21:30:53] could probably be turned into a generic helper [21:30:54] and not blocking the unrelated page view request. [21:30:56] <_joe_> tgr: great [21:30:57] it's then easy to have both, or replace one interface with the other [21:31:01] Should be trivial to change to an API [21:31:12] #info DanielK_WMDE: mobrovac: in my mind, both the special page and the api module should be thin wrappers around a class that implements the actual logic [21:31:12] "Moved ApiRunJobs to a special page instead of going through ApiMain and having to fight the logic there. As a separate internal API, this does not show up on the API help page and is no longer effected by $wgEnableAPI" [21:31:28] said Aaron in 2014 [21:31:41] DanielK_WMDE: JobRunner is the mediawiki PHP class abstracted from this. [21:31:48] _joe_: well, it's signing + encryption, so maybe overkill? [21:31:56] And is shared already between wmf/rpc/runJobs, SpecialRunJobs and maintenance/runJobs [21:32:20] <_joe_> tgr: yeah, overkill probably. We already have TLS for encryption at that level [21:33:14] the commit introducing the special page references https://phabricator.wikimedia.org/T64233 [21:33:31] #info the commit introducing the special page references https://phabricator.wikimedia.org/T64233 [21:33:36] thanks tim!° [21:34:40] #info The SpecialRunJobs is already abstracted with the JobRunner class and re-used by the core/maintenance/runJobs.php script too [21:35:07] if special:runjobs is used by the async mechanism then we can't really get rid of it [21:35:35] however, when you look at the source code, it's secret checking + param validation + invoking jobrunner [21:36:04] uh - this was because of the need to set headers? that should work from an api module, no? [21:36:07] which makes me think we can safely start using an api end point and leave the special page untouched [21:36:13] we also can't get rid of it if AaronSchulz still wants it [21:36:25] mobrovac: Why not? the internal mechanism has changed in backwards-incompatible ways at least twice in the last three major releases. I'm sure we can change or move it again. [21:37:08] I'm just saying that it's safe to leave it there given that it's a thin wrapper [21:37:38] even though cleaning up is always nice [21:37:46] I reviewed it earlier today with Aaron during our team meeting. He'll want to take another good look at it, but from first look no immediate concerns came to mind about problems with WMF using the special page instead of RPC, or with the special page becoming an API module - aside from wgEnableAPI compat, which is obsolete now. [21:38:01] ahhh [21:38:19] But yeah, we can deprecate and migrate over 1 major release just to be safe. [21:38:26] the old API action was disconnecting from the client and running jobs in the background [21:38:49] that was the reason for the log warning message that Aaron was fixing by moving to a special page [21:38:51] Krinkle: as in wgEnableApi is not relevant any more? (i.e. can't be effectively disabled) [21:39:03] indeed, afak that variable is deprecated. [21:39:24] RunSingleJob.php doesn't do that, and doesn't want that, so an API action that works like RunSingleJob is not such a big problem [21:39:25] it's not, suprisigingly enough [21:39:27] TimStarling: but couldn't that be handled in the api in the same way as it is in the special page? [21:39:43] Hm.. no, wgEnableAPI is (still) not deprecated. [21:39:58] probably time to kill it though [21:40:02] Yeah. [21:40:09] you don't benefit so much from being an API module if you're overriding the whole output layer [21:40:31] #info the old API action was disconnecting from the client and running jobs in the background [21:40:40] #info RunSingleJob.php doesn't do that, and doesn't want that, so an API action that works like RunSingleJob is not such a big problem [21:41:12] TimStarling: indeed, in that regard runSingleJob is "synchronous" to the request [21:41:15] #info you don't benefit so much from being an API module if you're overriding the whole output layer [21:41:42] Krinkle: I wonder what breaks if you disable the API :) [21:42:21] actually, it calls flush(), but I don't think it has any way to actually disconnect [21:42:36] it just has a comment saying "it would be cool if we could disconnect here" [21:42:45] die()? [21:42:49] yeah, we do the same in index.php/MediaWiki.php at the end of output before doing deferred updates [21:42:59] which includes, for third-parties, the job runner over http. [21:43:18] It allows the CGI layer to disconnect if it supports it, like HHVM. [21:43:29] Which keeps the process running but the http connection can be closed. [21:43:43] Although with output buffering and keepalive there isn't that much difference. [21:45:44] does moving to the special page and then looking at options for moving to an api module sounds ok to people? [21:46:06] we could move runsinglejob to an api module right away, though [21:46:38] (the shift key isn't my friend today apparently) [21:46:40] <_joe_> I would prefer we move runsinglejob to an api module indeed [21:46:45] mobrovac: +1 but i fear in doing this it will just keep being put on the backburner to move to api [21:47:46] I guess it's OK with me if that's how you want to do it [21:48:51] I would prefer an API module but wouldn't block using the special page [21:49:37] TimStarling: i'm for an api module too, but it sounds like the async nature of runjobs is the real impediment here [21:49:51] unless we override the api output pipe [21:50:00] Are we fine with one always being an API and one always being a special page? [21:50:02] you can introduce an API module which is synchronous [21:50:07] #info I would prefer an API module but wouldn't block using the special page [21:50:18] and leave the special page for now as the async entry point [21:50:41] Does rpc/RunSingleJob and would API/runSingleJob produce output of some sort? E.g. JSON API result? [21:50:48] yes [21:51:02] Krinkle: seems ugly, but if there are good reaons, estetics may need to take a back seat. [21:51:07] and it is compatible with the current APiResult format? e.g. one top-level object? [21:51:13] Krinkle: yeah, we need at least the return code for it, to know whether to retry the job [21:51:44] could there be other uses for an async api module base? [21:52:43] can't think of one. mediawiki's idea of async *is* jobs [21:52:57] if so, it might be worth introducing it, but if not, i'm not so sure [21:52:59] Looks like it only needs an HTTP status code. The ApiResult can be some generic "success":true placeholder I suppose. No compat worries there. [21:53:14] well, there is DeferredUpdates [21:53:32] for example when you do an edit, there are deferred updates that can theoretically happen after client disconnect [21:53:33] but that's not really async, just after output. [21:53:36] yes, both API and index.php requests are async by default. [21:53:45] But that's orchestrated at a lower level [21:53:47] that's the whole thing DanielK_WMDE [21:53:49] not by individual api modules. [21:53:54] i mean: deferred is not concorrent. [21:54:01] They both involve MediaWiki:restInPeace after request shutdown. [21:54:18] that is the sense in which Special:RunJobs is asynchronous [21:55:01] calling ignore_abort from an individual API module doesn't seem that weird or problematic. [21:55:02] oh ok [21:55:10] That's not overriding the output imho. [21:55:19] Special:RunJobs was introduced because the previous ApiRunJobs was calling flush(), which was breaking things [21:55:59] five minute warning [21:56:01] Krinkle: are you saying that using ignore_abort nowadays would be ok? [21:56:01] it's unclear to me whether calling flush() had any purpose in reality [21:56:14] but the idea of it as documented was to allow the client to disconnect [21:56:34] TimStarling: doesn't it allow the client to send another api request before the job finishes? [21:57:07] i suppose that's the intent behind disconnecting [21:57:15] TimStarling: Right. ignore_abort is to ensure the code keeps running after the in-page/post-shutdown client disconnects or times out. [21:57:19] but the flush() seems odd indeed. [21:57:22] mobrovac: is there anything you think we should address in the remaining minutes? [21:57:57] DanielK_WMDE: it sounds to me like we can hash out the special page vs api for runjobs on the task [21:58:03] otherwise, no, i wouldn't say so [21:58:17] I guess the issue there is that we need the in-page socket to stay open long enough to reach this point. The current special page still does that. [21:58:24] I guess flush() is the only way to make sure the http status header is sent [21:58:40] good point ^ [21:59:15] i would also guess that the special page overrides and hides the headers already sent error for that reason [21:59:31] Yep, the socket we have in MediaWiki.php#triggerAsyncJobs does an fread() until it gets at least 1 byte, and then it closes. [21:59:52] given fgets() is blocking until there is something, right? [22:00:05] there you go [22:00:12] So yeah, as an API module, RunJobs would be a probelm given it would need to override output handling from Api [22:00:16] everything makes sense with Aaron's code as long as you have an hour to stare at it [22:00:26] :) [22:00:27] #info the socket we have in MediaWiki.php#triggerAsyncJobs does an fread() until it gets at least 1 byte, and then it closes. [22:00:31] hehe [22:00:35] ok. [22:00:38] last words? [22:00:51] #endmeeting [22:00:51] Meeting ended Wed Oct 11 22:00:51 2017 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4) [22:00:51] Minutes: https://tools.wmflabs.org/meetbot/wikimedia-office/2017/wikimedia-office.2017-10-11-21.01.html [22:00:51] Minutes (text): https://tools.wmflabs.org/meetbot/wikimedia-office/2017/wikimedia-office.2017-10-11-21.01.txt [22:00:51] Minutes (wiki): https://tools.wmflabs.org/meetbot/wikimedia-office/2017/wikimedia-office.2017-10-11-21.01.wiki [22:00:51] Log: https://tools.wmflabs.org/meetbot/wikimedia-office/2017/wikimedia-office.2017-10-11-21.01.log.html [22:00:56] thank you all for coming! [22:01:06] thnx! / [22:01:15] SpecialRunJobs is a custom REST endpoint, much like anything else that calls OutputPage::disable() and is not suitable for migration to an API module in its current form. [22:01:54] Although I suppose we could make it fit,maybe, if we move the job execution on would-be ApiRunJobs to a deferred update and make the API module just return something generic. [22:02:16] TimStarling: too crazy? [22:02:45] index.php page view, post shutdown deferred updates + run jobs async by making a request to api.php, with its job runner in another deferred update. [22:03:03] to get around the output handling [22:03:23] but that still involves the special page [22:03:24] Krinkle: Wikibase has such an endpoint, too. Special:EntityData. [22:03:37] and of course action=raw [22:03:43] speaking of REST endpoints... [22:03:45] yep [22:04:09] you know at some point I am going to propose having a REST API in MW core [22:04:21] mobrovac: Nope, if we move job execution from inside SpecialPage::execute to a deferred update queued on that page, the specialrunjobs page wuldn't need to ignore abort or flush output, because we'd let MediaWiki do that naturally. [22:04:28] And the same could be done for a would-be ApiRunJobs module [22:05:16] so maybe a requirement for that new API could be migrating all these odd pseudo-APIs to it [22:05:21] lost you there Krinkle, but might be because of tiredness, but my question was - wouldn't that still involve calling the special page? (i guess the answer is no) :P [22:07:53] the kafka thing needs the job execution result, so the new API module can't defer [22:07:55] mobrovac: the answer is no, yeah. Right now for MW core stock running jobs without cron access is to, when viewing a page from apache, to, once the request is mostly done and the user has the page HTML, ask the CGI layer to disconnect if it can, and once that's done, or right away if it can't, to do jobs async. It does jobs async by opening a raw socket to localhost and forming a basic HTTP/1.1 GET request for Special:RunJobs, and [22:07:55] closing the socket as soon as its gets the start of any response. [22:08:35] mobrovac needs a synchronous job runner API [22:09:05] It does that by disabling the normal output from the special page, and flushing headers early, then continuing to execute jobs as part of the normal special page. [22:09:22] Yeah, the new RunSingleJob end point can be a normal synchronous API and has no issues or compat problems. [22:09:23] Krinkle: ha, ok, that's the "trigger the jobs as part of the request" feature [22:09:48] But I'm taking about the main runJobs endpoint becoming an API. We previously thought it wasn't possible as API because it needs to override outpout handling right? [22:10:18] the new API does not need to override output handling [22:10:31] Well, I just thought, we *can* do it, by making SpecialRunJobs not execute jobs when building the page, but via a DeferredUpdate. [22:11:09] The a new ApiRunJobs endpoint that would replace SpecialRunJobs would need output handling in its current implementation, but I'm suggesting there is a way to make it compatible and yet without output overriding. [22:11:44] TimStarling: Sorry for the confusion, but do you understand the idea? [22:11:49] yeah, your idea is an improvement on what was there before the special page was introduced [22:12:30] but considering we're talking about a feature that is only used on small installations, server support matters [22:13:17] not everyone will have fastcgi_finish_request() [22:13:23] The current special page still does this today (for third-parties), fgets(), ignore_abort, and flush() within SpecialRunJobs::execute(). Removing that in favour of making SpecialRunJobs run jobs via a DeferredUpdates means it wouldn't need to output early, instead we can let MediaWIki's default output handle it, which *already* also calls ignore_abort and flush() in restInPeace. [22:13:45] on every request [22:13:56] and that would make it compatible with API logic. [22:14:07] without fastcgi_finish_request(), it doesn't matter if it calls flush(), the UA will wait for more data until the deferred update is finished [22:14:07] without needing more server support or overriding of output. [22:14:19] ty o/ [22:15:02] I'm not suggesting we remove the indirection via the sub-request socket. We keep that. [22:15:10] yeah, got it [22:15:44] All that, just to show that we could (seemingly cleanly) move SpecialRunJobs to an API, without any downsides. [22:15:46] you can make an API module which returns and uses DeferredUpdate to actually run the job [22:15:51] Yep [22:16:50] as long as that behaviour is configurable via params since mobrovac needs job results [22:17:11] Well for the Run*Single*Job there wouldn't be any need to use a deferred update or return early. [22:17:28] you think that should be a separate action? [22:17:49] But if we want both SingleJob and RunJobs to be API modules, we can, by making the latter return early and use deferred updates to run the job (instead of manually flushing like it does now, to achieve the same result). [22:18:41] Aside from authentication, I don't see them sharing any code. One instantiates class JobRunner, the other JobExecutor. Internally JobRunner could use JobExecutor, but wouldn't be called directly by the API endpoint. [22:19:02] fair enoug [22:20:31] T115414 [22:20:32] T115414: Remove the ability to disable the API with $wgEnableAPI - https://phabricator.wikimedia.org/T115414 [22:23:27] TimStarling: Should that be an RFC, or is this something that could go through code-review alone once it has resourcing from someone? [22:24:59] it could just go through CR, it's been there once before: https://gerrit.wikimedia.org/r/#/c/246449/ [22:25:08] Aklapper wrote "I'd expect users to receive a deprecation warning in 1.26 to allow preparing for complete removal in 1.27 as expressed in the bug report." [22:25:35] which was apparently never done, it's not even soft-deprecated