#52446 closed defect (fixed)
Buildbot failure emails go to too many recipients
Reported by: | ryandesign (Ryan Carsten Schmidt) | Owned by: | macports-tickets@… |
---|---|---|---|
Priority: | High | Milestone: | |
Component: | contrib | Version: | |
Keywords: | Cc: | mojca (Mojca Miklavec), jmroot (Joshua Root), neverpanic (Clemens Lang), raimue (Rainer Müller), larryv (Lawrence Velázquez) | |
Port: |
Description (last modified by 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.
Change History (11)
comment:1 Changed 8 years ago by ryandesign (Ryan Carsten Schmidt)
Description: | modified (diff) |
---|---|
Priority: | Normal → High |
comment:2 follow-up: 3 Changed 8 years ago by mojca (Mojca Miklavec)
comment:3 follow-up: 4 Changed 8 years ago by 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
comment:4 follow-up: 5 Changed 8 years ago by larryv (Lawrence Velázquez)
This would avoid creating a new object repeatedly:
-
master.cfg
682 682 683 683 # same as original, but calls portMessageFormatter with access to interested_users 684 684 def buildMessageDict(self, name, build, results): 685 self.interested_users.clear() 685 686 msgdict = self.portMessageFormatter(self.mode, name, build, results, 686 687 self.master_status, self.interested_users) 687 688 return msgdict 688 689
comment:5 Changed 8 years ago by ryandesign (Ryan Carsten Schmidt)
comment:7 follow-up: 8 Changed 8 years ago by ryandesign (Ryan Carsten Schmidt)
Resolution: | → fixed |
---|---|
Status: | new → closed |
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?
comment:8 follow-up: 10 Changed 8 years ago by larryv (Lawrence Velázquez)
Replying to ryandesign@…:
I don't understand how it works.
buildMessageDict
clearsinterested_users
, passesinterested_users
toportMessageFormatter
, which does not appear to use it in any way, and then somehow by the timeuseLookup
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.
comment:9 Changed 8 years ago by 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.
comment:10 follow-up: 11 Changed 8 years ago by mojca (Mojca Miklavec)
Replying to larryv@…:
Replying to ryandesign@…:
I don't understand how it works.
buildMessageDict
clearsinterested_users
, passesinterested_users
toportMessageFormatter
, which does not appear to use it in any way, and then somehow by the timeuseLookup
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
populatesinterested_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?
comment:11 Changed 8 years ago by 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.
I would guess that
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
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
works, so I cannot pretend I'm an expert.
Maybe we need to override / initialize some further variable of
PortsMailNotifier
.