Page MenuHomePhabricator

gerrit-wm should not report commits from the l10n-bot
Closed, ResolvedPublic

Description

Each l10n update will create lots of commits but it would be nice if one didn't get a notification for each of them.


Version: unspecified
Severity: enhancement
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=46450

Details

Reference
bz35538

Related Objects

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 12:20 AM
bzimport added a project: Gerrit.
bzimport set Reference to bz35538.
bzimport added a subscriber: Unknown Object (MLST).

I think we should default to send the notifications. People not willing to receive them could set up a filter in their mail agent.

I can't imagine anyone who wouldn't want to filter those commits. It may not be a problem for those who are watching only few projects.

Was just treated to 198 e-mails connected to the L10n updates (up from 1). Scary.

This bug didn't mention "gerrit-wm", so I didn't find it in a search. I left a comment about the effect of these commits at bug 35427 comment 5.

I think Gerrit change 5547 fixes this.

Rewrite summary: we do not want to spam IRC channels.

It's not only about IRC. It's also about email. If it makes sense to have two bugs for those, then let's do that.

(In reply to comment #7)

It's not only about IRC. It's also about email. If it makes sense to have two
bugs for those, then let's do that.

I want the email notifications.

(In reply to comment #7)

It's not only about IRC. It's also about email. If it makes sense to have two
bugs for those, then let's do that.

Done as bug 36168.

This was a simple config change for the L10n-bot group (I hope). I enabled the "only notify authors" group setting.

If we survive the next L10n deluge, we'll know I was right and this can be FIXED.

Actually, this will be different from bug 36168, we'll still need hook changes here.

Change merged, should be fixed. Will know later today.

(In reply to comment #13)

Change merged, should be fixed. Will know later today.

This is not fixed. The bot started flooding again just a few minutes ago:


[18:39] <gerrit-wm> New patchset: L10n-bot; "Localisation updates from http://translatewiki.net." [mediawiki/extensions/AbsenteeLandlord] (master) - https://gerrit.wikimedia.org/r/8513
[18:39] <Reedy> Useful error message is useful
[18:39] * Reedy facepalms
[18:39] <gerrit-wm> New patchset: L10n-bot; "Localisation updates from http://translatewiki.net." [mediawiki/extensions/AbuseFilter] (master) - https://gerrit.wikimedia.org/r/8514
[18:39] <Ryan_Lane> well
[18:39] <Krenair> uh oh
[18:39] <gerrit-wm> New patchset: L10n-bot; "Localisation updates from http://translatewiki.net." [mediawiki/extensions/AkismetKlik] (master) - https://gerrit.wikimedia.org/r/8515
[18:39] <Krenair> I guess it didn't work
[18:39] <gerrit-wm> New patchset: L10n-bot; "Localisation updates from http://translatewiki.net." [mediawiki/extensions/ArticleFeedback] (master) - https://gerrit.wikimedia.org/r/8516
[18:39] <Ryan_Lane> fix our *shitty, shitty* authentication code and I'll report better errors
[18:39] <Reedy> We knew that early
[18:39] <gerrit-wm> New patchset: L10n-bot; "Localisation updates from http://translatewiki.net." [mediawiki/extensions/ArticleFeedbackv5] (master) - https://gerrit.wikimedia.org/r/8517
[18:39] <gerrit-wm> New patchset: L10n-bot; "Localisation updates from http://translatewiki.net." [mediawiki/extensions/AuthorProtect] (master) - https://gerrit.wikimedia.org/r/8518
[18:39] <Krenair> Might be a good idea to +q again
[18:39] <gerrit-wm> New patchset: L10n-bot; "Localisation updates from http://translatewiki.net." [mediawiki/extensions/Babel] (master) - https://gerrit.wikimedia.org/r/8519
[18:40] <gerrit-wm> New patchset: L10n-bot; "Localisation updates from http://translatewiki.net." [mediawiki/extensions/CategoryTree] (master) - https://gerrit.wikimedia.org/r/8520
[18:40] <Ryan_Lane> I've had a bug in for *years* asking to allow auth plugins to report custom errors
[18:40] <gerrit-wm> New patchset: L10n-bot; "Localisation updates from http://translatewiki.net." [mediawiki/extensions/CentralAuth] (master) - https://gerrit.wikimedia.org/r/8521
[18:40] <gerrit-wm> New patchset: L10n-bot; "Localisation updates from http://translatewiki.net." [mediawiki/extensions/CentralNotice] (master) - https://gerrit.wikimedia.org/r/8522
[18:40] <Ryan_Lane> though I guess I was kind of a cookie licker on it, since I said I'd do it
[18:40] <gerrit-wm> New patchset: L10n-bot; "Localisation updates from http://translatewiki.net." [mediawiki/extensions/Cite] (master) - https://gerrit.wikimedia.org/r/8523
[18:40] <gerrit-wm> New patchset: L10n-bot; "Localisation updates from http://translatewiki.net." [mediawiki/extensions/CodeEditor] (master) - https://gerrit.wikimedia.org/r/8524
[18:40] ... mode/MediaWiki-General +o Joan by ChanServ
[18:40] ... mode/MediaWiki-General +b *!*@manganese.wikimedia.org by Joan
[18:40] <gerrit-wm> New patchset: L10n-bot; "Localisation updates from http://translatewiki.net." [mediawiki/extensions/CodeReview] (master) - https://gerrit.wikimedia.org/r/8525
[18:40] <Krenair> I knew this was going to happen.

[18:40] ... mode/MediaWiki-General -o Joan by ChanServ

The issue here seems to be that the l10nuser variable is "Translation updater bot (l10n-bot@translatewiki.net)", when it should be "L10n-bot (l10n-bot@translatewiki.net)".

Ryan Lane has tweaked this variable and will be submitting a change shortly, I believe.

(In reply to comment #15)

The issue here seems to be that the l10nuser variable is "Translation
updater bot (l10n-bot@translatewiki.net)", when it should be "L10n-bot
(l10n-bot@translatewiki.net)".

We already tried that and it didn't work. Any other ideas?

(In reply to comment #17)

(In reply to comment #15)

The issue here seems to be that the l10nuser variable is "Translation
updater bot (l10n-bot@translatewiki.net)", when it should be "L10n-bot
(l10n-bot@translatewiki.net)".

We already tried that and it didn't work. Any other ideas?

I'm not sure why this is so elusive. The logic looks sound to me:

From <>:

		if subject and options.uploader != hookconfig.l10nuser:
			message = "New patchset: " + re.sub(' \(.*', "", options.uploader) + '; "' + subject + '" [' + options.project + "] (" + options.branch + ") - " + options.changeurl + "\n"
			self.log_to_file(options.project, options.branch, message)

I believe this is the correct branch. Print options.uploader and hookconfig.l10nuser and see why they're not equal? If you believe there's still a bug here.

maybe the field as a trailing space or some invisible char?

the hookconfig.l10nuser field could be made to only be the email address and then use a regex match. That will discard various issues.

Are you sure there is a subject?

I would add a logging/debugging infrastructure to write some informations to some /var/log/gerrit/ log file. That will help a bit.

Is this fixed now? I can't find anything in my logs for gerrit-wm in MediaWiki-General mentioning the l10n-bot since the 14th of June, but it's definitely made a lot of changes in Gerrit.

(In reply to comment #20)

Is this fixed now? I can't find anything in my logs for gerrit-wm in MediaWiki-General
mentioning the l10n-bot since the 14th of June, but it's definitely made a lot
of changes in Gerrit.

Pretty sure I got it fixed in gerrit change 11615. If nobody's been seeing anything from L10n-bot (but has been seeing other stuff), then let's mark this FIXED :)