Buildbot failure emails go to too many recipients

[ryandesign (Ryan Carsten Schmidt)]

Each buildbot failure email is sent not only to the maintainers of the ports affected by this build failure, but also to the recipients of all previous build failure emails.

I've looked at the code​ for a long time and don't understand it. Obviously the list of interested users isn't being cleared at the beginning of each build. But I don't see where in the code anything gets added to the list of interested users. I see that we have a custom PortsMailNotifier class that inherits from MailNotifier, which is "same as original, but calls portMessageFormatter with access to interested_users". But I don't see where portWatcherMessageFormatter ever makes use of the interested_users argument its been passed.

[mojca (Mojca Miklavec)]

I would guess that

class PortsMailNotifier(MailNotifier, object):
    def __init__(self, fromaddr, *args, **kwargs):
        self.interested_users = set()

should reset the list of recipients to an empty set, but I don't fully understand why it doesn't.

You could try one more thing, adding

interested_users = set()

somewhere on top of portWatcherMessageFormatter.

One thing I'm not sure about is when and how the committer gets added and I'm not sure how

def useLookup(self, build):
    ...
    return defer.gatherResults(dl)

works, so I cannot pretend I'm an expert.

Maybe we need to override / initialize some further variable of PortsMailNotifier.

[raimue (Rainer Müller)]

There is only one instance of PortsMailNotifier and it is created in our master.cfg. Therefore the code in __init__ is only run once.

As I understand it, the interested_users attribute is only used to pass the list of users from the portMessageFormatter to useLookup as there is no direct way for interaction.

Maybe it is as simple as this:

Index: master.cfg
===================================================================
--- master.cfg	(revision 153357)
+++ master.cfg	(working copy)
@@ -682,6 +682,7 @@
 
     # same as original, but calls portMessageFormatter with access to interested_users
     def buildMessageDict(self, name, build, results):
+        self.interested_users = set()
         msgdict = self.portMessageFormatter(self.mode, name, build, results,
                                             self.master_status, self.interested_users)
         return msgdict

[larryv (Lawrence Velázquez)]

This would avoid creating a new object repeatedly:

@@ -682,7 +682,8 @@

     # same as original, but calls portMessageFormatter with access to interested_users
     def buildMessageDict(self, name, build, results):
+        self.interested_users.clear()
         msgdict = self.portMessageFormatter(self.mode, name, build, results,
                                             self.master_status, self.interested_users)
         return msgdict

[ryandesign (Ryan Carsten Schmidt)]

Replying to larryv@…:

This would avoid creating a new object repeatedly:

Ok, let's try that.

[larryv (Lawrence Velázquez)]

Cc Me!

[ryandesign (Ryan Carsten Schmidt)]

This works, thanks. r153387

I don't understand how it works. buildMessageDict clears interested_users, passes interested_users to portMessageFormatter, which does not appear to use it in any way, and then somehow by the time useLookup is called, it's been filled with the right data?

[larryv (Lawrence Velázquez)]

Replying to ryandesign@…:

I don't understand how it works. buildMessageDict clears interested_users, passes interested_users to portMessageFormatter, which does not appear to use it in any way, and then somehow by the time useLookup is called, it's been filled with the right data?

portMessageFormatter populates interested_users in line 750 or thereabouts. I don’t really understand why it happens in that function.

[ryandesign (Ryan Carsten Schmidt)]

Dangit, I must have again been looking at my previous version of master.cfg where I had removed lines 750/751 for testing. Thanks.

[mojca (Mojca Miklavec)]

Replying to larryv@…:

Replying to ryandesign@…:

I don't understand how it works. buildMessageDict clears interested_users, passes interested_users to portMessageFormatter, which does not appear to use it in any way, and then somehow by the time useLookup is called, it's been filled with the right data?

You can look at what MailNotifier does in the buildbot sources. The PortsMailNotifier only overrides some functions. The useLookup is called by one of the class functions.

buildMessageDict() initializes a field that was added by us. Then it "forwards" that field to portMessageFormatter(). Not that self.interested_users is an output of portMessageFormatter(), not input. Now that I look at it again I see that I could probably do it differently, for example return it from portWatcherMessageFormatter() as part of msgdict.

So portWatcherMessageFormatter() fills in the list of interested users. And then useLookup() (called by another function) uses that list to add actual recipients.

portMessageFormatter populates interested_users in line 750 or thereabouts. I don’t really understand why it happens in that function.

Because that's a place where we know them already. We could of course duplicate the code that calculates the recipients (we could recalculate the recipients inside useLookup()) or we could write another class to do it. You can check how that was done in the old setup. It's not nearly ideal to do it that way, I know, but I had to do something ...

Does the committer also get a message about broken build?

[raimue (Rainer Müller)]

Replying to mojca@…:

Does the committer also get a message about broken build?

No. I got an email for ​a failing build, but the committer was not in To or Cc.

I think the list of committers could be obtained from build.getInterestedUsers() as in the old buildbot code.