stop #1656
stop #1656
Conversation
yetanotherion
force-pushed
the
master
branch
4 times, most recently
from
d050db1
to
fd625ce
Compare
| try: | ||
| b = yield self.master.db.buildrequests.claimBuildRequests(brids=[brid]) | ||
| except AlreadyClaimedError: | ||
| builds = yield self.master.data.get(("buildrequests", brid, "builds")) |
There was a problem hiding this comment.
There is a race condition here. The buildrequest might be claimed, but the build are not yet created. The claiming master has to find a slave, substanciate it, etc before creating the build.
As ee don't have a proper solution for this race condition, I would at least create a comment here stating this, and create a trac to follow up on this problem
|
|
||
|
|
||
| def triggeredBuildIsNotCreated(): | ||
| c = {} |
There was a problem hiding this comment.
you can still factorize those two functions. only 2 lines are different if I read correctly
yetanotherion
force-pushed
the
master
branch
2 times, most recently
from
905049a
to
8023b9c
Compare
|
@tardyp coment about test_stop_trigger factorization addressed |
|
👍 Please avoid rebase for the review comments addressing. This helps the review (can only review new patches) |
|
sorry for that, but a unit test failed in remove result. I preferred to integrate the fix in the related commit (2f4f24e) rather than modifying the git history (which seems like polluting it to me) |
|
(fyi tests are now ok in my station, let's see what travis says) |
build.py uses two attributes for the same goal: results and result. This commit gets rid of result, to be compatible with the overall nomenclature of the code. Note that as the result contains one verdict only, it seems a wider results -> result renaming would be appropriate. Change-Id: I463cd4467d093df939e8274a3101373d0169b8b5 Signed-off-by: Ion Alberdi <ialberdi@intel.com>
To be able to observe the events related to one build (by means of the method startConsuming) before the 'new' build event is triggered, this commit splits the addBuild method in two. Change-Id: I6d4ddbcbd02fab6233a3affee7dfba5bf7b22ec0 Signed-off-by: Ion Alberdi <ialberdi@intel.com>
this commit implements the stop of builds with associated integration tests. Change-Id: Ie81306b32cacebb1e277b8d3bf785a7ae2ccf2de Signed-off-by: Ion Alberdi <ialberdi@intel.com>
As the stop/cancel works by means of messages sent to the bus and associated callbacks, this commit removes the deprecated way of doing so. Change-Id: Id3480ef1b782762d9967b2a542db87b9e5f96cec Signed-off-by: Ion Alberdi <ialberdi@intel.com>
This commit adds a stop button in top of the build page. Change-Id: I9d95f5cab2e54f250dd0fa1dcb25da8a2ff269c8 Signed-off-by: Ion Alberdi <ialberdi@intel.com>
|
Some errors/timeout seem to have made the build fail The others are ok, does someone have rights to rebuild these two only ? |
Stop trigger integration tests, revelead a race condition. When updateSummary is called before the step is started, self.stepid is not set and thus the setStepStateString(self.stepid, ...) call failed. This commit fixes that issue. Change-Id: I725b9a4f791b1063bdac892be9dca27471f4b452 Signed-off-by: Ion Alberdi <ialberdi@intel.com>
|
👍 from travis |
| # Don't call the data API here, as the buildrequests might have been | ||
| # taken by another master. We just send the stop message and forget about those. | ||
| for b in builds: | ||
| self.master.mq.produce(("control", "builds", str(b['buildid']), "stop"), |
There was a problem hiding this comment.
Don't you need to set a default value while looking up for "reason" key into kwargs as you did below?
ie. dict(reason=kwargs.get('reason', 'no reason'))
|
@nam4dev comments addressed |
| @@ -533,22 +538,21 @@ def buildFinished(self, text, results): | |||
| state. | |||
|
|
|||
| It takes two arguments which describe the overall build status: | |||
| text, results. 'results' is one of SUCCESS, WARNINGS, or FAILURE. | |||
| text, results. 'results' is one of the possible results (see buildbot.status.results). | |||
There was a problem hiding this comment.
Somewhere in the code you use @type and @param to describe params, why not doing it here also?
With sphynx, you should be also able to do :ref:buildbot.status.results
|
👍 |
this commit implements two improvements on how the reason sent via the cancel message is computed: - setup a default value 'no reason' - the message is computed once for all messages to be sent. Change-Id: I4738e3f16606890a17c24f2d4eb5dad8b3d16597 Signed-off-by: Ion Alberdi <ialberdi@intel.com>
|
👍 |
following the POC from tardyp's commit.