Leipzig-based software developer. Previously, I worked on Wikidata, and before that on DokuWiki and WikiMatrix.
Tech: My Contributions
GitHub: micgro42
LinkedIn: https://www.linkedin.com/in/celenduin
🦔
Leipzig-based software developer. Previously, I worked on Wikidata, and before that on DokuWiki and WikiMatrix.
Tech: My Contributions
GitHub: micgro42
LinkedIn: https://www.linkedin.com/in/celenduin
🦔
Note that while the added changes improve the code, they do not yet address the fundamental issue brought up in this task and mentioned in the AC. So this task should not move to QA until the main points have actually been addressed.
Reading through CirrusSearch docs for an unrelated issue, I came across the following section which made me think of this task:
We did not do this, no other team did this, and by now CommunityConfiguration is deployed and so this task is now moot.
Note that this is more than just an i18n change. We MUST change the property from disabled to enabled too, even if it is just to create a new i18n message-key. If we would reuse the old key, then English UI users would see "task is enabled", but users of other languages would see "task is disabled" until it is translated for them as well.
Proposal for a long-term solution:
Let's move this to Blocked for now. It seems to require some resolution to T369608: Create a reasonable solution for working with CommunityConfiguration data.
Moving to QA per its position on the previous sprint board.
But is this actually ready for QA?
Not sure if there is much to QA around a maintenance script.
Thank you for the backport! 🙏 Logstash looks good so far.
Seems to have been an intermittent operational issue. No point in keeping a task from 2022 around. See also T316611 for an example of a similar task that was closed quickly.
Those are the errors from the refreshLinkRecommendations.php maintenance script, right? That's a PHP script. Why is this a Python error then? I'm confused.
To be rechecked after T368728 is resolved.
Thanks for the first fix! Though while this seems to have reduced the number of instances of this problem, it is still occurring.
This might be tricky to QA beyond just observing that "everything still works". This task was the proper fix for T365653, but we already had mitigations in place earlier.
The fix has been deployed and can now be verified on English Wikipedia: https://en.wikipedia.org/wiki/Special:CommunityConfiguration/GrowthSuggestedEdits where the relevant section now looks like this:
I think that will be resolved once we implement T368728: CommunityConfiguration: skipDashboardListing should fully hide the provider from UI.
The proposed short term solution
Do not show structured Add Link config if the frontend is disabled as an initial mitigation. This is not the desired solution, but it is very simple to implement, and it is arguably better than pretending the feature is enabled. It also gives us enough time to figure out what the long term solution should be.
while simple to implement, has the large disadvantage that any save to the Suggested Edits CommunityConfiguration form would remove the existing Link Recommendations settings, and enwiki also already has added non-trivial configuration to it.
Follow-up from Slack:
Not exactly UBN, but still we should ideally get a fix + backport done today.
Mh, unrelated changes aside, when I try to blank that field locally, I get a validation error for it: "NULL value found, but a string is required".
The first alternative that comes to mind right away is to follow the format that the API uses to work around this issue:
Specifically, Amir proposes to change the first paragraph of the current text (added like that in T367619):
Moving this to doing for creating some Grafana panels for the metrics added.
In T368405#9945822, @Sfaci wrote:I have been exploring a bit more the changes around 18th of May and there is no change that we can correlate to these events. The service code haven't been changed this year and the only changes we have done are related to the kubernetes configuration as I mentioned before. The closest change, regarding time, is https://gerrit.wikimedia.org/r/c/operations/deployment-charts/+/1033405 where some network policies were changed on 24th May (I guess it was deployed after that date) but I can't say whether that change could be related to this. @BTullis any idea here?
Taking a look at the grafana dashboard for edit-analytics for the last two months it seems that the latency is pretty stable (there are a couple of peaks but the rest of the chart is fine) so, considering that, I don't know what happens but I would say that these events are not related to the service itself. Regarding the timeouts you mentioned, just wondering if there is something preventing your app from reaching the service.
I think this can be closed.
In T363855#9953086, @Michael wrote:The remaining work has been split out into the subtasks:
- T369257: Server validation errors for items of Arrays with custom controls need special handling
- T369259: Server validation errors should show better label in main validation message link
This task can move forward to QA again once those tasks have been resolved.
No QA for this technical sub-task specifically, only for the parent: T363855: Improve validation errors wording and behavior
No QA for this technical sub-task specifically, only for the parent: T363855: Improve validation errors wording and behavior
Ah, after destroying all the containers and volumes, and carefully recreating them from scratch, it now seems to actually work!
In T369811#9980859, @Addshore wrote:And does search work etc when configured? :)
In T369591#9971327, @Sjoerddebruin wrote:Also, when opening on of the popups that feature a new item it briefly flashes very light blue in dark mode.
With the new binary the command succeeded, downloaded all the images and now I see "elasticsearch":true on Special:Mwdd. Thanks!
I've created a proof of concept: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CommunityConfiguration/+/1053632
Curious to hear your thoughts.
I don't seem to be able to reproduce any of the issues mentioned in the descriptions on enwiki's Special:Notifications (already on .wmf13). @Etonkovidova can you confirm?
This was a relatively rare error to begin with, so we probably need some time to verify that it is indeed gone. But if we see no new occurrences of it between July 19th 2024 and August 9th 2024 (4 weeks), then, I think, we can consider this verified as fixed.
Another sensible measure could be to add benchmarking in CI for this. That exists already in GrowthExperiments (currently broken: T329280), but CommunityConfiguration should not have those problems (that is, it does not depend on any other extensions), and ensuring that validation stays fast is even more important for CommunityConfiguration.
- Setting a default value for the maximum is tricky. CC does not allow default empty values (null) for some types, like the case of the number and integer. We could display a "very big" number as the default, but we cannot set an "empty" value for the field to signal the special case "don't apply the validation constraint if value is not set".