New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0.9.8: MailNotifier info gets "overwritten" if builders argument is the same #3398

Closed
larver-imvu opened this Issue Jun 30, 2017 · 6 comments

Comments

Projects
None yet
2 participants
@larver-imvu

larver-imvu commented Jun 30, 2017

I have a function mk_MailNotifiers like this:

    red_builders = ["foobuilder"]
    green_builders = ["foobuilder"]
    mns = []
    red_modes = ["failing", "warnings", "exception", "cancelled"]
    if red_builders:
        mns.append(reporters.MailNotifier(
            relayhost='smtp.corp.imvu.com',
            fromaddr='buildbot-red@imvu.com',
            sendToInterestedUsers=False,
            extraRecipients=["larver@imvu.com"],
            lookup="imvu.com",
            builders=red_builders,
            mode=red_modes,
            messageFormatter=formatter))
    if green_builders:
        mns.append(reporters.MailNotifier(
            relayhost='smtp.corp.imvu.com',
            fromaddr='buildbot@imvu.com',
            sendToInterestedUsers=False,
            extraRecipients=["larver@imvu.com"],
            lookup="imvu.com",
            builders=green_builders,
            mode='passing',
            messageFormatter=formatter))
    return mns

and I assign it in my buildmaster config to the c['services'] key.

If I run a build and cancel it, I do not get an email. If the build passes, then I do get an email.

If I reverse the order of the if branches, so that the green builder MailNotifier gets added first to the list, then I do get an email if I cancel a build.

So it looks like if the builders argument is the same for the two MailNotifiers, the one that comes last overwrites the earlier one(s).

I realize that in my case both red_builders and green_builders is the same, and that I should probably be using something more like mode="all" with just 1 MailNotifier, but I can't because I want red builds to have a different fromaddr argument (buildbot-red@imvu.com) than green builds.

Is this a legitimate bug or am I not using the MailNotifier correctly?

@larver-imvu

This comment has been minimized.

Show comment
Hide comment
@larver-imvu

larver-imvu Jun 30, 2017

I tested on 0.9.9 also and the same behavior is present.

larver-imvu commented Jun 30, 2017

I tested on 0.9.9 also and the same behavior is present.

@larver-imvu

This comment has been minimized.

Show comment
Hide comment
@larver-imvu

larver-imvu Jul 1, 2017

As a point of comparison, in Buildbot 0.8.5 (from which my code example above originates from), the behavior works as expected (no overwriting of settings).

larver-imvu commented Jul 1, 2017

As a point of comparison, in Buildbot 0.8.5 (from which my code example above originates from), the behavior works as expected (no overwriting of settings).

tardyp added a commit to tardyp/buildbot that referenced this issue Jul 4, 2017

@tardyp

This comment has been minimized.

Show comment
Hide comment
@tardyp

tardyp Jul 4, 2017

Member

looks like this is an issue with the algorithm with sets a name automatically for your notifier.

please see associated PR. can you set explicitly a name for your reporter as a workaround?

Member

tardyp commented Jul 4, 2017

looks like this is an issue with the algorithm with sets a name automatically for your notifier.

please see associated PR. can you set explicitly a name for your reporter as a workaround?

tardyp added a commit to tardyp/buildbot that referenced this issue Jul 4, 2017

@larver-imvu

This comment has been minimized.

Show comment
Hide comment
@larver-imvu

larver-imvu Jul 6, 2017

Unfortunately, setting an explicit name (I used name="green-mailer" and name="red-mailer") for each reporter does not change the behavior (on 0.9.9).

larver-imvu commented Jul 6, 2017

Unfortunately, setting an explicit name (I used name="green-mailer" and name="red-mailer") for each reporter does not change the behavior (on 0.9.9).

@larver-imvu

This comment has been minimized.

Show comment
Hide comment
@larver-imvu

larver-imvu Jul 6, 2017

Could it be that the new logic in e4c125a is not enough (maybe we need a self.name = name if name is not None)?

larver-imvu commented Jul 6, 2017

Could it be that the new logic in e4c125a is not enough (maybe we need a self.name = name if name is not None)?

@larver-imvu

This comment has been minimized.

Show comment
Hide comment
@larver-imvu

larver-imvu Jul 6, 2017

FWIW, I applied the patch in e4c125a locally and it does indeed fix the issue! I guess setting name=... is a more involved problem.

EDIT: I just tested again and can confirm that setting name=... (with different names, of course) for the two MailNotifiers is not a workaround.

larver-imvu commented Jul 6, 2017

FWIW, I applied the patch in e4c125a locally and it does indeed fix the issue! I guess setting name=... is a more involved problem.

EDIT: I just tested again and can confirm that setting name=... (with different names, of course) for the two MailNotifiers is not a workaround.

@tardyp tardyp closed this in #3412 Jul 6, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment