[00:10:55] (PS1) Ssmith: Fix remote thing [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/144859 [00:13:08] (CR) Ssmith: [C: 2] Fix remote thing [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/144859 (owner: Ssmith) [15:31:27] (PS1) Ejegg: Add dates to banner hide cookies (server side) [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/144991 [16:56:50] ejegg: re 144991, is there a reason u want to avoid json in the cookie? [16:57:26] ejegg: IMO our job will be easier in the future if there are explicit semantics, or a major API revision level at least [16:58:17] you saw https://gerrit.wikimedia.org/r/#/c/143232/ ? [17:03:38] rats, I got netsplit if u replied [17:04:08] internet at the office is a hot mess [17:04:21] really? I'm on a wire, this is unusual [17:05:34] ejegg: looks like we both got netsplit, if u replied [17:05:56] awight: only avoided json for size, but could use it instead [17:06:06] yeah the network upstairs just went totally nuts for a minute.. but it's back [17:06:19] mwalker said legal decided no need for fuzzing, so I didn't include that [17:07:35] ejegg: good point, keep the latency down huh. How about a major API rev? Or you want to just rely on the exploded element count? [17:08:01] my new favourite word of the day: Backpfeifengesicht, in german it means "a face badly in need of a fist" [17:08:03] ejegg: my only thought here is that we may want to experiment with multiple cookie styles in the future. [17:08:19] count, or names. [17:08:24] mwalker hahahaha [17:08:31] but api rev would make it easier [17:08:48] ejegg: names, as in the cookie's suffix? [17:09:25] no, element names in the JSON [17:10:01] meganhernandez cookies [17:10:09] hehe ^^ [17:11:04] ejegg: ok any way u see fit, I just want to simplify future cookie migrations, so we can simply deploy code that handles multiple formats of cookie [17:11:23] OK, let's use JSON, but keep element names short, like {v:1,c:12345678,e:13456789} [17:11:25] ejegg: what you wrote so far will totally work for that, it's just a bit obfuscated. [17:11:38] ejegg: sure (though I think double-quotes are part of the spec) [17:12:09] ...but still check for plain old 'hide' cookies in this first deployment [17:12:13] mwalker: any words to share about your experience using pipe-delimited data structures for bucketing? [17:12:42] * awight finishes atypical cup of coffee WHEEEEE [17:14:05] mwalker: from my outsider's perspective, that looked like it became cumbersome as you added features [17:14:22] ya; it was horrible [17:14:24] use json [17:14:37] ok, i'm convinced! [17:14:38] it worked... but... [17:14:39] * awight slips mwalker two bits [17:14:47] ooooh [17:14:53] * mwalker puts it on top of his change stack [17:14:59] shiny... [17:16:19] ejegg: u might look at the existing cookie payload before deciding to abbreviate, even... [17:16:45] -> centralnotice_hide_fundraising=hide; metawikiUserID=2043420; metawikiUserName=Awight+%28WMF%29; centralnotice_bannercount_fr12=0; centralnotice_bannercount_fr12-wait=0%7C0%7C0; centralnotice_only2times_tou=2; centralnotice_only2times_tou-wait=49%7C1405709104645%7C0; centralauth_LoggedOut=1404168142; centralauth_User=Awight+%28WMF%29; centralnotice_bannercount_wikimania14=1; centralnotice_bannercount_wikimania14-wait=37%7C1406917847995%7C0; [17:16:53] * awight sweats a little bit about there being passwords [17:17:54] jeez, with every single request... [17:37:49] (Abandoned) Awight: WIP (FR #1703) make banner hide cookie more forward-compatible [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/143232 (owner: Awight) [17:48:31] (PS2) Ejegg: Add dates to banner-hiding cookies [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/144853 [17:58:04] (PS2) Ejegg: Add dates to banner-hiding cookies (server side) [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/144991 [18:12:00] (PS1) Ejegg: Add a 'No valid form' RapidHtml form [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/145028 [18:38:24] Uh, oh. enwiki/Main_Page BannerRandom: mw.centralNotice.insertBanner( false /* due to internal exception */ ); [18:39:25] that's fine if there are no banners [18:39:30] or if not all slots are filled [18:39:40] mwalker: that would be... the wrong message tho [18:39:56] no; we throw an emptybannerexception in those cases [18:39:59] it's an annoying bug [18:40:03] but it's what happens [18:40:28] mwalker: no, emptybannerexception turns into simply "false" [18:40:52] are you absolutely sure? because I've traced that behaviour into empty slots before [18:41:48] mwalker: yes, I'm looking at SpecialBannerLoader now [18:47:26] mwalker: looks like my LDAP users are all lacking the wmf group :-/ Do you have a minute to look at logstash for me? [18:52:01] awight i'm not sure how much crossover is, but while you're working on imports, there are also 2 other new ones MG wants... [18:52:06] https://trello.com/card/card-1497/5345989ac6f014b5534ce4c8/1497 [18:52:09] https://trello.com/card/card-1497/5345989ac6f014b5534ce4c8/1576 [18:52:28] dang don't click on that [18:52:41] * awight blinders self [18:52:44] these: [18:52:46] https://wikimedia.mingle.thoughtworks.com/projects/online_fundraiser/cards/1497 [18:52:50] https://wikimedia.mingle.thoughtworks.com/projects/online_fundraiser/cards/1576 [18:52:59] i mean, you can, it just leads to nonsense :P [18:53:08] atgo: yep, thx for the heads-up. no overlap though [18:53:13] ahh alrighty [18:58:33] awight lunch? [18:58:37] FR lunch, that is [18:59:23] atgo: wooohooo [19:25:02] (CR) Siebrand: [C: -1] "i18n/L10n reviewed." (1 comment) [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/145028 (owner: Ejegg) [19:40:22] (PS2) Ejegg: Add a 'No valid form' RapidHtml form [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/145028 [20:08:12] (CR) Awight: [C: 1] "Great!" (2 comments) [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/144991 (owner: Ejegg) [20:11:50] awight: I added the expires date to the cookie value to make it available to JS. The thinking was to let override logic / mixins use both that and the creation date to change the hide/show decision [20:12:18] ejegg: oh, interesting [20:12:25] speaking of which, we'll want to let mixins force banners, not just veto them. [20:12:46] I think the expiration date should be a dependent variable though, not drive logic [20:13:10] you're right about forcing banners. mwalker had a plan for that, which I was fighting as being overly complicated [20:13:16] but it was better than what we have. [20:13:50] teal dear, we were talking about failing over to the next available banner when the original die roll would serve a banner that wants to hide itself. [20:13:56] what is the use case you're imagining? [20:14:23] dependent variable... so store created stamp and intended duration? [20:15:56] or store created stamp and reason (donated/closed banner), and read the current respective durations from mw.config? [20:16:54] ejegg: yes the latter is what I was thinking--store the root reason, and then we can recalculate expiry later [20:18:43] so the mixin could fiddle with the config values for per-campaign overrides [20:19:32] ejegg: and when would we force a banner? What would the default behavior have been? [20:20:11] if the original expiration was for a year, but we wanted to move it earlier by a month [20:20:44] right now, if the mixin returns true, we still want to check the cookie for hide/show [20:22:37] i like the idea of storing the reason though. [20:24:59] ejegg: wait, u lost me. Perhaps sleep dep. When is this hypothetical request? during that 11-12 month window? [20:25:46] donor hits thank you page August 2014, gets 12 month hide cookie [20:26:34] k [20:26:38] FR decides July 2015 is a crucial campaign, and wants to show banners to ppl who have donated as recently as 10 months ago [20:26:44] aha yes ok [20:27:00] so the mixin should simply not hide, rather than new forcing behavior, right? [20:27:19] assuming that we have already replaced inline hide code with a mixin [20:27:48] I'd be wary of replacing all hide functionality with a mixin [20:27:59] oh yeah? why? [20:28:08] if we're putting the reason in, we should at least leave the 'closed banner' check inline [20:28:09] cos it might not work? :p [20:28:18] hmmmm [20:28:27] cos you'd have to enable the 'respect close banner button' mixin everywhere [20:28:36] or piss people off [20:28:53] OIC, that hide code is in the ext.bannerController module [20:29:03] by "inline" i meant, included in the banner payload [20:29:13] ah, yeah [20:29:40] and that check will still hide the banner in July 2015 if the mixin doesn't veto [20:29:41] but, fwiw we definitely have the option of enabling a mixin by default. Would not in this case though, I agree w/ u. [20:29:45] oh [20:29:47] hrmph [20:29:59] I hadn't noticed this [20:30:22] If we go the dependent route, the mixin could change the mw.config.donorPesterDelay [20:30:28] and affect the subsequent calculation [20:31:03] I would like to get that code out of the controller, actually, or at least factor into its own ResourceLoader module [20:31:10] whatev, I suppose [20:31:40] I am not a fan of the impressionResultData hook, but have not come up with a better alternative [20:31:58] oh, to alter the resultData? [20:32:11] yah that seems very fragile [20:32:14] I was still thinking of the preload [20:32:34] fwiw, this would be a great time to rethink the preload mechanism... [20:33:41] one major shortcoming is that it does not allow us to do the failover to another banner thing [20:33:41] mwalker and gwicke were theorizing that we could send all potential banners in the response, and then client-side code chooses from among them [20:34:32] I still think that's a horrible idea actually [20:34:48] I'm significantly more of a fan of the proxy approach (not that it exists...) [20:34:56] proxy approach? [20:35:15] aye; the idea is to replace Special:BannerRandom with a proxy service [20:35:35] this would have several benefits, a big one being that we would no longer explode the cache [20:36:09] but... since the idea was also to have it run in Node.JS; it could also run preloading stuff on the server and be able to make a decision about what, if any, banner it needed to send [20:36:17] (e.g. dont send a banner if it's just going to hide) [20:36:24] currenly exploding b/c of the 30 slots X all other variables we send to BannerRandom? [20:36:30] *nods* [20:36:54] https://www.mediawiki.org/wiki/Requests_for_comment/CentralNotice_Caching_Overhaul_-_Frontend_Proxy [20:37:00] I think mwalker's napkin calculation was order of 10^7 combinations [20:37:08] wheee! [20:37:37] ejegg: I think... you just inherited that [20:37:51] * awight fingers nose [20:38:08] erm... what fun, I think? [20:38:39] There is a related problem in CentralNotice : special/SpecialGlobalAllocation.php [20:39:15] * awight lays off messing with ejegg's cookie implementation. [20:39:28] Please feel free to bounce ideas though [20:39:53] for now, let's just get some more useful values into the cookies [20:40:10] I think I communicated the important part: all that cookie/hide code should be considered fragile, but also in need of improvement. [20:54:46] awight: is there a generic way to specify which server-side config vars to pass into mw.config? [20:55:34] ejegg: yes. efResourceLoaderGetConfigVars [20:55:41] thanks! [20:55:55] hah, that must have been a annoying surprise, that your new var was not available [20:56:19] No, I figured we didn't dump all the globals in there! [20:56:33] eggsactly [20:56:41] just didn't see it in the .modules.php and wasn't sure where to look next [21:27:46] (PS1) Ssmith: add to README [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/145133 [21:31:30] (PS2) Ssmith: add to README [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/145133 [22:33:49] (PS3) Ssmith: add to README [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/145133 [23:36:45] K4-713 awight i don't really know how to split up the weird contribution card, now that i look at it. #1573 [23:36:59] hmm [23:37:42] So, we need a spike to see if we lost any more like this. [23:38:23] Depending on how that turns out, we may need another card to re-run those transactions from... wherever, once we plug the hole. [23:39:22] And, a card to fix the problem that awight found with VEF. [23:39:47] atgo: Make sense? Or did we want to break it down farther? [23:39:56] that's all good to me, yeah [23:40:35] And, I suppose, one card to follow up with GC to find out why the donor was charged in USD? [23:42:12] Or, maybe slightly more zoomed out than that, a spike to make sure we sent the transaction in VEF, and pull the XML to send to GC for followup. [23:42:35] K4-713: imo the most important card is "determine why the exception caused a silent failure to insert rather than a failmail" [23:42:55] I was assuming that would go with the "fix it" one. [23:43:31] awight: Do you want that to be a separate thing? [23:46:02] hmm ok. [23:46:33] i'm going to revisit these splitting up cards tomorrow [23:48:10] (PS1) Ssmith: Merge branch 'master' into HEAD [wikimedia/fundraising/dash] (deployment) - https://gerrit.wikimedia.org/r/145167 [23:48:15] (CR) jenkins-bot: [V: -1] Merge branch 'master' into HEAD [wikimedia/fundraising/dash] (deployment) - https://gerrit.wikimedia.org/r/145167 (owner: Ssmith) [23:52:19] (CR) Ssmith: [C: 2 V: 2] Merge branch 'master' into HEAD [wikimedia/fundraising/dash] (deployment) - https://gerrit.wikimedia.org/r/145167 (owner: Ssmith) [23:52:22] (CR) jenkins-bot: [V: -1] Merge branch 'master' into HEAD [wikimedia/fundraising/dash] (deployment) - https://gerrit.wikimedia.org/r/145167 (owner: Ssmith) [23:57:01] (CR) Awight: [C: 2 V: 2] "woohoo!" [wikimedia/fundraising/dash] - https://gerrit.wikimedia.org/r/145133 (owner: Ssmith)