[00:00:04] we have submodules to avoid checking the same code in multiple repos, but under vendor we're already duplicating tons of 3rd party repos [00:00:07] well, I was sort of okay with the situation in which composer managed everything correctly, except for the submodules that we wanted to manage manually. [00:00:26] The 3rd-party stuff seems fine [00:00:50] submodules weren't to avoid bloat, IMO, they were to make it simple to upgrade our libs [00:00:53] mmm [00:00:58] I think I'm starting to agree with you, though [00:01:03] actually, yeah. [00:01:08] please do! [00:01:12] but to update DonationInterface, I have to move the submodule out of the way, composer install the new version (to update autoload and .lock), then move the submodule back into place and update it to the new revision [00:01:30] heh, you convinced yrself before i could type my argument [00:02:00] I was imagining we wanted to work in the vendor/wikimedia/* dirs but of course not. We do the merge square dance elsewhere, and pull down the final product. [00:02:37] that totally works. [00:02:52] d'oh, I think I have to push one more change to DI's composer.json to make the WMFFramework happen [00:03:09] I thought that was already done? [00:03:25] ehh, no, I'll require the alias switcher file where we need it in crm [00:13:57] AndyRussG: why do the fixtures have to be removed while u refactor? [00:14:20] Also, do you have a use case in mind for the mock globals? [00:15:34] awight: well they were changing a lot, so I wanted to look at a clean slate for a while, but they'll all go back before this patch is out of WIP [00:15:42] The mock globals' use case is in there already [00:16:10] number of buckets? It's the same for all tests tho [00:16:16] It's cos we have to test all the available buckets and provide expected results for all [00:16:47] I'm fine with setting the globals, I only ask cos I'm gatekeeping a bit wrt complexity that might not get used [00:16:49] Shouldn't depend on the actual config value, easier if it's the same for all fixtures [00:17:05] No it's already used there... grep around and ull find it [00:17:07] right. But why not just set directly, outside the fixture data? [00:17:30] because it's tied to the fixture data. That's the value needed by this specific fixture data [00:17:37] Change it and you have to change the fixture data [00:17:55] do you plan to add fixture data that requires different settings? [00:18:15] Well, not yet :) [00:18:21] But we might! [00:18:36] And it's not much complexity added for a gain in clarity of what goes with what [00:18:47] * AndyRussG is sure it's all very very clear [00:18:51] jk [00:18:52] hehehe [00:19:08] ok no worries, if it might get used then it's totally rad [00:19:32] yeah grep and you'll see it, in both phpunit and qunt [00:19:44] qunit [00:20:47] O_O [00:21:03] I'll stop bothering u now :p [00:21:40] I do see it being read and used to set globals--my doubt was only whether we would be actually changing this per fixture [00:21:57] what tipped me off was that the key is at the top level of the file, rather than under the tests... [00:28:27] arrrggghh. vendor gitdir royally snafu'ed. Let's see if I can at least make it work the wrong way [00:32:04] (CR) Awight: "Great idea!" (10 comments) [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/188687 (owner: AndyRussG) [00:39:11] (PS1) Ejegg: Update DonationInterface and Smashpig [wikimedia/fundraising/crm/vendor] - https://gerrit.wikimedia.org/r/190393 [00:39:32] aww [00:40:14] no freaking clue how to actually fix the submodules [00:40:43] I mean, we can all rm -rf and clone fresh if it comes to that [00:41:00] hmm [00:41:07] well, let me try one more time [00:42:45] also: word on the street is that Linus hates submodules, hence the awful and broken implementation in git. [00:43:34] well, the linux kernel probably isn't checking in a whole lot of third party libs [00:44:08] I'm remembering a terrible, vague snippet--you need to use a weirdly bleeding-edge version of git in order to remove submoduleas [00:44:48] the version I've got /does/ manipulate .gitmodules for you when you git rm the path [00:44:58] what version? [00:45:08] I have 1.7.10.4 [00:45:11] 2.1.0 [00:45:18] oh. u got it then [00:45:23] weirdo ;) [00:45:41] the 3-d version [00:45:45] but there was still stuff in .git/modules [00:45:53] that was screwing everything up [00:48:57] I rsync'd too late... [00:48:59] sigh [00:50:31] (PS2) AndyRussG: Sample banner impressions client-side if no choiceData [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/188394 (https://phabricator.wikimedia.org/T45250) (owner: Ejegg) [00:50:34] (CR) jenkins-bot: [V: -1] Sample banner impressions client-side if no choiceData [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/188394 (https://phabricator.wikimedia.org/T45250) (owner: Ejegg) [00:51:37] well, it worked but I couldn't say what the steps were. I'll see if I can update in another dir [00:53:56] (PS1) Awight: WIP convert wikimedia stuff from submodules to vanilla dirs [wikimedia/fundraising/crm/vendor] - https://gerrit.wikimedia.org/r/190398 [00:54:27] Hrm? [00:54:59] I hacked off all the .git/modules cruft... just to see if we can do the merge on our dev boxen without The Pain. [00:55:20] yeah, but look at the diff for the wikimedia subdirs [00:55:23] -Subproject commit a1944f2c21dcc24d05b0846ff8e5c8176aeffda1 [00:55:24] +Subproject commit 34dd2fd6a22014fd4ff9a33ef8535da16677e2e9 [00:55:48] that's as far as I've gotten [00:55:53] yeah that's not good [00:55:56] it might take two commits [00:56:05] I also killed the lines in .git/config [00:56:05] and... I can't do the submodule rm with my stable git. [00:56:19] can you try committing just the submodule rm? [00:56:38] oooh. another clue: we have to run "git submodule sync" when doing the checkout [00:56:54] yep, I did that as 1 commit, now am trying to add and it only adds the top-level dir [00:57:06] thinking it's cause there's a .git file in each [00:57:17] can I see the rm? [00:57:20] so i've added */**/.git to .gitignore [00:57:26] yipes! [00:57:33] I don't think it's like that... [00:57:44] (PS1) Ejegg: Step 1: try to kill the submodules [wikimedia/fundraising/crm/vendor] - https://gerrit.wikimedia.org/r/190399 [00:57:46] I think this is fallout from the submodules [00:58:00] well, that looks fine [00:58:50] didn't work for me, either. [00:58:57] § Fundraising Sprint Devo, § Fundraising Dash: BUG: Fraud widget numbers don't seem right - https://phabricator.wikimedia.org/T87810#1035810 (CCogdill_WMF) @Ejegg thanks for the explanation! That's not at all how my query results were originally explained to me, so @PPena - the data I pulled for France, Israe... [00:59:41] I did: rm -rf vendor && git checkout vendor && cd vendor && git submodule sync (no submodules) && git review -d 190399 && cd .. && git submodule update -i --recursive [00:59:45] * awight gasps [01:00:02] I think they're being stored in the crm/.git dir [01:00:20] this is interesting: Submodule 'DonationInterface' (https://gerrit.wikimedia.org/r/wikimedia/fundraising/crm/vendor) registered for path 'wikimedia/DonationInterface' [01:00:23] Submodule 'wikimedia/SmashPig' (https://gerrit.wikimedia.org/r/wikimedia/fundraising/crm/vendor) registered for path 'wikimedia/SmashPig' [01:00:34] the "wikimedia/" prefix is missing from the DI submodule [01:00:46] oh and the URLs are hosed. [01:00:48] weird [01:01:23] oooh [01:01:27] you mean in crm/.git/modules/vendor? [01:01:36] it was my submodule update recursive that caused them to be recreated [01:01:39] ejegg: yes [01:02:11] I think your commit works [01:02:16] yeah, I tried submodule deinit vendor, rm -rf vendor, then a non-recursive update [01:02:24] hmm? [01:02:32] but did you git review -d 190399? [01:03:29] (PS3) AndyRussG: Sample banner impressions client-side if no choiceData [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/188394 (https://phabricator.wikimedia.org/T45250) (owner: Ejegg) [01:03:31] (CR) jenkins-bot: [V: -1] Sample banner impressions client-side if no choiceData [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/188394 (https://phabricator.wikimedia.org/T45250) (owner: Ejegg) [01:04:43] (PS1) Awight: WIP testing whether top-level submodule purge helps [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/190401 [01:07:27] sigh. [01:07:28] git submodule update -i --recursive [01:07:32] Submodule 'civicrm' () registered for path 'civicrm' [01:07:34] Submodule 'drupal' () registered for path 'drupal' [01:07:37] Submodule 'vendor' () registered for path 'vendor' [01:07:39] warning: unable to rmdir wikimedia/DonationInterface: Directory not empty [01:07:42] warning: unable to rmdir wikimedia/SmashPig: Directory not empty [01:07:45] Submodule path 'vendor': checked out '40df8782f1a4567b40d43208c7c9ee12d4898c27 [01:07:47] Hmm [01:11:30] yeah, the submodule hangover is passed, but I still can't add DonationInterface and Smashpig as normal subdirs unless I move their .git folders [01:14:37] (PS4) AndyRussG: Sample banner impressions client-side if no choiceData [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/188394 (https://phabricator.wikimedia.org/T45250) (owner: Ejegg) [01:15:14] (CR) AndyRussG: "Rebased" [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/188394 (https://phabricator.wikimedia.org/T45250) (owner: Ejegg) [01:15:16] (CR) jenkins-bot: [V: -1] Sample banner impressions client-side if no choiceData [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/188394 (https://phabricator.wikimedia.org/T45250) (owner: Ejegg) [01:16:52] (PS5) AndyRussG: Sample banner impressions client-side if no choiceData [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/188394 (https://phabricator.wikimedia.org/T45250) (owner: Ejegg) [01:17:14] (CR) AndyRussG: "Fixed silly rebase error" [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/188394 (https://phabricator.wikimedia.org/T45250) (owner: Ejegg) [01:22:05] MediaWiki-extensions-CentralNotice, Wikimedia-Fundraising: Special:RecordImpression should die in a fire - https://phabricator.wikimedia.org/T45250#1035861 (AndyRussG) As per discussions with FR, I just amended Elliot's [[ https://gerrit.wikimedia.org/r/#/c/188394/ | CN patch for review ]] to only downsample... [01:22:15] Got 2 run! [01:34:39] Fundraising-Backlog, § Fundraising Sprint E: Change errors on credit card forms from popup to red text - https://phabricator.wikimedia.org/T86214#1035891 (CCogdill_WMF) [03:10:17] § Fundraising Sprint Devo, § Fundraising Dash: BUG: Fraud widget numbers don't seem right - https://phabricator.wikimedia.org/T87810#1036004 (atgo) So @ppena does this work for you? Should we update the widget to display the same results as Caitlin's query? [03:24:50] § Fundraising Sprint Devo, § Fundraising Dash: BUG: Fraud widget numbers don't seem right - https://phabricator.wikimedia.org/T87810#1036008 (CCogdill_WMF) @atgo I don't think we should replace the fraud widget; that is correct as it is. It would just be nice to have a separate rejection widget :) Le jeudi 1... [03:35:10] MediaWiki-extensions-CentralNotice, Wikimedia-Fundraising: Special:RecordImpression should die in a fire - https://phabricator.wikimedia.org/T45250#1036011 (AndyRussG) I should add the caveat that it won't be an improvement at times like now, when there are campaigns targeting all projects and all languages.... [09:14:48] Wikimedia-Fundraising, Wikimedia-Fundraising-CiviCRM: Allow mismatched refunds - https://phabricator.wikimedia.org/T89437#1036231 (awight) NEW [09:49:44] hi jessicarobell, which desktop banner do you want to test first in Sweden: remind me later or the open amount field? [09:51:08] I think the open amount field is a better first test, to try for enSE. Thank you the-wub! [09:51:22] ok thanks [16:19:42] AndyRussG: where is that dependency injection extension? :( [16:20:23] It's already been the thing to use in a dozen places! yet unattainable... [16:21:07] awight: from my perspective, I guess it's lost in backlog! [16:22:05] right below a bunch of other things like an EP RFC, Enums (which I'd like to improve a bit before rolling out Editor Campaigns) and the data mapper :( [16:22:52] Ho hmm so much technical debt and so little income [16:22:59] But you're right, the interest rates are kiling us [16:24:00] BTW Andrew Otto was having a look at our hhvm log mystery and posted on Phab [16:25:18] Yeah that was good to see. [16:25:27] What do you think? [16:25:52] Yeah that was nice [16:26:01] I think it's ops's problem now, basically [16:26:32] If nobody can say for certain why the error seems to be repeated, they should certainly investigate IMO. [16:26:46] Yeah... I mean, I don't want the issue to be sidelined because no one cares and then the site to explode because no one cares [16:26:54] I agree that if they aren't curious, I'll just stand here and skullscratch a bit. [16:27:28] I can understand they have bigger fish to fry, but it's out of our hands because the mystery and potential consequences are on their turf [16:27:57] Unless, of course, it turns out to be our code that is magically calling SpecialBannerRandom [16:28:31] But assuming it's not, then they're the ones who can investigate and would take responsibility for any ill consequences of not doing so [16:29:15] So I'm like, either wait until they investigate and tell us what's up, or until they say they won't investigate and tell us to go ahead and kill that class [16:29:41] That way at least we won't get 503s because somebody else's problem [16:29:55] Does that make sense? [16:30:36] (back in a few minutes, then I'm out on a short errand and back again, I'll get backscroll tho!) [16:30:46] (P.S. Hope Mari feels better!) [16:31:29] that totally sounds right. donno if you want to explain as much in the task [16:31:49] awight: I did say as much to Andrew [16:32:03] :) ok sorry, I missed that comment. Thanks! [16:33:09] np, thanks also! :) [16:33:22] have fun bassooning! [17:33:45] Hehe, I think the octupus broke grrrit-wm [17:33:55] awight: Nice opener. [17:34:01] * awight tips hat [17:34:41] * awight shepherds octopus back into its tank with a jagged broom handle [17:35:09] is grrrit-wm who broadcasts our patches as we post for review? [17:35:19] cuz I just pushed one and don't see an announcement [17:35:32] o_O [17:35:42] it quit the channel a second ago, maybe our furious onslaught of patches damaged its feelings [17:36:05] "Ow, my feeling! ...That was my only copy." [17:36:09] octopi are pretty sensitive. [17:36:19] haha [17:36:24] * awight inadvertently changes color [17:36:54] would an octopus wear eight pants, or four? [17:37:45] * K4-713 squints [17:38:03] * awight one more question about the tech lead scheme you drunkenly shared last night. [17:38:06] aargh [17:38:23] Maybe you just call octopus pants a "bunch" instead of a "pair". [17:39:48] Also, I wasn't that drunk. [17:39:50] :p [17:39:52] Baahaha [17:39:56] FOR THE RECORD [17:40:18] I'm just saying: It's a wide spectrum. [17:40:19] K4-713: hey, do you have time to CR a bit? [17:40:24] * K4-713 sighs [17:40:27] Nope. [17:40:30] ok np [17:40:43] * awight eyes various other busy people [17:40:49] Seems indicative of... something. [17:42:00] K4-713: well, don't worry about a full CR but does this make sense IYO? https://gerrit.wikimedia.org/r/#/c/190484/ [17:42:17] * K4-713 takes large swig of coffee,;clicks [17:42:28] Oh, haha [17:42:35] cos: [17:42:49] So... the escaping bit. [17:42:54] it causes slight but permanent brain damage to anyone we might onboard :) [17:42:57] yes [17:43:10] I was thinking, escaping should be done at the point of use. But that's a religious thing [17:43:16] I have mild urges to travel back in time and smack myself for doing that. [17:43:21] Yes, exactly. [17:43:25] ok [17:43:28] Exactly that thing. [17:43:30] I'll work towards that [17:43:42] You can call it "normalized" if you change where the escaping happens. [17:43:43] For now, I think we can ignore "escaped" cos it might be a lie [17:43:49] Nah, it's in DD. [17:43:56] OK well that's reassuring at least [17:44:07] I wouldn't lie about something like that. [17:44:10] Probably. [17:44:18] But if DD::$normalized escaped, let's just call that "normalized" for now [17:44:21] That assumes, of course, that it's still there. [17:44:55] if there's a place in the code where we actually NEED escaped data, I'll add a big red comment that says "this has already been escaped to tell your friend to chill" [17:45:08] Hurm. [17:45:25] I just don't like it. Any of it. At all. [17:45:42] But, I could like it. [17:45:51] I could like it again. [17:46:30] Also, the old name doesn't really say what exactly it's been escaped *for*. [17:46:46] The answer, is basically "forms". [17:47:30] ah. I think that will be done transparently by any modern template renderer [17:48:03] So, we probably need to change that anyway. [17:48:08] You normally have to say "{{ varname | noescape_cos_i_like_to_die }} [17:48:21] Unless you like double-escaping. [17:48:24] right [17:48:36] I love it. [17:49:00] kthx [17:49:10] Call me in a week and you can do the CR, still :p [17:49:18] * K4-713 nods gravely [17:50:47] Aw, nuts. [17:50:51] I have run out of time. [17:51:04] * K4-713 vanishes [18:31:48] § Fundraising Tech Backlog, Wikimedia-Fundraising, MediaWiki-extensions-DonationInterface: Expiration date error message is wrong when date < now - https://phabricator.wikimedia.org/T89388#1037184 (CCogdill_WMF) This isn't a part of her doc; I actually didn't have her go through the Worldpay flow. We do have... [18:49:01] § Fundraising Tech Backlog, Wikimedia-Fundraising, MediaWiki-extensions-DonationInterface: Expiration date error message is wrong when date < now - https://phabricator.wikimedia.org/T89388#1037216 (awight) I'd love to see this doc! Just a small suggestion for the UX discussion, it's not technically difficult... [18:55:38] Wikimedia-Fundraising-CiviCRM, § Fundraising Sprint Devo: BUG: Undo a recent merge of two contact records in Civi - https://phabricator.wikimedia.org/T89408#1037249 (awight) I think we have to recover the anonymous contribution IDs from a backup, then do an update in our database to: a) Undelete the anonymous... [18:57:36] Wikimedia-Fundraising-CiviCRM, § Fundraising Sprint Devo: BUG: Undo a recent merge of two contact records in Civi - https://phabricator.wikimedia.org/T89408#1037262 (awight) [19:03:15] awight: thanks for the CR of the samplepatch! As well as improving following your comments, I'm gonna re-submit with the more agressive version, which will turn on sampling if there are no campaigns after client-side filtering, too (following my afterthoughts on the Phab task, https://phabricator.wikimedia.org/T45250) [19:10:27] AndyRussG: interesting, sounds great to me! [19:11:20] awight: K thanks! [19:12:49] K4-713: hi, how's it going? If u have a bit of time today, there are a couple issues floating about that I'd be quite happy to have ur input on :) thx in advance! [19:13:16] Time is currently the one thing I am having a serious problem with. [19:13:21] But, what's up? [19:17:36] K4-713: briefly... :) (1) It'd be nice to decide ASAP what to do as a temporary measure for https://phabricator.wikimedia.org/T45250 [19:17:52] This is turning down RecordImpressions whenever possible... [19:17:55] Hum. [19:18:09] I was reading the thread, and it looked like you had a temp patch all ready to go. [19:18:11] Is that not the case? [19:18:12] I'm about to submit a more agressive version of the patch, as mentioned in my last comment on the thread [19:18:15] Yeah [19:18:21] Just wanted to check in that that's a go [19:18:29] Yeah, I don't see any reason not to. [19:18:35] Cool! :) thanks [19:18:59] But, I maintain that we need to get everybody in the same place so we can figure out where the misfire is. [19:19:20] Which misfire? [19:19:21] It seems like all the things that you guys wanted to do to address the problem, were also unacceptable. [19:19:37] Ah right [19:19:44] This tells me that we don't have a clear understanding about what's ultimately needed. [19:19:56] Yeah in the latest patch and the one I'm about to send up Ellery's and Megan's concerns are addressed [19:20:19] Yeah, I was about to say, that seems like a decent start. [19:20:19] This shouldn't affect any of the data they're actually using, since it just turns things down when there are no campaigns [19:20:42] I'm kind of... astonished that wasn't the way it worked before. [19:20:43] Ops has provided a few moments of clarity, I think--mostly, they want us to stop loading the clients. Server load is not a do-or-die thing. [19:20:53] We will still have to find something better medium-term, and yeah that does require a bit more detailed study of what's needed [19:21:00] err, wrong idiom but u know what I mean... [19:21:24] If the extra request is slowing down page render, it's totally unacceptable. [19:21:44] It's surely making it less responsive especially on some platforms [19:21:52] We're not sure that's the case--anecdotally it looked okay to me, but Ori thinks this request is in the critical path, and delays onLoad. [19:21:55] Also, from my perspective, this is perfect timing to open the door to talk about Ellery's plans going forward, and why they may be too... brutal? [19:22:06] (conveniently the opinion I had to start with) [19:22:09] AndyRussG: that sounds right! [19:22:11] K4-713: lol [19:22:22] I'll also ask someone for some immediate varnish tuning, BTW (that was shunned earlier in the thread along the lines of "let's get to the root cause instead", but I think it's a fine temporarymeasure) [19:22:41] There's no reason for PHP to be handling this another second [19:23:00] Yep. [19:23:04] AndyRussG: That sounds great, but again it's the client side that really seems to be bringing on the bile [19:23:25] awight: yeah! And also, agreed, it's good they're worried about that :) [19:23:32] So, just for my own... no, sanity is the wrong word. [19:23:48] Mmm? [19:23:52] Just for my own [approximation of sanity] - when do you want to deploy that patch? [19:23:59] Monday [19:24:02] okay [19:24:08] And they know it's Monday? [19:24:09] Oh wait, is that a holiday? [19:24:19] Tuesday [19:24:21] Office holiday, yes. [19:24:25] Okay. [19:24:36] Would you make sure everybody knows that's the plan? [19:24:44] OK u bet! [19:24:49] Thanks. [19:24:50] thanks :) [19:24:55] Mmmm issue #2.... [19:25:01] There's more? [19:25:06] Okay, let 'er rip. [19:25:08] Yeah that was number one [19:25:17] https://phabricator.wikimedia.org/T89258 [19:25:28] So, long story slightly less long: [19:25:37] Wikimedia-Fundraising, MediaWiki-extensions-DonationInterface, § Fundraising Sprint Devo: Tech Review of LATAM processor - https://phabricator.wikimedia.org/T87046#1037402 (awight) stalled>Resolved [19:25:39] * K4-713 gets popcorn [19:25:55] The deploy last Tuesday kinda broke some dead code that we didn't even care about breaking 'cause it's... dead [19:26:10] But then it started causing errors in the hhvm.log... like, a lot! [19:26:18] Can we roll it into the nearest ditch? [19:26:27] Well, yes, but... [19:26:35] The errors are not fully explained [19:26:51] They are partly explained by 30k/day incoming requests that trigger the dead code [19:27:01] Right, I get that. [19:27:03] * awight lets albatross fester [19:27:07] But, that might not even be us. [19:27:32] Yeah, it's surely some eternally cached JS... But are in fact a million more errors in the logs than can be accounted for by the 30k/day requests [19:27:52] So, I've been bugging Ops and others to figure out why that is [19:27:58] Okay, so why *wouldn't* we roll it in to the nearest ditch? [19:28:14] Yeah also in my last comment on that thread... [19:28:30] "What if whatever is calling this broken code then breaks worse and breaks other stuff? Or what if this is a symptom of some unexpected behavior that needs to be uncovered, and removing the code for Special:BannerRandom just masks it? I confess I'd feel happier and cozier getting to the bottom of this before deploying such a change." [19:28:48] So basically I'd like to add one more comment on that thread [19:29:09] Saying that we'll go ahead and remove the code, but if shit breaks because of it, it's on Ops's shoulders [19:29:20] (in slightly more diplomatic wording) [19:29:38] I mean, if it were /my/ cluster and that were happening, I'd definitely want to know why [19:29:41] Hurm. [19:29:50] But as FR we don't have the means. And it's clearly and ops issue [19:29:58] What if you change the dead endpoint to do nothing but log something mildly informative about wth is going on? [19:30:26] K4-713: well yes, in theory [19:30:59] But it's not just Web entry point... It's that I don't have a backtrace that tells me what is calling it [19:31:10] So it'd somewhat hypothetical is all [19:31:30] I mean, I have _one_ backtrace, that points to a legit request that triggers this code [19:31:31] we have one backtrace... that says it's a normal web request from the ghostly legacy code [19:31:34] hehe [19:31:40] * awight goes back to popcorn [19:32:01] But what I don't have is a sample of backtraces to know that all the calls to that code are due to requests [19:32:19] Which in fact would be weird, because if that's the case they're not making it into the request logs [19:32:44] A theory would be that there are _way_ too many retries being made by varnish when it gets a 503 [19:32:59] hum [19:33:08] Another theory is that some other code in core or another extension is somehow getting there [19:33:33] Yet another theory, I suppose the most likely, is that I'm stupidly missing something totally obvious [19:33:33] Well, normally, I'd say let's just roll it into a ditch in a way that's trivial to revert. [19:33:50] Mmmm [19:33:58] With informative logging that would tell you if something deeply alarming was still going on. [19:34:14] OK, I think we can try that :) [19:34:20] I mean, I don't object [19:34:28] I'm not sure what we'd be hoping to learn from holding off. [19:34:53] Yeah... but I just wouldn't like to see a broken site and everyone pointing fingers and saying, it wasn't my problem [19:34:56] And, if we're clogging the error logs... learning is down all over. [19:35:03] Yeah also true [19:36:21] So I guess we could try that, again I just wanted to add to the thread that the decision is ops's, but I wouldn't want to go saying that without checking in :) [19:37:17] I dunno, maybe I'm taking the wrong approach/attitude [19:38:01] § Fundraising Dash, § Fundraising Sprint Devo: BUG: Fraud widget numbers don't seem right - https://phabricator.wikimedia.org/T87810#1037430 (CCogdill_WMF) I just checked in with Pats about this. We want to make sure we're understanding everything correctly, and if so, make a few requests. As I understand it... [19:38:15] AndyRussG: It seems to me that they already made their preferences known. [19:38:51] K4-713: OK.... sounds good! [19:38:55] Ha, more related emails reinforcing this premise. [19:39:32] hmm? [19:39:50] Went to Engineering. [19:40:02] Hum [19:40:25] greg-g wants us to stop emitting errors. :) [19:40:58] K4-713: OK we can do a small and simple patch for just that [19:41:10] and I think even deploy if that's OK [19:41:34] AndyRussG: Great. Just make sure you can unwind it just as simply in case it's more insidious than we thought. [19:42:24] OK [19:42:37] K4-713: So you're OK w/ a Friday deploy in this case, I guess [19:42:50] K4-713: mmm. point of interest, I've already written a ridiculous patch which destroys all of the legacy behavior. [19:43:06] K4-713: :) [19:43:07] It's not what you describe, I don't suggest unwinding it. [19:43:11] awight: I can't stop looking at the word "ridiculous" in that statement. [19:43:38] WEll... https://gerrit.wikimedia.org/r/#/c/190239/ [19:43:40] I got carried away. [19:44:13] It's stuff that has to be done eventually, so rather than pull on single threads which each might cause minor issues, I took the whole sweater off. [19:44:29] I see. [19:44:44] I mean, it can be rolled back if deployment fails, but I don't suggest doing so after patches go on top of it. [19:44:53] * K4-713 raises eyebrow [19:45:16] IMO, the only thing we have to lose is that potentially, 30k/day of people getting old banner allocations now get nothing. [19:45:32] awight: If you can convince AndyRussG to deploy it... [19:45:37] AndyRussG: please temper with your more sane approach? [19:45:41] :D [19:45:52] awight: no [19:45:56] :p [19:45:58] oh dear [19:46:04] then we're lost ;) [19:46:10] awight: K4-713: greg-g: I'm making a patch to just bow out on Special:BannerRandom::execute() [19:46:26] It's simple, definitely won't break anything else, etc etc [19:46:28] Sound good? [19:46:43] of course! [19:46:47] K :) thanks! [19:46:48] just bow out? [19:46:49] Woah. [19:46:53] We agree on a thing. [19:47:04] greg-g: "return;" i believe [19:47:08] :) [19:47:11] What's the fastest way to bow out in PHP/MW? [19:47:27] Get a job at Uber. [19:47:29] Just that? Not send some code like doesn't exist or something [19:47:31] baaahahaha [19:47:39] :( [19:47:55] IMO we don't want to spam 404s either? [19:48:07] +95/-745, wow [19:48:10] We could add a hard deprecation phase in later if we wish [19:48:21] * awight glares [19:48:27] can you test on beta cluster? [19:48:45] greg-g: I think AndyRussG is suggesting a one-liner to obviate my patchturd [19:48:58] http://en.wikipedia.org/wiki/List_of_HTTP_status_codes how about 204 deleted? [19:49:07] awight: it's not a patchturd! [19:49:07] awight: :) :) [19:49:41] Well, I can hardly label it surgically precise :p considering that I caused this snafu in the first place! [19:50:56] what's the mediawikiway of sending a 204 response? [19:51:08] I think my feelings of guilt drove me to lay the baroque patchturd. [19:51:52] lol, guilt -> indigestion? [19:52:09] usually [19:52:13] if you're a good catholic, at least [19:52:48] Doesn't Phabricator have like a place for quotes like bugzilla did? [19:53:02] haha, someone is working on it (of course) [19:53:09] * greg-g goe sto get lunch [19:53:20] Oh hey yeah. [19:53:23] Lunch. [19:53:38] AndyRussG: if you must :p https://office.wikimedia.org/wiki/Bash [19:53:55] Wouldn't be the first time. [19:54:08] awight: I just noticed, though, that yours have a sort of theme. [19:54:09] aww man that's a good one from Feb [19:54:26] K4-713: :( one day I might progress beyond the scatophilia [19:57:06] Wikimedia-Fundraising-CiviCRM, § Fundraising Sprint Devo: BUG: Gateway Reconciliation Report not pulling all donations - https://phabricator.wikimedia.org/T89288#1037467 (awight) @RLewis, can you give more details about the report you run? I assume the discrepancy is something related to soft credits, but I'... [19:58:36] AndyRussG: looks like, $this->getOutput()->setStatusCode ; but I'd be curious to hear from greg-g or other opsen which status codes will not be annoying to them. [19:58:52] ask ops [19:59:12] K [19:59:49] awight: looks good! [20:00:31] hey K4-713 i'm ravenous... want to go for noms? [20:05:53] Ahrg silly me, I misread what 204 meant [20:06:38]