[17:55:52] does anyone have a comprehensive diagram of redirects and how they work on 1. client-side, 2. varnish layer, 3. PHP server ? [17:57:10] milimetric: wikitext redirects? [17:57:57] MatmaRex: I'm kind of asking about *all* redirects. Like has someone drawn up a comprehensive version of all ways responses can redirect a user from the original address? [17:58:12] like both Barack Obama -> Barack_Obama and the wikitext redirect pages [18:00:13] hm, okay. all i can tell you is that wikitext redirects are not actually HTTP redirects, they just display the target page at the redirecting page. :) (and there's a bit of JS to update the displayed URL afterwards) [18:02:11] the logic for redirecting to canonical titles is in PHP code, though, in MediaWiki::tryNormaliseRedirect(). i have no idea what happens before then. [18:16:42] cool, we were hoping against hope there might be some awesome diagram somewhere that explained it all. I think we'll have to make said awesome diagram :) [18:16:54] milimetric: https://phabricator.wikimedia.org/T104755 [18:26:14] thx ori, I guess we all agree the logic for redirects is kind of crazy. We'll still have to draw up a diagram of all of it so we can make our page title normalization better and our stats cleaner. But I'm glad people are thinking about cleaning this up and I support this task [18:35:05] tgr|away, anomie: https://gerrit.wikimedia.org/r/254896 , https://gerrit.wikimedia.org/r/254897 round off my "leave it better than you found it" dabble with securepoll [18:35:36] * anomie hears "securepoll" and hides [18:35:45] heh [18:43:12] ori: you realize this leaves you in the "oh! ori touched it last" hot seat until you trick the next person into fixing something for it [18:44:15] that already happened [18:44:51] James A. asked me to help now because I helped with the board election scripts last May [18:44:58] doh [18:47:11] thanks bd808 [18:47:31] the next one is a bit more complex to review [18:48:14] bd808: it's not, really -- the diff is misleading. what it is highlighting as differences are actually changes that i made to arbcomlist.php and which tgr and aaron reviewed [18:48:41] https://gerrit.wikimedia.org/r/#/c/254375/ primarily [18:49:36] but anyways, no rush, these won't get used until the next elections [18:54:36] thanks! [18:54:42] yw [18:55:25] bd808: thanks (again) for https://wikitech.wikimedia.org/wiki/Help:Self-hosted_puppetmaster too, using that now to get a multi-dc test setup in labs [19:00:00] I'd love to take credit for that page but I don't think I've ever edited it. I did write a lot of stuff about how we are using the shared puppetmaster and cherry-picks for the deployment-prep project -- https://wikitech.wikimedia.org/wiki/Nova_Resource:Deployment-prep/How_code_is_updated#Puppet_and_Salt [20:25:49] legoktm: hey, I thought about updating a few composer.json files for mediawiki 1.26 release [20:26:01] legoktm: basically adding name/homepage/type (for installer) and license https://gerrit.wikimedia.org/r/#/c/254918/1/composer.json,unified [20:26:43] hashar: I don't think we want to encourage installation of extensions via composer [20:27:16] wasn't it the whole point of your RFC? :D [20:28:03] no? [20:28:28] I will stick to simply adding require-dev and the test scripts so :-) [20:29:37] legoktm: +1 [20:29:51] I think we have generally come to the conclusion that installing extensions via Composer has a narrow use case (single wiki) and could cause problems if adopted widely due to difficulties for wiki farms [20:30:38] Also the way that Composer installed extensions register/bootstrap seems to have performance implications [20:33:02] ok ok :-) [20:33:17] maybe I will end up extending the registration system to introduce entry point [20:33:26] and chain out to composer / npm as needed [21:58:32] csteipp, bd808, tgr|away: FYI, https://www.mediawiki.org/wiki/User:Anomie/SessionManager_and_AuthManager. Feel free to edit mercilessly. [22:04:07] Question: Using the TitleMoveComplete hook, I'm trying to get the revision ID before, and after a move. $title->getLatestRevID() and $newtitle->getLatestRevID() are giving me (new_revision+1) and old_revision respectively though, what am I doing wrong? [22:04:28] Looking at the revision table, it looks like the move itself creates two new entries [22:21:05] https://phabricator.wikimedia.org/T118391#1826742 wow, this is sick [22:21:17] magically duplicating reference blocks [22:24:23] err, where is AuthManager implemented? [22:24:52] MaxSem: in gerrit! [22:25:18] link or it didn't happen! ;) [22:25:50] MaxSem: https://gerrit.wikimedia.org/r/#/c/195297/ [22:26:31] so why the hell are we deprecating code that has no replacement yet? :P [22:26:54] which part MaxSem ? [22:27:21] User::setPassword(), for example [22:28:39] oh, just for fun [22:28:50] * MaxSem bites bd808 [22:32:00] That particular one is a non-warning deprecation because AuthManager isn't done yet [22:32:12] it even says that in the commit that introduced it [22:32:46] https://github.com/wikimedia/mediawiki/commit/3d0b4fea3dfb94610be0f0e9d8ff1cb24f106707 [23:19:05] bd808, what's the current best way to make user password invalid? [23:19:18] replace it with a long random string? [23:21:33] MaxSem: I actually don't know. tgr|away or anomie probably do know [23:21:50] * bd808 just pretends to understand what his team is working on [23:23:04] MaxSem: create an invalid password object, convert to string, and write that to DB [23:23:52] write as in directly? cuz feeding it to ssetPassword() results in a bunch of warnings [23:24:04] PasswordFactory::newInvalidPassword [23:24:46] Warning: strlen() expects parameter 1 to be string, object given in /vagrant/mediawiki/includes/password/PasswordPolicyChecks.php on line 38 [23:24:46] Warning: strlen() expects parameter 1 to be string, object given in /vagrant/mediawiki/includes/password/PasswordPolicyChecks.php on line 38 [23:24:46] Warning: strlen() expects parameter 1 to be string, object given in /vagrant/mediawiki/includes/password/PasswordPolicyChecks.php on line 38 [23:25:12] yeah, that method expects a string [23:25:36] and the canonical invalid password is the empty string and that would probably fail the policy checks [23:26:04] most use cases are covered by User::newSystemUser [23:26:08] and if you convert it to string, Fatal error: Uncaught exception 'PasswordError' with message '* Passwords must be at least 7 characters. [23:27:22] if newSystemUser does not work for you, just see what it does internally and do that [23:27:37] but that's not future-proof; newSystemUser itself is [23:27:37] it pokes the empt password directly into the db [23:27:47] boo [23:28:37] Can't we document parameter by using documentation comments? Array keys as https://gerrit.wikimedia.org/r/#/c/249679/4 seems like it would cause more problems than it solves. It makes code look like order doesn't matter when it does. For what? To save one line of code? [23:32:05] Krinkle: I don't get what you mean [23:32:17] that patch does not change the method signature at all [23:32:59] tgr: I assume your intent is to change code from object => array( // foo 'First', // bar 'Second ) to array( 'foo' => 'First', 'bar' => 'Second') [23:33:24] yes [23:33:24] When in fact the factory in question does not take named arguments, does not take an options object, but positional arguments [23:33:39] What does this gain? [23:33:39] it makes it easier for extensions to change configuration arrays [23:33:58] It sounds to me like every use case beyond comments will not work. [23:34:08] because the position is significant. [23:35:04] If some other unrelated interface supports an array with named keys, it will have to translate those keys into the correct position still. [23:35:06] So it doesn't win anything? [23:35:20] It just makes it confusing and will probably cause a logging outage or two at some point in the future. [23:35:25] true, the extensions are messing with array keys one level higher [23:36:04] I ran into this problem with some code but don't remember where exactly [23:36:06] I like named keys, don't get me wrong. [23:36:13] it might have used keys for documentation [23:36:43] But if it's only for the purpose of inline comments, I think we're better off using actual inline comments [23:37:22] it was https://gerrit.wikimedia.org/r/#/c/195297/64/includes/DefaultSettings.php I believe [23:39:32] anomie: do you remember if those 'authoritative' keys need to be manipulated or are for documentation only? [23:40:06] arg0 there is an array [23:40:17] args => array( array() ) [23:40:36] oh yeah, that's the $options parameter [23:41:03] sorry, no idea what code it was then [23:41:15] maybe I just imagined it [23:42:10] I don't know that I saw a great benefit in 249679 but I did'nt see any harm in it either. If you disagree Krinkle we can hash it out now before there is any usage [23:42:54] I spotted it and felt it was an anti-pattern, but I like the merits, so making sure there isn't a stable use case behind it . [23:43:06] But it seems like there isn't, so yeah, then we better avoid it before adoption. [23:43:27] in any case ObjectFactory should handle associative arrays better than throwing an error about invalid offset [23:43:50] if that's by throwing an informative exception, that's fine by me [23:44:00] that's what the splat operator would do (or I guess an exception about non-numeric keys) [23:44:16] Unless we make ObjectFactory support associative arrays (and passes them onto the constructor, which would take array $options as parameter, like with BagOStuff and ObjectCache classes do already) [23:44:45] it does that already as a positional arg [23:44:47] in this case the keys at https://gerrit.wikimedia.org/r/#/c/195297/64/includes/DefaultSettings.php do seem confusing. It sets a key that isn't actually used. If you have more than one, it'll be very easy to remove one and break it. [23:45:00] there is no way to support named arguments without using reflection and reflection is terribly slow [23:46:20] bd808 is right about that diff, it passes array( 'authritative' => true ) as an argument to the new object, so that's unrelated, I just confused things [23:46:30] Oh, well spotted. Yeah the case at 195297 will work then because it's the options parameter, not positional [23:47:07] I think in that case an error for unknown offset would probably be valuable. [23:47:23] since it could easily mask someone accidentally not nesting the array [23:56:28] Krinkle: curiously, call_user_func_array() does what tgr's patch does [23:56:39] https://3v4l.org/kb7dt [23:56:42] PHP does it implicitly yes. [23:56:46] Because it's flattened [23:56:59] But that doesn't mean it's a pattern or interface we should encourage [23:58:53] hhvm also does that flattening for the splat operator, but PHP 5.6+ raises a fatal error -- https://3v4l.org/O1BY2