[Buildbot] #3529: Add warning in trigger step if collapseBuild=True for the target builder
Buildbot trac
trac at buildbot.net
Tue May 10 20:22:24 UTC 2016
#3529: Add warning in trigger step if collapseBuild=True for the target builder
------------------------+--------------------
Reporter: tardyp | Owner:
Type: enhancement | Status: new
Priority: major | Milestone: 0.9.0
Version: master | Resolution:
Keywords: |
------------------------+--------------------
Description changed by tardyp:
Old description:
> as per discussion
>
> @djmitche> #topic collapseBuildRequest=False by default
> 18:41 <@djmitche> now, what WAS the topic
> 18:42 <@tardyp> collapseBuildRequest=True right now
> 18:42 <@djmitche> I shouldn't be allowed on irc
> 18:42 <@tardyp> and this does not play well with triggered build
> 18:42 <@tardyp> So people which use trigger step will all have the
> problem (I did 4 years ago)
> 18:42 <@tardyp> and ed did this week
> 18:43 @djmitche set the topic: A Software Freedom Conservancy Project |
> Buildbot-0.8.12 | docs: http://docs.buildbot.net/current/ | tutorial:
> http://docs.buildbot.net/current/tutorial |
> http://irclogs.jackgrigg.com/i[…]node.net/buildbot | meetings: Tuesdays
> 1630 UTC / https://titanpad.com/buildbot-agenda
> 18:43 <@tardyp> I would recommend to put it False by default, and to let
> people set it to true when they need to optimize
> 18:43 ⇐ @jaredgrubb quit (~Adium at 76.74.153.36) Quit: Leaving.
> 18:43 <@djmitche> changing a default is pretty ugly though
> 18:44 @djmitche set the topic: A Software Freedom Conservancy Project |
> Buildbot-0.8.12 | docs: http://docs.buildbot.net/current/ | tutorial:
> http://docs.buildbot.net/current/tutorial |
> https://irclogs.jackgrigg.com/irc.freenode.net/buildbot | meetings:
> Tuesdays 1630 UTC / https://titanpad.com/buildbot-agenda
> 18:44 → bb-github joined (~bb-github at 192.30.252.42)
> 18:44 <bb-github> [buildbot] mention-bot commented on issue #2141: By
> analyzing the blame information on this pull request, we identified
> @tomprince, @djmitche and @rutsky to be potential reviewers
> https://git.io/vwsl3
> 18:44 ← bb-github left (~bb-github at 192.30.252.42)
> 18:44 <@djmitche> is it possible to just disable it with triggered
> builds?
> 18:45 <@djmitche> my worry is, changing the default is pretty subtle, and
> will change behavior even for people not using triggered builds
> 18:45 <@tardyp> I tried to fix it that way, but there is no real way to
> be sure that a build is comming from a trigger step (from within the
> colapse br)
> 18:45 <cmouse> tardyp: i'm using the buildbot reporter myself, and it
> mostly works
> 18:45 <@tardyp> it will change the optimisation
> 18:45 <cmouse> tardyp: the only thing that is not working is that it's
> not reporting start of build
> 18:45 <@tomprince> The trigger scheduler could annotate the BR.
> 18:45 <cmouse> tardyp: i have no idea why it's not doing it :(
> 18:45 <@tardyp> while for people using trigger step, it just skip builds
> 18:46 <@djmitche> ^^ even as a temporary hack, I think that makes sense
> 18:46 <myheadhurts> So I'm running into the following issue:
> http://trac.buildbot.net/ticket/3528 and I'm thinking of approaching the
> fix in the following way: https://github.com/buildbot/buildbot/pull/2141
> 18:46 <myheadhurts> Would love any feedback on the approach
> 18:47 <@tardyp> ok. So we change the True behaviour to : compatible
> sourcestamps *and* not coming from a Trigger step
> 18:47 <@djmitche> #info build request collapsing and triggered builds do
> not get along well -- triggered builds end up skipped
> 18:47 <@tardyp> I'm fine with that too.
> 18:47 <@djmitche> tardyp: I think so -- that's even a good medium-term
> fix (since it sounds like this is architectural, not just a bug)
> 18:47 → poz2k4444 joined
> (uid22701 at gateway/web/irccloud.com/x-rugnxioslzukcfmd)
> 18:47 <bb-trac> [trac] #3528/undecided (new) updated by rutsky (Related
> PR: https://github.com/buildbot/buildbot/pull/2141)
> http://trac.buildbot.net/ticket/3528
> 18:48 <@djmitche> #agreed will configure collapsing to skip triggered
> builds by annotating the triggered builds
> 18:48 <@tomprince> If people want collapsing builds, then it isn't clear
> that skipping triggered builds isn't desired behavior.
> 18:48 <@djmitche> myheadhurts: without looking deeply at the PR, I'm
> really happy to see you working on the latent worker support :)
> 18:48 <@bdbaddog> so if you have multiple triggered builds waiting, they
> would no longer be collapsed?
> 18:48 <@tardyp> tomprince: I dont see a usecase where people would like
> to skip triggerred build
> 18:49 <@djmitche> a skipped triggered build has weird semantics if
> waitForFinish=True
> 18:49 <@djmitche> or whatever that parameter is
> 18:49 <@bdbaddog> yes.. deadlock..
> 18:49 <@tardyp> well no deadlock
> 18:49 <@tomprince> If you have a multi-stage build pipeline, but only
> care about results on the latest commit, if several triggers happen
> before the triggered build runs, there isn't a reason to run the earlier
> triggers.
> 18:49 <myheadhurts> djmitche: Hopefully soon (once company approves it)
> I'll have a PR for upgrading boto2 -> boto3 and handling cross-account
> support for AWS. :)
> 18:50 <@djmitche> oo, very nice!
> 18:50 <@djmitche> so regarding collapsing and triggering -- it sounds
> like this is something we should defer until 0.9.x
> 18:50 <@bdbaddog> parent build would hang forever if triggered build gets
> skipped..
> 18:50 <@tomprince> Probably doesn't make sense in the waitForFinish case
> (at least when that is being used for synchronization not just
> reporting).
> 18:50 <@djmitche> right
> 18:50 <@tomprince> Doesn
> 18:51 <@tomprince> t it just wait for the buildrequest to be completed,
> which does happen for collapsed requests?
> 18:51 → @jaredgrubb (opped) joined
> 18:51 <@tardyp> yes
> 18:51 — @tomprince can't remember the code, but seems to recall that was
> how things worked
> 18:51 <@tardyp> you will just see 4 builds when you awaited 8
> 18:52 <@tardyp> because the colapsing dont create build, it directly
> skips the br
> 18:52 <@tardyp> and that is fine with the waiting logic in trigger step,
> as it only waits for a complete event of the br
> 18:53 → bb-github joined (~bb-github at 192.30.252.41)
> 18:53 <bb-github> [buildbot] tomprince commented on issue #2141: I don't
> think this a correct fix. I think the intent of `build_wait_timeout == 0`
> is that the worker will only run a single build. I don't think this
> change preserves that behavior. https://git.io/vws81
> 18:53 ← bb-github left (~bb-github at 192.30.252.41)
> 18:54 <@tardyp> it actually waits for the buildset
> 18:54 <@djmitche> so how badly broken is it right now?
> 18:54 <@djmitche> It sounds like it works, it's just not the semantics
> you're expecting
> 18:54 <@djmitche> as in, you're expecting every triggered build to
> actually occur, and instead they get collapsed
> 18:55 <@tardyp> well you start 8 builds with trigger, you have no idea
> what collapsing is, and eventually you get 4 builds, and buildbot seems
> happy
> 18:55 <@tardyp> that is for me very weird for a new commer
> 18:55 <@tardyp> build request collapsing should for me be an advanced
> feature, that new commers should not be aware of
> 18:56 <@tomprince> I think that not collapsing is probably a slightly
> better default, but it isn't clear wether it is enough better to warrant
> *changing* the default.
> 18:56 <@djmitche> right
> 18:56 <@djmitche> we're already changing so much other stuff
> 18:56 <@djmitche> I think we should leave this be for now
> 18:56 <@tomprince> Given the experience of python 3, that seems
> reasonable.
> 18:57 <@tomprince> If you want to change the default, a slightly better
> option might be to deprecate having a default at all, and then remove the
> default.
> 18:57 <rutsky> can we somehow warn user that he might get not what he
> expected?
> 18:57 <rutsky> or add appropriate warning in docs?
> 18:57 <@tardyp> that is a good idea
> 18:57 <@tomprince> That seems sensible.
> 18:58 → bb-github joined (~bb-github at 192.30.252.41)
> 18:58 <bb-github> [buildbot] aelsabbahy commented on issue #2141: Hmm,
> that's definitely not what I want then. Since it's very important for us
> that we only have single use workers.... https://git.io/vws4o
> 18:58 ← bb-github left (~bb-github at 192.30.252.41)
> 18:58 <@tardyp> in the trigger step, if the triggered builder has True in
> collapse, we warn in the stdio
> 18:58 <@djmitche> that sounds good
> 18:58 <cmouse> should write a trac issue about OpenStackLatentWorker not
> always killing the worker.
> 18:58 <@tardyp> with link to the doc
New description:
as per discussion
{{{
@djmitche> #topic collapseBuildRequest=False by default
18:41 <@djmitche> now, what WAS the topic
18:42 <@tardyp> collapseBuildRequest=True right now
18:42 <@djmitche> I shouldn't be allowed on irc
18:42 <@tardyp> and this does not play well with triggered build
18:42 <@tardyp> So people which use trigger step will all have the problem
(I did 4 years ago)
18:42 <@tardyp> and ed did this week
18:43 @djmitche set the topic: A Software Freedom Conservancy Project |
Buildbot-0.8.12 | docs: http://docs.buildbot.net/current/ | tutorial:
http://docs.buildbot.net/current/tutorial |
http://irclogs.jackgrigg.com/i[…]node.net/buildbot | meetings: Tuesdays
1630 UTC / https://titanpad.com/buildbot-agenda
18:43 <@tardyp> I would recommend to put it False by default, and to let
people set it to true when they need to optimize
18:43 ⇐ @jaredgrubb quit (~Adium at 76.74.153.36) Quit: Leaving.
18:43 <@djmitche> changing a default is pretty ugly though
18:44 @djmitche set the topic: A Software Freedom Conservancy Project |
Buildbot-0.8.12 | docs: http://docs.buildbot.net/current/ | tutorial:
http://docs.buildbot.net/current/tutorial |
https://irclogs.jackgrigg.com/irc.freenode.net/buildbot | meetings:
Tuesdays 1630 UTC / https://titanpad.com/buildbot-agenda
18:44 → bb-github joined (~bb-github at 192.30.252.42)
18:44 <bb-github> [buildbot] mention-bot commented on issue #2141: By
analyzing the blame information on this pull request, we identified
@tomprince, @djmitche and @rutsky to be potential reviewers
https://git.io/vwsl3
18:44 ← bb-github left (~bb-github at 192.30.252.42)
18:44 <@djmitche> is it possible to just disable it with triggered builds?
18:45 <@djmitche> my worry is, changing the default is pretty subtle, and
will change behavior even for people not using triggered builds
18:45 <@tardyp> I tried to fix it that way, but there is no real way to be
sure that a build is comming from a trigger step (from within the colapse
br)
18:45 <cmouse> tardyp: i'm using the buildbot reporter myself, and it
mostly works
18:45 <@tardyp> it will change the optimisation
18:45 <cmouse> tardyp: the only thing that is not working is that it's not
reporting start of build
18:45 <@tomprince> The trigger scheduler could annotate the BR.
18:45 <cmouse> tardyp: i have no idea why it's not doing it :(
18:45 <@tardyp> while for people using trigger step, it just skip builds
18:46 <@djmitche> ^^ even as a temporary hack, I think that makes sense
18:46 <myheadhurts> So I'm running into the following issue:
http://trac.buildbot.net/ticket/3528 and I'm thinking of approaching the
fix in the following way: https://github.com/buildbot/buildbot/pull/2141
18:46 <myheadhurts> Would love any feedback on the approach
18:47 <@tardyp> ok. So we change the True behaviour to : compatible
sourcestamps *and* not coming from a Trigger step
18:47 <@djmitche> #info build request collapsing and triggered builds do
not get along well -- triggered builds end up skipped
18:47 <@tardyp> I'm fine with that too.
18:47 <@djmitche> tardyp: I think so -- that's even a good medium-term fix
(since it sounds like this is architectural, not just a bug)
18:47 → poz2k4444 joined
(uid22701 at gateway/web/irccloud.com/x-rugnxioslzukcfmd)
18:47 <bb-trac> [trac] #3528/undecided (new) updated by rutsky (Related
PR: https://github.com/buildbot/buildbot/pull/2141)
http://trac.buildbot.net/ticket/3528
18:48 <@djmitche> #agreed will configure collapsing to skip triggered
builds by annotating the triggered builds
18:48 <@tomprince> If people want collapsing builds, then it isn't clear
that skipping triggered builds isn't desired behavior.
18:48 <@djmitche> myheadhurts: without looking deeply at the PR, I'm
really happy to see you working on the latent worker support :)
18:48 <@bdbaddog> so if you have multiple triggered builds waiting, they
would no longer be collapsed?
18:48 <@tardyp> tomprince: I dont see a usecase where people would like to
skip triggerred build
18:49 <@djmitche> a skipped triggered build has weird semantics if
waitForFinish=True
18:49 <@djmitche> or whatever that parameter is
18:49 <@bdbaddog> yes.. deadlock..
18:49 <@tardyp> well no deadlock
18:49 <@tomprince> If you have a multi-stage build pipeline, but only care
about results on the latest commit, if several triggers happen before the
triggered build runs, there isn't a reason to run the earlier triggers.
18:49 <myheadhurts> djmitche: Hopefully soon (once company approves it)
I'll have a PR for upgrading boto2 -> boto3 and handling cross-account
support for AWS. :)
18:50 <@djmitche> oo, very nice!
18:50 <@djmitche> so regarding collapsing and triggering -- it sounds like
this is something we should defer until 0.9.x
18:50 <@bdbaddog> parent build would hang forever if triggered build gets
skipped..
18:50 <@tomprince> Probably doesn't make sense in the waitForFinish case
(at least when that is being used for synchronization not just reporting).
18:50 <@djmitche> right
18:50 <@tomprince> Doesn
18:51 <@tomprince> t it just wait for the buildrequest to be completed,
which does happen for collapsed requests?
18:51 → @jaredgrubb (opped) joined
18:51 <@tardyp> yes
18:51 — @tomprince can't remember the code, but seems to recall that was
how things worked
18:51 <@tardyp> you will just see 4 builds when you awaited 8
18:52 <@tardyp> because the colapsing dont create build, it directly skips
the br
18:52 <@tardyp> and that is fine with the waiting logic in trigger step,
as it only waits for a complete event of the br
18:53 → bb-github joined (~bb-github at 192.30.252.41)
18:53 <bb-github> [buildbot] tomprince commented on issue #2141: I don't
think this a correct fix. I think the intent of `build_wait_timeout == 0`
is that the worker will only run a single build. I don't think this change
preserves that behavior. https://git.io/vws81
18:53 ← bb-github left (~bb-github at 192.30.252.41)
18:54 <@tardyp> it actually waits for the buildset
18:54 <@djmitche> so how badly broken is it right now?
18:54 <@djmitche> It sounds like it works, it's just not the semantics
you're expecting
18:54 <@djmitche> as in, you're expecting every triggered build to
actually occur, and instead they get collapsed
18:55 <@tardyp> well you start 8 builds with trigger, you have no idea
what collapsing is, and eventually you get 4 builds, and buildbot seems
happy
18:55 <@tardyp> that is for me very weird for a new commer
18:55 <@tardyp> build request collapsing should for me be an advanced
feature, that new commers should not be aware of
18:56 <@tomprince> I think that not collapsing is probably a slightly
better default, but it isn't clear wether it is enough better to warrant
*changing* the default.
18:56 <@djmitche> right
18:56 <@djmitche> we're already changing so much other stuff
18:56 <@djmitche> I think we should leave this be for now
18:56 <@tomprince> Given the experience of python 3, that seems
reasonable.
18:57 <@tomprince> If you want to change the default, a slightly better
option might be to deprecate having a default at all, and then remove the
default.
18:57 <rutsky> can we somehow warn user that he might get not what he
expected?
18:57 <rutsky> or add appropriate warning in docs?
18:57 <@tardyp> that is a good idea
18:57 <@tomprince> That seems sensible.
18:58 → bb-github joined (~bb-github at 192.30.252.41)
18:58 <bb-github> [buildbot] aelsabbahy commented on issue #2141: Hmm,
that's definitely not what I want then. Since it's very important for us
that we only have single use workers.... https://git.io/vws4o
18:58 ← bb-github left (~bb-github at 192.30.252.41)
18:58 <@tardyp> in the trigger step, if the triggered builder has True in
collapse, we warn in the stdio
18:58 <@djmitche> that sounds good
18:58 <cmouse> should write a trac issue about OpenStackLatentWorker not
always killing the worker.
18:58 <@tardyp> with link to the doc
}}}
--
--
Ticket URL: <http://trac.buildbot.net/ticket/3529#comment:1>
Buildbot <http://buildbot.net/>
Buildbot: build/test automation
More information about the bugs
mailing list