[11:12:31] FR is actually pretty well-written code for its time [11:13:47] it just has been abandoned for a decade and now whoever has to touch it first pays the accumulated maintenance costs of those 10 years [11:15:41] I stand by what I said in the stewardship review: review tracking is valuable and not terribly complex, freezing the last reviewed revision is very complex and of questionable value [11:17:15] huwiki disabled version freezing in 2018 [11:17:47] here's the newish editor count (users with 5-25 edits): https://stats.wikimedia.org/v2/#/hu.wikipedia.org/contributing/editors/normal|line|all|activity_level~5..24-edits|monthly [13:26:26] tgr: the theory being that having changes be immediately visible post-save for other users encouraged editors? [13:26:50] That seems plausible, but I'm skeptical because 1) they probably wouldn't know that pre-save, and 2) they do see their own changes post-save immediately, right? [13:27:05] but maybe the UI is different than I remember.. [13:30:25] Krinkle: I think the direct effect is that anon edits went way up (these were people whoe didn't see their own edit before) and that pulled up new registrations [13:31:00] (new registrations didn't actually go up visiblyy but that stat is dominated by 0-edit accounts and not really useful) [13:31:34] anon edits: https://stats.wikimedia.org/v2/#/hu.wikipedia.org/contributing/edits/normal|bar|all|editor_type~anonymous|monthly [13:32:15] active anons: https://stats.wikimedia.org/v2/#/hu.wikipedia.org/contributing/active-editors/normal|line|all|editor_type~anonymous|monthly [13:32:25] (that's almost 200% growth) [13:32:58] ah, interesting. [13:33:06] while anon editors have a session, we don't make use of it. [13:33:07] not sure of the exact mechanism but I can't think of any other change to attribute it to [13:33:23] Not that I'm particularly a fan of revision control, but if we wanted to, it's something we could fix. [13:33:40] yeah, that makes sense. forgot that anons don't see it post-save. [13:34:07] I think they see it immediately post-save [13:34:11] but not after that [13:34:15] hm.. ok [13:34:38] good point about sessions though [13:49:25] <_joe_> can I add active anons are exposing themselves to LE retaliation in many countries, and we shall protect the ip addresses from public view? [13:49:56] <_joe_> anons that edit the wikis get a session, thus see the site uncached by the edge [13:50:43] <_joe_> at least that's how it worked last I checked (3 years ago?) [13:52:16] <_joe_> oh you mean they don't appear on the page, cached or not, until reviewed [13:53:42] _joe_: Yeah, a FlaggedRev'ed page has a sticky revision that a reviewer promotes. For logged-in users (actually logged-in, not just having a session) we bypass that and always show latest. For anons we show the last approved rev. [13:54:32] <_joe_> lol [13:54:55] <_joe_> this invalidates half the assumptions I made about serving cached content to logged-in users [13:56:14] <_joe_> good to know :) [13:56:24] It's not incompatible with the direction I have in mind for simpler skin / better parser cache (aka great logged-in user perf, micro contribs, things like dark mode and per-anon-user language variant fragmentation without breaknig stuff) [13:58:30] But I do hope we can remove it, because it does add complexity. [13:59:11] <_joe_> yeah I was hoping we could have caching at the edge for a good part of what constitutes a page, independently of the logged-in status or not [14:02:54] you can, you'd just have to vary it on logged-in status [14:03:28] <_joe_> so we'd have to add another cookie signaling that [14:03:35] yeah [14:03:40] contents varying by user rights or status is unavoidable though, e.g. red links typically show more information to admins, "View history" shows more if you have auth for deleted revs, same for various other things. So it can't be entirely user agnostic. [14:03:44] <_joe_> and duplicate all cache because of flaggedrevs [14:03:46] <_joe_> :) [14:04:07] <_joe_> Krinkle: my idea involves edge-composition [14:04:33] making this easier to find, is part of why I want us to improve MW first and make the skin simpler, thus codifying what we allow it to vary by (right now we allow vary by everything and anything PHP can access one way or another). [14:04:40] <_joe_> no session -> default page. Session -> we compute the "exterior" of the page, and that includes the right version of the interior [14:04:44] revdel etc. is not really varying, it would have to make content uncacheable for privileged users [14:04:57] <_joe_> Krinkle: right, it makes sense [14:05:09] that or a different URL to actually expose hidden content, which we already do in most cases [14:05:09] This is why every Parsoid+skin edge service we've prototyped failed because it ignores 99% of MW. It's double the work if we try to approach it from the far end, instead of the inside. [14:05:52] Then once we have <20ms latency from MW backends for logged-in users we can "optimise" it by re-implementing what's left of the skin layer in a separate (insert fav lang) service at the edge with the parser cache query being HTTP instead or something like that. [14:06:20] as Wikipedia-scale optimisation for edge latencies [14:08:16] But yeah, basically I want to make it an option (perhaps off by default for third parties) to take away the ability for skin callbacks to access global state and do whatever. instead, requiring the output to be augmented with only logic that varies by the given parameters (and for MW to cache it by the hash of those params, so varying by anything externral would mean it shows up for random users). We can then see what our needs are, and what [14:08:17] the cost is of certain features, what their real requirements and needs are, and work with that. [14:08:39] I think there's a fair amount of "randomness" in some of the skin feautures that are unpredictble right now because they can be, not because they needed to be. [14:57:59] tgr: wanna roll that out in prod first, or ok to land in master as well? [14:59:00] Krinkle: let's go with prod, I'll give fixing master a shot later [14:59:06] k [14:59:23] tgr: going for it now, calendar looks clear [14:59:40] thanks! I have to run for a meeting [15:12:19] that was a quick meeting. [18:14:31] Reedy: think we're ready for a release of IPUtils? [18:15:30] My only outstanding question... Is whether to bring Antoine out as an author in composer.json with him being on the gpl header [18:21:48] the history is exciting [18:21:57] originally there was ProxyTools.php, a bunch of global functions created by Tim. then Hashar moved the IP related ones into a single class named IP (and added the GPL header) [18:22:28] then someone (Aaron IIRC) moved the remaining functions in ProxyTools (wfIsConfiguredProxy, etc) into GlobalFunctions, and then someone moved them into IP [18:22:53] Heh [18:22:56] and then later I moved them out of IP and into a new ProxyLookup [18:23:18] Ok, so if Antoine isn't specifically an author of it... I wouldn't worry about it too much [18:23:24] Release and strip from core then :) [18:25:02] I think a plurality of the code was written by Tim, even to this day [18:34:02] Should we maybe remove that @author tag then? :P [18:42:53] * xSavitar checks in [18:43:19] Thanking tgr and Krinkle for helping me fix T226448 [18:43:22] T226448: Fatal logged after renaming files: "LocalFile.php: Call to a member function purgeEverything() on boolean" - https://phabricator.wikimedia.org/T226448 [18:43:28] Much appreciated [18:46:03] Sorry about the mess! [18:49:32] xSavitar: It's not all your fault. ;) This is why we do peer code review [18:51:42] Yeah. Reedy, I feel bad inside when these kind of things happen to my patches :( [18:52:46] The patch was huge and for some reason, I mixed things up somewhere, I should be more careful next time [18:55:20] xSavitar: don't beat yourself up about it - that's why 'I iz borked the wikiz' t-shirt exists ;) [19:03:12] okay [19:06:47] xSavitar: stuff like that happens, especially with big refactoring patches where you can't realistically test all the affected code [19:07:37] ideally the unit tests would catch it, but our file handling code has little test coverage [19:07:48] As above, this is why we review code. So it's not just the developer that thinks it's ok, there's at least one other person that has agreed [19:08:04] Mistakes happen, we wouldn't move forward otherwise [19:08:45] to some extent that's what your patches (and dependency injection in general) are paving the road for, easier testability and eventually better coverage [19:10:21] so it's worth paying the cost of occasional accidents [19:17:48] * Krinkle uninstalls Babel locally because its Database class keeps coming up in quick open file [20:15:48] Is there a thing like DeferredUpdate, but executed while it's still possible to set a cookie? [20:22:06] MaxSem: isn't the cookie send as a header and thus before the output? That sounds wayyy too early and would need to sync on the output? [20:22:13] so hardly deferred [20:24:29] Considering that we cache all the output in OutputPage and mostly output it in one go at the end, it should still be possible to spit out cookies fairly late in the process - theoretically [20:24:44] Just not DeferredUpdate late :] [20:25:08] So I was asking if there's some mechanism already... [20:46:14] MaxSem: Pre-send DeferredUpdate, yes. [20:46:31] MaxSem: Also, db->onIdle or postCommit callback might also be relevant [20:46:49] so that it is cancelled if the DB transactino of the reuqest failed [20:46:57] There is a DeferrredUpdate utlity class just for that