[00:03:15] (CR) Ejegg: [C: 1] "Looks +2able, could do some tiny WmfFramework cleanup" (3 comments) [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/300918 (https://phabricator.wikimedia.org/T131798) (owner: Awight) [00:04:54] (CR) Ejegg: [C: 2] "ah, you clean up renderWikiText in the next patch. Let's go for it!" [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/300918 (https://phabricator.wikimedia.org/T131798) (owner: Awight) [00:07:00] (PS4) Awight: [WIP] Check whether tests are missing CTID [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/304116 [00:07:02] (PS14) Awight: WmfFramework-ize some HTTP request functions [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/300918 (https://phabricator.wikimedia.org/T131798) [00:07:04] (PS26) Awight: Some decoupling of GatewayPage from GatewayType [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/300804 (https://phabricator.wikimedia.org/T131798) [00:09:13] (PS15) Ejegg: WmfFramework-ize some HTTP request functions [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/300918 (https://phabricator.wikimedia.org/T131798) (owner: Awight) [00:09:32] (CR) Ejegg: [C: 2] "woohoo!" [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/300918 (https://phabricator.wikimedia.org/T131798) (owner: Awight) [00:11:59] argh, now Mustache is explicitly coupled to gatewaypage [00:14:50] (PS5) Awight: [WIP] Check whether tests are missing CTID [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/304116 [00:14:52] (PS27) Awight: Some decoupling of GatewayPage from GatewayType [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/300804 (https://phabricator.wikimedia.org/T131798) [00:16:45] (PS6) Awight: Don't be nice about missing contribution_tracking_id [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/304116 [00:17:13] gotta run! [00:24:12] (Merged) jenkins-bot: Merge branch 'master' into wmf_deploy [extensions/CentralNotice] (wmf_deploy) - https://gerrit.wikimedia.org/r/304130 (owner: AndyRussG) [00:39:47] (Merged) jenkins-bot: WmfFramework-ize some HTTP request functions [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/300918 (https://phabricator.wikimedia.org/T131798) (owner: Awight) [00:40:03] (CR) Zppix: [C: 1] Don't be nice about missing contribution_tracking_id [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/304116 (owner: Awight) [02:21:42] (CR) jenkins-bot: [V: -1] Some decoupling of GatewayPage from GatewayType [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/300804 (https://phabricator.wikimedia.org/T131798) (owner: Awight) [02:21:44] (CR) jenkins-bot: [V: -1] Don't be nice about missing contribution_tracking_id [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/304116 (owner: Awight) [03:15:30] (CR) Eileen: [C: 2] "I've added this to an upstream PR - good spotting" [wikimedia/fundraising/crm/civicrm] - https://gerrit.wikimedia.org/r/304110 (owner: Ejegg) [03:18:36] (Merged) jenkins-bot: Fix DedupeFind cache set call [wikimedia/fundraising/crm/civicrm] - https://gerrit.wikimedia.org/r/304110 (owner: Ejegg) [14:23:30] fundraising-tech-ops, Operations, Security-General: use granularity (g=) restrictions for wikimedia.org fundraising DKIM records - https://phabricator.wikimedia.org/T142205#2544058 (akosiaris) p:Triage>Normal [16:37:36] fr-tech fyi the CN update that had been planned for last night should go out on the train in a couple hrs :) [16:37:47] Good times [16:38:12] Hope so!!! [16:43:31] nice [16:50:26] (CR) Ejegg: "Works if you add this to TestingGatewayPage:" [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/300804 (https://phabricator.wikimedia.org/T131798) (owner: Awight) [16:52:01] (PS1) Ejegg: Mock TestingGatewayPage->getPageTitle() [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/304260 [16:52:27] fr-tech easy fix there would let me just rebase awight's last DI orphan patch and make it pass tests ^^ [16:53:50] Looking [16:55:21] err, one sec [16:55:27] XenoRyet: lemme try one other thing [16:55:34] 10-4 [16:58:09] (PS2) Ejegg: Mock TestingGatewayPage->getPageTitle() [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/304260 [16:58:17] OK, I think that's slightly more consistent [16:58:29] Should return the same Title object as other ways to get it [17:01:11] Yea, that looks better. [17:01:20] (CR) XenoRyet: [C: 2] Mock TestingGatewayPage->getPageTitle() [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/304260 (owner: Ejegg) [17:03:09] (Merged) jenkins-bot: Mock TestingGatewayPage->getPageTitle() [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/304260 (owner: Ejegg) [17:04:29] (PS28) Ejegg: Some decoupling of GatewayPage from GatewayType [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/300804 (https://phabricator.wikimedia.org/T131798) (owner: Awight) [17:19:46] (CR) Ejegg: [C: 2] "Looking good!" [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/300804 (https://phabricator.wikimedia.org/T131798) (owner: Awight) [17:21:25] (Merged) jenkins-bot: Some decoupling of GatewayPage from GatewayType [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/300804 (https://phabricator.wikimedia.org/T131798) (owner: Awight) [17:35:26] (PS3) Ejegg: Quit deleting from pending queue, stop saying limbo [extensions/DonationInterface] - https://gerrit.wikimedia.org/r/301630 (https://phabricator.wikimedia.org/T133433) [17:44:19] XenoRyet: up to anything fun? [17:45:50] Heh, no. Heal.ht benefits bureaucracy at the moment. Navia keeps not believing me about what we're using our flex account for. [17:45:58] Gotta make the paperwork gods happy. [17:46:04] aw man [17:46:32] I could put it aside if you've got something needs looking at though. [17:46:49] I'm starting to look at some drupal modules to see if they're unused, just wondered if you wanted to go on a spelunking expedition [17:46:55] nothing urgent at all [17:47:32] Yea, better finish up this other thing if it's not urgent. [17:48:40] (PS4) Ejegg: DO NOT MERGE: import info from pending DB, not AMQ [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/299938 (https://phabricator.wikimedia.org/T122641) [17:48:42] (PS2) Ejegg: WIP tests for combining pending DB info [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/300584 (https://phabricator.wikimedia.org/T122641) [18:00:30] (PS1) Ejegg: Centralized list of enabled modules [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/304275 [18:03:47] (PS1) Ejegg: Use module list from crm repo [wikimedia/fundraising/civicrm-buildkit] - https://gerrit.wikimedia.org/r/304276 [18:08:25] (PS2) Ejegg: Centralized list of enabled modules [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/304275 [18:12:33] (PS2) Ejegg: Use module list from crm repo [wikimedia/fundraising/civicrm-buildkit] - https://gerrit.wikimedia.org/r/304276 [18:27:04] (PS3) Ejegg: Use module list from crm repo [wikimedia/fundraising/civicrm-buildkit] - https://gerrit.wikimedia.org/r/304276 [18:34:18] (PS1) Ejegg: globalcollect_audit doesn't need contribution_audit [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/304284 [18:37:17] (PS1) Ejegg: Remove unused modules [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/304285 [18:37:53] fr-tech: I'm pretty sure we don't need those 3 modules ^^ [18:38:13] But it would be nice to have an independent investigation :) [18:38:33] (CR) jenkins-bot: [V: -1] Remove unused modules [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/304285 (owner: Ejegg) [18:38:36] Also, I think the centralized module list is going to be handy [18:38:59] That test failure was expected, cross-repo dependency on one of the patches above [18:39:26] yeah definitely good to have a central list [18:39:39] i've been keeping one in my home dir [18:39:50] matching vagrant patch: https://gerrit.wikimedia.org/r/304282 [18:56:13] (PS1) Ejegg: Remove some unused message fields [wikimedia/fundraising/SmashPig] - https://gerrit.wikimedia.org/r/304289 (https://phabricator.wikimedia.org/T140959) [19:01:01] (PS1) Ejegg: Only compare pending fields with values [wikimedia/fundraising/SmashPig] - https://gerrit.wikimedia.org/r/304293 (https://phabricator.wikimedia.org/T140959) [19:01:50] fr-tech: can anybody review those? Might need to poke around in other repos to confirm unused-ness of fields [19:02:22] ejegg: I could take a look a bit later... [19:02:54] My plate is a bit full for probably the rest of the day. [19:03:13] I'd like to get those out today if possible, since right now the pending queue/db comparisons are all logging false positive differences [19:05:18] i'm pretty deep in virtualbox networking but i'll try to take a break soon [19:25:56] (PS7) Ejegg: Update composer libs and resolve global namespace conflict [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/301521 (owner: Awight) [19:40:07] (CR) Cdentinger: Remove some unused message fields (1 comment) [wikimedia/fundraising/SmashPig] - https://gerrit.wikimedia.org/r/304289 (https://phabricator.wikimedia.org/T140959) (owner: Ejegg) [19:40:39] (CR) Ejegg: Update composer libs and resolve global namespace conflict (2 comments) [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/301521 (owner: Awight) [19:47:52] (PS2) Ejegg: Only compare pending fields with values [wikimedia/fundraising/SmashPig] - https://gerrit.wikimedia.org/r/304293 (https://phabricator.wikimedia.org/T140959) [19:49:14] (CR) Ejegg: "Maybe a little CRM cleanup needed - I'd appreciate another set of eyes on that! I rebased the other 'compare Pending' patch to be independ" [wikimedia/fundraising/SmashPig] - https://gerrit.wikimedia.org/r/304289 (https://phabricator.wikimedia.org/T140959) (owner: Ejegg) [19:49:42] cwd I think the other patch will actually fo the trick solo [19:49:50] *do the trick [19:50:03] https://gerrit.wikimedia.org/r/304293 [19:52:24] ejegg: why are those things in the message if we don't want them on the receiving end? [19:52:50] I think they're legacy paypal-specific stuff [19:53:49] I saw country_2 in the (unused) log_audit.module [19:54:41] what about the 3 that are specified? [19:54:59] those are stomp-specific fields [19:55:20] ah gotcha, so when we compare messages we don't want to compare those [19:55:37] or rather, correlationId is stomp specific, and the other two are related to SmashPig's serialization of those JsonSerializableObject classes [19:56:00] yeah, we want to have quiet logs on that front unless there are substantive differences [19:56:10] ah ha, any way we could fix that in smashpig? [19:56:41] have it not include the keys if there are no values? [19:57:17] err, i thought that unset loop would do the trick [19:57:58] yeah i mean it'd probably be the same effect, it just seems like doing something unnecessary and then doing another thing to undo it [19:58:09] would be simpler to not do it in the first place [19:58:48] oh yeah, we could remove all the default values from DonationInterfaceMessage, but I'm not sure if that would affect other stuff [19:59:04] so for now I'd just like to fix it in the comparison code [19:59:16] ok [19:59:21] (CR) Cdentinger: [C: 2] Only compare pending fields with values [wikimedia/fundraising/SmashPig] - https://gerrit.wikimedia.org/r/304293 (https://phabricator.wikimedia.org/T140959) (owner: Ejegg) [19:59:29] thanks! [19:59:35] np [20:00:11] (Merged) jenkins-bot: Only compare pending fields with values [wikimedia/fundraising/SmashPig] - https://gerrit.wikimedia.org/r/304293 (https://phabricator.wikimedia.org/T140959) (owner: Ejegg) [20:01:11] (PS1) Ejegg: Merge branch 'master' into deployment [wikimedia/fundraising/SmashPig] (deployment) - https://gerrit.wikimedia.org/r/304314 [20:01:19] (CR) Ejegg: [C: 2] Merge branch 'master' into deployment [wikimedia/fundraising/SmashPig] (deployment) - https://gerrit.wikimedia.org/r/304314 (owner: Ejegg) [20:21:43] (CR) Ejegg: [V: 2] Merge branch 'master' into deployment [wikimedia/fundraising/SmashPig] (deployment) - https://gerrit.wikimedia.org/r/304314 (owner: Ejegg) [20:22:05] hi eileen! [20:22:34] got some patches to keep the enabled modules list in sync, if you want to take a look [20:23:01] https://gerrit.wikimedia.org/r/304275 https://gerrit.wikimedia.org/r/304276 https://gerrit.wikimedia.org/r/304282 [20:25:52] !log updated SmashPig from 5c180de6e424be10f9d61052c2c8dca7e0e825af to 4ba14a10518f80fe7e20fadc493d78c796c15921 [20:25:57] Logged the message at https://wikitech.wikimedia.org/wiki/Server_Admin_Log, Master [20:30:33] (PS15) Ejegg: Orphan rectification for Drush (SEE NOTES) [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/300708 (https://phabricator.wikimedia.org/T141487) (owner: Awight) [20:31:59] (PS5) Ejegg: Not using referrer from messages [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/289887 (https://phabricator.wikimedia.org/T110564) [20:51:21] MediaWiki-extensions-FundraisingChart, Security-Reviews: Security review for mediawiki/extensions/FundraisingChart - https://phabricator.wikimedia.org/T68805#709084 (dpatrick) This, and T67834, appear stalled. Is anything more needed/expected on this ticket? We will assume not if no response is given by... [20:58:17] Fundraising-Backlog, Wikimedia-Fundraising-CiviCRM: Fold 'donor comment' field into 'note' - https://phabricator.wikimedia.org/T142747#2545450 (Ejegg) [21:10:04] (PS1) Ejegg: Merge 'donor comments' custom field into note [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/304326 (https://phabricator.wikimedia.org/T142747) [21:16:33] fr-tech: got a single error on production from our rollout, just related to translatable banner messages and extension registration, so I think we could do a quick fix rather than a revert... Discussing with Reedy in [21:16:39] #wikimedia-dev [21:35:03] Fundraising-Backlog, Wikimedia-Fundraising-CiviCRM, Patch-For-Review: Fold 'donor comment' field into 'note' - https://phabricator.wikimedia.org/T142747#2545669 (Ejegg) a:Ejegg [21:35:26] Fundraising-Backlog, Wikimedia-Fundraising-CiviCRM, Patch-For-Review: Fold 'donor comment' field into 'note' - https://phabricator.wikimedia.org/T142747#2545450 (Ejegg) Any objections, Civi users? [21:42:51] relocating... [22:25:32] (CR) Ejegg: "Not needed if Ie379158d2a324ab312aee merges first" [wikimedia/fundraising/crm] - https://gerrit.wikimedia.org/r/303701 (owner: Ejegg) [22:26:34] AndyRussG: anything I can do to help? I looked at the registration core code, and it does indeed define the things you specify in extension.json [22:27:16] ejegg: just about to finish the patch, then we should test, dunno if we can push it out todayz, mebbe... [22:27:30] Ah OK so that's comforting! [22:27:34] thx much :) :) [22:27:59] So that's guaranteed to be called when we do wfLoadExtension( 'CentralNotice' ) ? [22:29:58] yeah, but I'm less certain of the 'conditional' bits [22:30:08] hmmm [22:30:10] Fundraising-Backlog, Wikimedia-Fundraising-CiviCRM, Patch-For-Review: Fold 'donor comment' field into 'note' - https://phabricator.wikimedia.org/T142747#2545899 (MBeat33) Seems like an easy win @Ejegg [22:30:20] afaict, they don't get registered at all when conditional is true [22:30:45] the namespaces? that makes sense... [22:30:47] Or the constants? [22:30:48] oh, derp, the defines still happen [22:30:54] right cool beanz [22:32:23] The redundancy without autoverification could lead to hard-to-find bugz [22:32:42] Still [22:33:06] Fundraising Sprint Octopus Untangling, Fundraising-Backlog, Wikimedia-Fundraising-CiviCRM, Patch-For-Review, Unplanned-Sprint-Work: Fold 'donor comment' field into 'note' - https://phabricator.wikimedia.org/T142747#2545931 (Ejegg) [22:34:05] back soon... [22:34:11] ejegg: cool thx! [22:37:28] Fundraising Sprint Octopus Untangling, Fundraising-Backlog, Wikimedia-Fundraising-CiviCRM, Patch-For-Review, Unplanned-Sprint-Work: Fold 'donor comment' field into 'note' - https://phabricator.wikimedia.org/T142747#2545985 (CCogdill_WMF) Sounds good to me! [22:42:47] (PS1) AndyRussG: Define namespaces in extension.json [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/304410 [22:42:55] ejegg: ^ [22:42:59] looking [22:43:02] Reedy: ^ [22:43:09] thx!!!! [22:43:42] Yesterday awight found we can test some translation stuff on the beta cluster [22:44:34] I don't see any other incidences of this error on prod... [22:51:50] AndyRussG: Yeah, those lines are useless now [22:58:08] (CR) Ejegg: [C: 1] "Looks like this should be safe, but I'll smoke test it some. Unrelated problem: looks like the namespaces don't get registered for the in" [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/304410 (owner: AndyRussG) [22:59:28] ejegg: I think $wgNoticeUseTranslateExtension wants dropping from the condiitional [22:59:46] or moving into it, and wrapping $wgTranslateMessageNamespaces [22:59:57] right, that's what I was thinking [23:00:13] I presume that's just a migration artact [23:00:17] *artefact [23:01:36] hmm [23:01:39] I dunno [23:02:42] ejegg: Reedy I think those namespaces actually only used when translate extension? see the comments I added in the hook, copied from the README, copied in turn from the old CentralNotice.php [23:03:22] ah, got it [23:03:46] However, it's still kinda hoakey [23:07:04] It seems BannerMessageGroup class is only used if $wgNoticeUseTranslateExtension [23:07:31] Anyway, I'd rather keep the conditional as it was, just to keep the changes small, especially if we want to push something out quickly [23:07:38] Does that make sense? [23:07:50] (as far as sense can be made in CentralNotice) [23:10:05] yah [23:11:26] (CR) Ejegg: [C: 2] "ooh, those defines /ARE/ necessary! getCanonicalNamespaces is called after ExtensionRegistry->queue but before ExtensionRegistry->load, wh" [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/304410 (owner: AndyRussG) [23:13:02] (Merged) jenkins-bot: Define namespaces in extension.json [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/304410 (owner: AndyRussG) [23:13:25] K let's hammer in a bit when it lands in betacluster! [23:14:03] I wonder where we can see warnings generated in betacluseter? [23:14:19] (CR) Legoktm: "Er wait, what is calling getCanonicalNamespaces before extensions are being loaded??" [extensions/CentralNotice] - https://gerrit.wikimedia.org/r/304410 (owner: AndyRussG) [23:19:36] RRrrg how can I get a treeish link from diffusion? [23:19:38] hi [23:19:43] legoktm: hi! [23:19:44] what is calling getCanonicalNamespaces? [23:20:19] legoktm: nothing, I don't think. It's a class variable whose value is defined using a NS constnat [23:20:37] Not that that necessarily makes sense [23:20:42] Here is the error [23:20:56] Notice: Use of undefined constant NS_CN_BANNER - assumed 'NS_CN_BANNER' in /srv/mediawiki/php-1.28.0-wmf.14/extensions/CentralNotice/includes/BannerMessageGroup.php on line 12 [23:21:03] On prod, visible in logstash [23:21:10] Just following the extension registration deploy [23:21:49] hmm [23:21:52] I guess it's just that something references that class and it gets autoloaded before the constants are defined [23:22:26] https://github.com/wikimedia/mediawiki-extensions-CentralNotice/blob/master/includes/BannerMessageGroup.php#L12 [23:22:55] but...that should be impossible, given that the autoloader entry is only added at the same time as the defines [23:23:02] ah, sorry, never mind [23:23:19] I can't reproduce it... [23:24:02] but the first time I stepped through that fn, it went into the if ( !defined( ... ) ) branch [23:24:09] hmmm. [23:24:32] well, if the cache is cold, then autoloader entries are added earlier and at a different time than the define [23:24:42] maybe that's why you saw it on the first time? [23:24:51] ejegg: to you have translate extension and all installed? [23:24:52] ah, let me try deleting that [23:24:56] AndyRussG: yep [23:25:04] Ooooh cool! [23:25:18] try bumping ExtensionRegistry::CACHE_VERSION and see if it come sback? [23:25:56] maybe that's why we're only seeing one error in the logs on prod also? [23:27:39] huh, deleted cache dir and bumped that version, and still no dice [23:27:46] it's cached in APC [23:27:51] ah [23:28:01] but touching the version should invalidate [23:28:37] well, the belt & suspenders on that patch should still be safe [23:28:40] I kind of doubt that it's that [23:28:41] yes [23:28:44] but I'd like to find the root cause [23:29:20] I'm having trouble seeing any web code path that doesn't load the extensions first, since they're right at the top of Setup.php [23:30:32] ohhhh, I had hit the page a few minutes before I updated the CN code [23:32:19] so the first time I hit the page after update, exportExtractedData used the stale stuff [23:33:09] hmm, no, key includes mtime of all the .jsons [23:33:16] (of course) [23:41:27] ejegg: legoktm: do you think this change won't prevent that error? [23:41:52] Mmm also I dunno if anywhere there would be a stack trace from prod, or who would have permission to fetch it? [23:42:18] Nor what the user experience when it occurs... [23:44:24] Provided the NS defines happen early, it should stop the error? Should we see if we can push this out then pick apart the cause more later? [23:44:32] sorry, i must have done that first stepthrough on the old code, i seriously don't see any way the hook fires before the defines [23:44:33] ejegg: legoktm: Reedy ^ [23:44:56] AndyRussG: no the change is fine, but I'd like to find out what is causing it [23:45:06] Right [23:45:22] I'm just seeing the window of opportunity for deploying it tonite closing... [23:45:25] legoktm: please ignore my report, I think I was looking at the wrong version [23:50:49] ejegg: legoktm: Reedy: translatable banner messages working on the beta cluster (where the patch has been pushed to...) [23:50:51] https://meta.wikimedia.beta.wmflabs.org/wiki/CNBanner:Stewvote-text1/de [23:50:58] https://meta.wikimedia.beta.wmflabs.org/wiki/Special:Translate?group=Centralnotice-tgroup&filter=&language=de&action=translate [23:51:04] lets just deploy it then? [23:51:12] yeah! [23:51:23] ejegg: ? [23:52:12] SWAT window still has a few minutes left on it.... [23:54:36] sure! [23:55:14] K cool thx!!! [23:59:50] is there any way i can do a normal browser search in new gerrit?