User Details
- User Since
- Aug 21 2020, 11:05 AM (170 w, 6 d)
- Availability
- Available
- IRC Nick
- urbanecm
- LDAP User
- Urbanecm work
- MediaWiki User
- Martin Urbanec (WMF) [ Global Accounts ]
Yesterday
Thanks everyone. Those points make a lot of sense. In that case, let's go ahead and merge @Soda's patch that implements the change. +2'ed in Gerrit and moving this to QA.
I cannot reproduce the issue anymore. Looks like someone was failing logins for too long and blocked everyone else. We might want to alter the message to say something else than "Invalid password", to avoid similar surprises in the future.
This is an urgent problem.
Wed, Nov 29
I started working on this today by reorganizing classes responsible for user options a bit (they were in MediaWiki\User, I moved them to MediaWiki\User\Options instead). I'm now working on a class to load conditional user defaults from configuration. After that, we'll also need to do this:
Tue, Nov 28
Mon, Nov 27
Thank you everyone for the comments here. As far as I can see, all of the suggestions gathered in this discussion were logged as Phabricator tasks. With that in mind, I'm resolving the task. Any future feedback is welcomed at the project MW.org talk page.
Thank you very much for taking the time to report the task and to submit a patch. At the very moment, the empty state of the Impact module is displayed when the user has some mainspace edits. However, once the Impact module gets enabled (=you have at least one mainspace edit), it looks like this:
Hi @Ladsgroup, this should now be done in production. One last huge batch of upserts is to be expected (because of an unrelated issue, we've bumped the version constant, resulting in regenerating all user impact data). Once that finishes, the size of queries should be considerably smaller. Leaving it open for you to verify – happy to make further changes if needed.
Automated comment: Community configuration usage (2023-11-26)
FTR: I ended up backporting this to make the T351898 fixes easier to backport.
Added basic Regression tests to this as well, to prevent it from happening in the future.
Sun, Nov 26
The issue is caused by 946538: Replace MediaWikiIntegrationTestCase::$tablesUsed with automatic query tracking. I don't currently understand why, but reverting that patch locally makes the issue go away. Pinging @Daimona @tstarling as the patch author/reviewer – can you help with fixing this problem, please?
Fri, Nov 24
FYI: T351888: Flow integration tests block GrowthExperiments merges with an unexpected ContainerDisabledException kind of blocks this, since tests failing (not sure why).
Hi @Trizek-WMF, thanks for reporting. I tried that at Czech Wikipedia (https://cs.wikipedia.org/w/index.php?title=Speci%C3%A1ln%C3%AD:Dosah_p%C5%99%C3%ADsp%C4%9Bvk%C5%AF&uselang=en), and I got this:
Hi @Ladsgroup, I've uploaded a patch if you want to review. I think that omitting the days with zeros is going to be the biggest change in that patch, since impact data aren't calculated for a lot of bots already anyway. On enwiki, we apparently have user impact data generated only for nine bots in total (P53863). We can certainly explicitly exclude bots from the automated generation (they'd still be included for manual requests via /w/rest.php/growthexperiments/v0/user-impact/%23USERID, and I think that's behaviour that we should keep).
This is most likely a fallback from T336203: Positive reinforcement: Deploy the new Impact module to all Wikipedias, where we enabled the feature on all Wikipedias. The serialization format was not changed recently, but deploying this everywhere (esp enwiki I guess) probably made this issue way more visible. I'll look into this and upload some patches to make the blob size smaller.
Thu, Nov 23
Thanks for testing @Etonkovidova. I reviewed in test.wikipedia production as well, and the notification not appearing is most definitely a bug – moving back to Doing. @Cyndymediawiksim, can you please check why this is happening and fix the bug Elena found?
Ack, thank you for the confirmation. Leaving the task open for it to go through Growth's QA.
Hi @Etonkovidova, thanks for the tests. As T306349: Public-facing API for querying image suggestion recommendations and submitting user feedback is not resolved, we currently do not have a publicly available API for the image recommendation service. In other words, the current image recommendation API can be only called from Wikimedia production, and it is not available in Wikimedia cloud (including the beta sites). To make the image recommendation feature at least somehow testable in beta, we mirror production recommendations in beta (if Foo article has an image recommendation in production enwiki, the Foo article in beta enwiki will have the same image recommended). If you accept an image in production for one article, the same article in beta should lose the recommendation as well.
I can reproduce the issue locally, but only when I run all Flow tests together (composer phpunit:entrypoint extensions/Flow/tests/phpunit/). When I run only composer phpunit:entrypoint extensions/Flow/tests/phpunit/Api/ApiFlowEditTopicSummaryTest.php, everything passes. This makes me think the issue is caused by another Flow test not being perfectly self-contained and breaking the service container by somehow calling MediaWikiServices::destroy and not re-initing the container afterwards, which appears to be the only way how this exception can be triggered. The question is why this happens.
Wed, Nov 22
I'll look into this.
Tue, Nov 21
This is now done. I managed to miss a critical point in the documentation, so I attempted to clarify the docs as well.
Mon, Nov 20
Hello @Pppery, this problem should now be fixed in production. Can you please try again and let us know if Community configuration works properly now?
Automated comment: Community configuration usage (2023-11-19)
Thanks @Etonkovidova for the tests. I think this is caused by the fact that the expiry logic runs daily at a specific time (14:27 UTC, to be precise), which means the account expires once it is at least a day old at 14:27 UTC. I personally don't think this is a problem (there'd be at most a day delay over a year) though. If @KStoller-WMF thinks this is problematic behaviour, we can certainly revisit.
I'll investigate, thanks for reporting.
Fri, Nov 17
I figured the root cause here. Even before the fix above, GrowthExperiments did drop the mentor/mentee relationship from the database and features like the Homepage or Help panel checked the mentorship state (enabled/opt-out) before directing the user to a mentor. However, in the case of the Help panel, the check only happened in the user interface. Backend still attempted to assign the mentor and pass that information to the Help panel UI. This is now fixed by checking for opt-out within MentorManager itself, so that any other part of GrowthExperiments receives correct data about mentorship from now on.
Thanks for the reply. Does that mean that in this task, the message should only change for the (one-off) T330071 reassignment? I originally understood it as change the message for any mentor/mentee reassignment (whether done by the script, or by a mentor quitting themselves, or by an admin removing). But, maybe I got that incorrectly.
Thu, Nov 16
So, upon an initial investigation, I have some thoughts about how this bug can be fixed. I'll need more time to be able to prepare a correct fix for the problem. Thank you for your patience!
There are two sub-bugs here.
@Trizek-WMF Hello, thanks for filling this task! There is a second, similar, message used when an admin removes a mentor using Special:ManageMentors. Currently, it reads $admin removed $oldmentor from mentorship. Do you think we should update that message as well? If so, would Automated reassignment of mentees since $admin removed $oldmentor from mentorship work? Or would you prefer if we use a different copy for that scenario?
Thanks @Trizek-WMF and @Janui, thanks for the report. I investigated this bug, and I can see that @Janui is opted out of mentorship, but they have a mentor assigned regardless. This is definitely a bug somewhere, but I'm not yet sure where exactly.
Tue, Nov 14
@Etonkovidova This is now testable at beta wikis where IP Masking is enabled. I set the expiry to 1 day as requested. Please let me know what you think and whether it is OK to increase the expiry to a year, as requested. For now, moving to QA. The other part (T344694) is still in CR.