[MacPorts] #52446: Buildbot failure emails go to too many recipients
#52446: Buildbot failure emails go to too many recipients --------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: defect | Status: new Priority: Normal | Milestone: Component: contrib | Version: Keywords: | Port: --------------------------+-------------------------------- 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 [browser:contrib/buildbot-test/master.cfg 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. -- Ticket URL: <https://trac.macports.org/ticket/52446> MacPorts <https://www.macports.org/> Ports system for the Mac operating system
#52446: Buildbot failure emails go to too many recipients ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: defect | Status: new Priority: High | Milestone: Component: contrib | Version: Resolution: | Keywords: Port: | ---------------------------+-------------------------------- Changes (by ryandesign@…): * priority: Normal => High Old description:
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 [browser:contrib/buildbot-test/master.cfg 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.
New description: 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 [browser:contrib/buildbot-test/master.cfg 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. -- -- Ticket URL: <https://trac.macports.org/ticket/52446#comment:1> MacPorts <https://www.macports.org/> Ports system for the Mac operating system
#52446: Buildbot failure emails go to too many recipients ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: defect | Status: new Priority: High | Milestone: Component: contrib | Version: Resolution: | Keywords: Port: | ---------------------------+-------------------------------- Comment (by mojca@…): 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`. -- Ticket URL: <https://trac.macports.org/ticket/52446#comment:2> MacPorts <https://www.macports.org/> Ports system for the Mac operating system
#52446: Buildbot failure emails go to too many recipients ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: defect | Status: new Priority: High | Milestone: Component: contrib | Version: Resolution: | Keywords: Port: | ---------------------------+-------------------------------- Comment (by raimue@…): 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 }}} -- Ticket URL: <https://trac.macports.org/ticket/52446#comment:3> MacPorts <https://www.macports.org/> Ports system for the Mac operating system
#52446: Buildbot failure emails go to too many recipients ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: defect | Status: new Priority: High | Milestone: Component: contrib | Version: Resolution: | Keywords: Port: | ---------------------------+-------------------------------- Comment (by larryv@…): This would avoid creating a new object repeatedly: {{{#!patch @@ -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 }}} -- Ticket URL: <https://trac.macports.org/ticket/52446#comment:4> MacPorts <https://www.macports.org/> Ports system for the Mac operating system
#52446: Buildbot failure emails go to too many recipients ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: defect | Status: new Priority: High | Milestone: Component: contrib | Version: Resolution: | Keywords: Port: | ---------------------------+-------------------------------- Comment (by ryandesign@…): Replying to [comment:4 larryv@…]:
This would avoid creating a new object repeatedly:
Ok, let's try that. -- Ticket URL: <https://trac.macports.org/ticket/52446#comment:5> MacPorts <https://www.macports.org/> Ports system for the Mac operating system
#52446: Buildbot failure emails go to too many recipients ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: defect | Status: closed Priority: High | Milestone: Component: contrib | Version: Resolution: fixed | Keywords: Port: | ---------------------------+-------------------------------- Changes (by ryandesign@…): * status: new => closed * resolution: => fixed Comment: 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? -- Ticket URL: <https://trac.macports.org/ticket/52446#comment:7> MacPorts <https://www.macports.org/> Ports system for the Mac operating system
#52446: Buildbot failure emails go to too many recipients ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: defect | Status: closed Priority: High | Milestone: Component: contrib | Version: Resolution: fixed | Keywords: Port: | ---------------------------+-------------------------------- Comment (by larryv@…): Replying to [comment:7 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. -- Ticket URL: <https://trac.macports.org/ticket/52446#comment:8> MacPorts <https://www.macports.org/> Ports system for the Mac operating system
#52446: Buildbot failure emails go to too many recipients ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: defect | Status: closed Priority: High | Milestone: Component: contrib | Version: Resolution: fixed | Keywords: Port: | ---------------------------+-------------------------------- Comment (by ryandesign@…): Dangit, I must have again been looking at my previous version of master.cfg where I had removed lines 750/751 for testing. Thanks. -- Ticket URL: <https://trac.macports.org/ticket/52446#comment:9> MacPorts <https://www.macports.org/> Ports system for the Mac operating system
#52446: Buildbot failure emails go to too many recipients ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: defect | Status: closed Priority: High | Milestone: Component: contrib | Version: Resolution: fixed | Keywords: Port: | ---------------------------+-------------------------------- Comment (by mojca@…): Replying to [comment:8 larryv@…]:
Replying to [comment:7 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? -- Ticket URL: <https://trac.macports.org/ticket/52446#comment:10> MacPorts <https://www.macports.org/> Ports system for the Mac operating system
#52446: Buildbot failure emails go to too many recipients ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: defect | Status: closed Priority: High | Milestone: Component: contrib | Version: Resolution: fixed | Keywords: Port: | ---------------------------+-------------------------------- Comment (by raimue@…): Replying to [comment:10 mojca@…]:
Does the committer also get a message about broken build?
No. I got an email for [https://build.macports.org/builders/ports-10 .8_x86_64_legacy-watcher/builds/1821 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. -- Ticket URL: <https://trac.macports.org/ticket/52446#comment:11> MacPorts <https://www.macports.org/> Ports system for the Mac operating system
participants (1)
-
MacPorts