[00:16:23] ugh, incomplete homegrown dependency injection means no constructor arguments can be other objects [00:17:15] in smash pig? [00:21:59] cwd yeah :( [00:22:24] constructor-parameters is always just pulled in with val(), which means only string and arrays [00:22:55] XenoRyet: any thoughts on dependency injection? [00:23:24] This thing just reminded me of some ways I've been going astray: https://www.mediawiki.org/wiki/Requests_for_comment/Dependency_injection [00:23:46] Let me read up a bit [00:23:51] like peppering SmashPig::Configuration access through the code [00:25:45] ideally there's a top level dispatcher so that control is passed downwards? [00:28:44] (PS1) Ejegg: Factory for payment providers [wikimedia/fundraising/SmashPig] - https://gerrit.wikimedia.org/r/338296 [00:29:32] cwd yeah, I figure the payment provider factory should be called from the entry point [00:30:16] and it should tell the payment provider class (aka gateway adapter, but broken up by method) all about the rest of the configuration [00:31:20] That patch ^^^ is a trivial implementation, basically just wrapping configuration::object [00:31:43] but it should be enough to make the DonationInterface tests look nice [00:32:17] or a bit nicer, anyway [00:32:42] tests will still need to know about SmashPig\Configuration [00:32:52] but at least they can use mocks [00:50:22] Fundraising-Backlog, FR-Smashpig: Reduce use of SmashPig\Core\Configuration where possible - https://phabricator.wikimedia.org/T158374#3035260 (Ejegg) [00:53:24] (PS1) Ejegg: Get some static Config access out of Authenticator [wikimedia/fundraising/SmashPig] - https://gerrit.wikimedia.org/r/338300 (https://phabricator.wikimedia.org/T158374) [01:25:42] (PS1) Ejegg: Move Ingenico base API wrapper functions to own class [wikimedia/fundraising/SmashPig] - https://gerrit.wikimedia.org/r/338303 (https://phabricator.wikimedia.org/T158374) [01:45:39] (PS1) Ejegg: Reduce Configuration use in BankPaymentProvider [wikimedia/fundraising/SmashPig] - https://gerrit.wikimedia.org/r/338306 (https://phabricator.wikimedia.org/T158374) [01:45:56] good night all! [14:42:06] Fundraising Sprint Deferential Equations, Fundraising-Backlog, FR-Ingenico, Unplanned-Sprint-Work: turn on 3DS for Sweden, Norway and Denmark - https://phabricator.wikimedia.org/T158357#3034779 (MBeat33) We're seeing lots of bank rejections today related to lack of 3DS - do we know when this will... [15:53:37] fr-tech anyone around to review a settings change? [15:54:07] sure [15:54:26] thanks cwd! I just pushed it to the settings repo [15:55:06] pretty sure the substance is correct, just looking for another set of eyes to detect syntax errors [15:56:09] yeah...i don't see any problems [15:56:43] if only you could lint php without php [15:56:51] Fundraising Sprint Deferential Equations, Fundraising-Backlog, FR-Ingenico, Unplanned-Sprint-Work: turn on 3DS for Sweden, Norway and Denmark - https://phabricator.wikimedia.org/T158357#3034779 (Ejegg) @MBeat33 I'll have it turned on within 15 minutes [15:59:10] thanks cwd [15:59:47] syntax highlighting in vim goes a long way towards assuring me [16:00:45] hmmm yes, i wonder how hard it would be to take that syntax file and have it spit out yes or no [16:18:33] !log updated 3DS rules for DK, SE, and NO [16:18:37] Logged the message at https://wikitech.wikimedia.org/wiki/Server_Admin_Log [16:19:12] Fundraising Sprint Deferential Equations, Fundraising-Backlog, FR-Ingenico, Unplanned-Sprint-Work: turn on 3DS for Sweden, Norway and Denmark - https://phabricator.wikimedia.org/T158357#3034779 (Ejegg) OK, this should be turned on [16:35:48] Fundraising Sprint Deferential Equations, Fundraising-Backlog, FR-Ingenico, Unplanned-Sprint-Work: turn on 3DS for Sweden, Norway and Denmark - https://phabricator.wikimedia.org/T158357#3036786 (MBeat33) Many thanks @Ejegg [17:56:17] ejegg: feel like helping me squash this resultswitcher thing? [17:56:42] cwd sure! [17:57:03] cracking it open now, will hop in the -talk shortly [17:57:24] sweet, i'm gonna migrate out to the office and will be there soon [18:00:21] fr-tech: The First Commandment for Technicians: [18:00:21] Beware the lightening that lurketh in the undischarged [18:00:21] capacitor, lest it cause thee to bounce upon thy buttocks in a most [18:00:21] untechnician-like manner. [18:00:21] -- discuss. [18:24:04] cwd I'm going to head down one flight to where I was working the other day [18:24:31] np! [18:29:33] (PS4) Cdentinger: WIP deprecate globalcollect resultswitcher [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/330635 [18:35:56] (PS5) Cdentinger: WIP deprecate globalcollect resultswitcher [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/330635 [18:36:38] (PS6) Cdentinger: WIP deprecate globalcollect resultswitcher [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/330635 [18:42:43] Fundraising Sprint Waiting for Godot, Fundraising-Backlog, Unplanned-Sprint-Work: missing utm_medium in the donation link - https://phabricator.wikimedia.org/T151951#2833097 (Ejegg) @DStrine this shouldn't need points as it was closed before fr-tech had to touch it [18:43:45] (PS7) Cdentinger: WIP deprecate globalcollect resultswitcher [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/330635 [18:50:31] (PS8) Cdentinger: WIP deprecate globalcollect resultswitcher [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/330635 [18:55:00] cwd that's looking pretty good [18:55:16] the only deleted stuff we haven't replicated is the test for 'multiple attempts to process' [18:55:33] I'll grep some logs to see if that was ever catching anything [18:57:00] thanks! [18:59:44] cwd huh, it caught 5 in all of last year [19:00:06] I wonder whether the GET_ORDERSTATUS results tell us that it's already been processed, though? [19:01:06] let's see, the orphan rectifier ought to be asking exactly that question [19:04:15] so, it just calls confirm_creditcard [19:04:43] but that damn convoluted function has a bunch of special cases for orphans [19:04:51] dang [19:04:58] i see it does call getorderstatus [19:06:04] right, then we map the returned STATUSID to an action [19:06:39] OK, I think we're good [19:06:58] let's see if we can actually smoke test this [19:10:05] rgh got a new kernel and the vbox module is missing [19:10:22] i think i must dkpg-reconfigure or something [19:12:53] o no! [19:13:25] guh [19:13:27] Error! Bad return status for module build on kernel: 4.9.0-1-amd64 (x86_64) [19:14:01] 4.9, huh? aren't you bleeding edge! [19:17:47] this is why i don't really like developing on my laptop [19:18:02] modern laptops need newer software than the servers i'm building for [19:18:59] i have ambitions about making some reasonably secure "cloud based" development environments for fr-tech [19:19:56] huh, wouldn't using them for this kind of smoke test put them in pci scope? [19:20:29] are you loading live creds? [19:20:45] ah, I guess the sandbox doesn't count [19:21:22] yeah, i think we could do *most* stuff in the clear, so if we put them behind some decent auth i wouldn't feel too scared [19:21:25] so first we'd need a way for fr-tech to run the maintenance scripts on prod for things like batch refunds [19:22:02] well we could continue to do some stuff from laptops in the mean time [19:22:45] oh good there's no longer a virtualbox candidate in stretch [19:23:01] :P [19:23:43] jessie just doesn't do it for you? [19:25:56] i find the support for modern laptops to be pretty lacking [19:26:23] the kernel is ancient and doesn't handle realtime stuff very well [19:26:32] ah bummer. [19:26:51] right, i was using it on a laptop from like 6 years ago [19:27:17] just about the right vintage for jessie [19:27:30] ok just had to add a repo, reinstalling [19:27:37] thanks oracle [19:28:22] looks liek this spammer scraped the staff page [19:28:27] :P [19:44:46] ok, got a test thing bounced back [19:44:56] but I got this error: Order/Session OID mismatch 1192275998/1192275998 [19:45:10] So... something in that if condition isn't quite right [19:45:47] cwd ^^ [19:46:34] ah cool, thanks [19:46:43] yeah the original code is strange and i don't understand it [19:48:16] ejegg: you think we can just get away with: $oid !== $session_oid ? [19:50:28] yeah, it's really silly to have that other thing [19:50:51] cool [19:50:55] ah right, the strpos returns 0 [19:51:14] were we explicitly comparing to false before? [19:51:50] (PS9) Cdentinger: WIP deprecate globalcollect resultswitcher [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/330635 [19:55:51] cwd seems to do the right things now! [19:56:09] awesome, thanks for testing that [19:56:13] just got vagrant up again [19:56:21] big install on slow internet [19:57:33] thank goodness it's not metered any more! [19:57:50] fundraising-tech-ops, DBA: fundraising database tuning - https://phabricator.wikimedia.org/T158446#3037278 (Jgreen) [20:00:21] fundraising-tech-ops, DBA: fundraising database tuning - https://phabricator.wikimedia.org/T158446#3037294 (Jgreen) 1) current, low load "show status" {F5676034} 2) "show variables" {F5676038} 3) my.cnf {F5676045} [20:00:37] (PS10) Ejegg: WIP deprecate globalcollect resultswitcher [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/330635 (owner: Cdentinger) [20:04:20] (CR) Ejegg: "Much simplified! One suggestion for comment enhancement, but I'd say this is ready to un-WIP" (1 comment) [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/330635 (owner: Cdentinger) [20:08:53] (PS11) Cdentinger: deprecate globalcollect resultswitcher [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/330635 [20:10:23] (PS2) AndyRussG: Add $wgCentralSelectedMobileBannerDispatcher global for mobile [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/338160 (https://phabricator.wikimedia.org/T154954) [20:10:57] cwd do you think you can actually write any tests for that? [20:11:19] (PS3) AndyRussG: Add $wgCentralSelectedMobileBannerDispatcher global for mobile [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/338160 (https://phabricator.wikimedia.org/T154954) [20:11:21] or is that TODO just a pipe dream? [20:12:00] (PS4) AndyRussG: Purge banner content from front-end cache on banner save [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/336237 (https://phabricator.wikimedia.org/T154954) [20:12:24] ejegg: heh...it does sound like a tough are to test [20:12:28] *area [20:12:38] yah, for real [20:12:47] but maybe as we move stuff to smash pig a path will become clear [20:13:03] more able to mock out http stuff [20:13:11] (PS12) Ejegg: deprecate globalcollect resultswitcher [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/330635 (owner: Cdentinger) [20:13:58] (CR) Ejegg: [C: 2] "Nice!" [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/330635 (owner: Cdentinger) [20:14:22] thanks! [20:15:06] one thing my personal search for reliable internet has taught me is that the numbers a connection is stamped with do not really represent how usable it is [20:15:21] speed is relative to a lot of things [20:15:36] (Merged) jenkins-bot: deprecate globalcollect resultswitcher [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/330635 (owner: Cdentinger) [20:15:58] just now i did two different tests, one reports upstream as <100kb and the other >4mb [20:16:03] you are what you measure [20:18:39] (CR) AndyRussG: Purge banner content from front-end cache on banner save (1 comment) [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/336237 (https://phabricator.wikimedia.org/T154954) (owner: AndyRussG) [20:19:46] fr-tech ^ banner purge patch now ready 4 review again! :D [20:19:53] (as well as new parent patch) [20:20:14] looking AndyRussG ! [20:24:18] ejegg: thx! [20:28:26] cwd if you get a chance can you look at https://gerrit.wikimedia.org/r/338296 ? [20:28:31] (PS1) Cdentinger: process payment before popping out of iframe [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/338410 [20:29:31] ejegg: i saw that, looks fine, doesn't address the issue of it getting called in strange places, but that's probably out of scope [20:29:37] this was more just for mockability? [20:30:33] i'm imagining e.g. that a top level router uses the factories and passes the stuff downwards [20:30:36] Fundraising Sprint Deferential Equations, Fundraising-Backlog, MediaWiki-extensions-CentralNotice: Central Notice banners slow to save - https://phabricator.wikimedia.org/T158084#3037330 (AndyRussG) Hi! Aaaarg, this is pretty awful. I tried saving two different banners. The first one took 2 minutes,... [20:33:24] cwd yeah, I decided that the silliness with getClassForGateway was not so good [20:33:59] cwd right, once all the gateway logic is in the PaymentProvider class, it'll be the GatewayPage that calls the factory [20:34:24] AndyRussG: all looking good so far, just going to try to smoke test [20:35:11] ejegg: ah cool thx! [20:35:16] (CR) Ejegg: [C: -1] "Maybe https://gerrit.wikimedia.org/r/338296 instead? On second thought, this is actually kinda gross." [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/338277 (https://phabricator.wikimedia.org/T128692) (owner: Ejegg) [20:35:30] ejegg: mebbe don't merge just yet, I also want to make sure I've smoketested the latest version of all the codepaths [20:35:34] thx! [20:42:37] k, will be patient [21:07:27] (CR) Cdentinger: [C: 2] Factory for payment providers [wikimedia/fundraising/SmashPig] - https://gerrit.wikimedia.org/r/338296 (owner: Ejegg) [21:07:49] thanks for the CR cwd! [21:08:29] youb et [21:08:42] s/b / b/ [21:09:08] (Abandoned) Ejegg: Use dependency injection to get BankPaymentProvider [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/338277 (https://phabricator.wikimedia.org/T128692) (owner: Ejegg) [21:10:28] (Abandoned) Ejegg: WIP look up iDEAL banks, cache in Memcache [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/336333 (https://phabricator.wikimedia.org/T128692) (owner: Ejegg) [21:14:26] ejegg: sorry i'm a little lost as how to get the url to the liberator [21:14:37] i think you had a solution? [21:14:50] cwd sure, let me see... [21:16:33] cwd oh dang, I was thinking of the setClientVariables function we have in the adapter [21:16:44] but that's called by the MakeGlobalVariablesScript hook [21:17:00] so hmm.... [21:17:19] we need to add to the js config on our own schedule [21:20:35] are you imagining it has a php variable in it that gets interpreted? [21:21:33] cwd ok, OutputPage has a function addJsConfigVars [21:22:16] cwd nope, we add them to a set of globals that the client-side js accesses with mw.config.get [21:22:27] ah cool [21:22:49] yeah, no messing around with in-page scripts interpolating across languages [21:23:22] thank goodness [21:23:43] :) I think we totally eradicated that from DI soon after killing RapidHTML [21:24:36] Now we just need to name the config var. LiberationDestination? [21:25:02] sounds good to me [21:25:08] :) [21:25:14] will this happen instead of ->redirect()? or in addition? [21:25:40] I'd like to replace all the ->redirect() if possible [21:25:53] only certain gateways will need the liberator [21:26:06] but it'll work for all of them [21:26:23] it gets added it gatewaypage now, are only the ones in need of liberation using that? [21:26:24] I guess a bit slower for the ones that don't need it. [21:26:38] cwd good question! [21:26:57] (Merged) jenkins-bot: Factory for payment providers [wikimedia/fundraising/SmashPig] - https://gerrit.wikimedia.org/r/338296 (owner: Ejegg) [21:27:26] k, going to do a final rework of the DonationInterface iDEAL lookup patch [21:28:41] cwd so yeah, if it's not too complicated to select the liberator only when isReturnFramed() [21:28:51] and still redirect otherwise, that might be optimal [21:29:29] ah yeah, i think that should be easy enough [21:29:34] but if it's too messy, top.document.href='blah' will still work [21:37:24] (PS1) Ejegg: Make static method not abstract [wikimedia/fundraising/SmashPig] - https://gerrit.wikimedia.org/r/338443 [21:37:35] fr-tech trivial thing to stop 'tests failed' spam from scrutinizer ^^^ [21:39:09] and by fr-tech I guess I mean cwd :P [21:39:34] (not picking on you, just lamenting the emptiness) [21:40:47] heh, yeah it's a ghost town nowadays [21:43:20] ejegg: will stuff i add to js config be automagically namespaced as wgDonationInterface? [21:44:53] cwd nope, it comes out just like it goes in [21:45:06] see Amazon adapter's setClientVariables [21:45:11] and their use in amazon.js [21:46:50] cool, ty [21:50:24] (PS1) Ejegg: WIP: getAllValidForms uses isValidForm [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/338458 (https://phabricator.wikimedia.org/T136254) [21:54:10] (CR) Cdentinger: [C: 2] Make static method not abstract [wikimedia/fundraising/SmashPig] - https://gerrit.wikimedia.org/r/338443 (owner: Ejegg) [21:54:30] thanks again [21:55:29] (Merged) jenkins-bot: Make static method not abstract [wikimedia/fundraising/SmashPig] - https://gerrit.wikimedia.org/r/338443 (owner: Ejegg) [21:55:38] fr-tech dstrine-away btw also forgot to mention in standup that I started looking at T158084. It seems awful. My first suspicion is that it's because of some extra message cache processing that is in a deferred job that runs before the http response is sent to the client. Was planning on seeing if it happens on the beta cluster too, then making a patch with some instrumentation to push only [21:55:39] T158084: Central Notice banners slow to save - https://phabricator.wikimedia.org/T158084 [21:55:40] there, to see where the bottlneck is... does that make sense? thx!!! [21:56:07] (I'm sortof but should get backskrillll...) [21:57:07] AndyRussG|sortof: if it is reproducable on the beta cluster, an instrumentation patch for debug definitely makes sense [21:57:52] AndyRussG|sortof: "awful" as in a bad bug affecting other things.. or awful as in really annoying to work on? [21:59:08] ejegg: it seems like this approach would make that function's definition dependent on both isReturnFramed and whether we want to show success or failure [22:00:47] cwd I guess I was thinking that the displayFailPage and displayThankYouPage methods would switch on isReturnFramed [22:01:32] could even override $this->redirect to take isReturnFramed into account, if you want to get really funky! [22:01:39] but wouldn't they have to redefine setClientVars? [22:02:05] cwd oh, I found a think you can call from not-hook-world [22:02:15] should i call addJsConfigVars directly? [22:02:30] OutputPage::addJsConfigVars [22:02:43] cwd Sure, it's a public method! [22:02:58] cool cool yeah, i think i might just override redirect in that case [22:03:04] nice :) [22:06:26] (CR) jerkins-bot: [V: -1] WIP: getAllValidForms uses isValidForm [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/338458 (https://phabricator.wikimedia.org/T136254) (owner: Ejegg) [22:21:21] (PS2) Cdentinger: process payment before popping out of iframe [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/338410 (https://phabricator.wikimedia.org/T153972) [22:24:15] (CR) jerkins-bot: [V: -1] process payment before popping out of iframe [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/338410 (https://phabricator.wikimedia.org/T153972) (owner: Cdentinger) [22:24:35] frankly i'm glad that failed [22:24:42] our test coverage in this area is not stellar [22:25:00] yeah, page level is tough [22:25:18] heh damn it [22:25:22] it's that random failure [22:26:16] (CR) Cdentinger: "recheck" [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/338410 (https://phabricator.wikimedia.org/T153972) (owner: Cdentinger) [22:28:23] (CR) jerkins-bot: [V: -1] process payment before popping out of iframe [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/338410 (https://phabricator.wikimedia.org/T153972) (owner: Cdentinger) [22:30:16] strange [22:32:14] https://integration.wikimedia.org/ci/job/npm-node-6-jessie/5627/console [22:32:23] maybe it only checks files if they change? [22:32:54] (PS3) Cdentinger: process payment before popping out of iframe [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/338410 (https://phabricator.wikimedia.org/T153972) [22:33:48] weird indeed [22:35:11] ejegg: looks like cruft so i removed it, unless that's some sort of trick? [22:36:11] (PS4) Ejegg: Look up iDEAL banks, provide PSR6 to SmashPig [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/337420 (https://phabricator.wikimedia.org/T128692) [22:37:07] sounds good! [22:37:43] ejegg: do we need to be coming from a wmf ip to test ingenico at all? [22:39:43] cwd yep [22:39:57] the old VPN still works till the end of the month, though! [22:41:17] does not auth for some reason [22:41:24] probably easier to just get on the new one [22:43:48] long as you've got the right yubikey version! [22:44:18] i have a 4 laying around somewhere but i might have hacked it to death [22:45:21] ejegg: any chance you'd like to try that change on the vpn and see if it magically works first try which it won't? [22:47:01] ejegg: your yubikey wont' work with this or it's policy that they are separate? [22:47:41] needs to be a yubikey 4 [22:47:53] mine's a 2 [22:48:36] do you know why it has to be 4? looks like they just use the otp feature, same as the old ones [22:48:52] ccccccevfibtrlecbvfrrjtnlrerfruhegluuvcbujve [22:49:41] i have this 4 that also has a pgp key loaded on it [22:50:01] the interface is bananas [22:50:11] I wish I knew the details! [22:50:45] So, I've got to relocate, but I'll try that latest patch out in like a half hour [22:51:27] thanks! no rush [23:06:25] dstrine-away: it's not affecting site visitors directly, but it's so slow I think it probably does affect fr-online's workflow :( [23:07:12] AndyRussG|sortof: yeah we knew it was pretty annoying. I thought you had more bad news in some other category [23:07:45] dstrine: no, just that it was even slower for me than what they reported [23:07:53] oh ok [23:08:13] With luck it'll be easyish-ly-fixable [23:08:13] well still sounds like the right thing to look at right now [23:08:20] yep [23:08:21] cool [23:29:21] (PS5) Ejegg: Look up iDEAL banks, provide PSR6 to SmashPig [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/337420 (https://phabricator.wikimedia.org/T128692) [23:33:50] (CR) jerkins-bot: [V: -1] Look up iDEAL banks, provide PSR6 to SmashPig [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/337420 (https://phabricator.wikimedia.org/T128692) (owner: Ejegg) [23:37:27] cwd I'm getting the thank you page inside the frame [23:37:38] trying in other browser in case of cached js [23:38:06] nuts [23:38:10] i'm getting my vpn set up [23:39:11] nope, same result in other browser [23:43:53] i'm sure the js part will take some bashing [23:47:45] have a great weekend everyone!