From c047fa9ce44d7b677a2256240340dde57b99c02a Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Thu, 25 Sep 2014 16:36:12 -0400 Subject: [PATCH 1/8] change step state_strings to state_string: DB model --- .../db/migrate/versions/038_step_string.py | 30 +++++++ master/buildbot/db/model.py | 3 +- ...est_db_migrate_versions_038_step_string.py | 78 +++++++++++++++++++ 3 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 master/buildbot/db/migrate/versions/038_step_string.py create mode 100644 master/buildbot/test/unit/test_db_migrate_versions_038_step_string.py diff --git a/master/buildbot/db/migrate/versions/038_step_string.py b/master/buildbot/db/migrate/versions/038_step_string.py new file mode 100644 index 00000000000..c6b78ec68e4 --- /dev/null +++ b/master/buildbot/db/migrate/versions/038_step_string.py @@ -0,0 +1,30 @@ +# This file is part of Buildbot. Buildbot is free software: you can +# redistribute it and/or modify it under the terms of the GNU General Public +# License as published by the Free Software Foundation, version 2. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more +# details. +# +# You should have received a copy of the GNU General Public License along with +# this program; if not, write to the Free Software Foundation, Inc., 51 +# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +# +# Copyright Buildbot Team Members + +import sqlalchemy as sa + + +def upgrade(migrate_engine): + metadata = sa.MetaData() + metadata.bind = migrate_engine + + steps_table = sa.Table('steps', metadata, autoload=True) + + # no attempt is made here to move data from one table to the other, since + # there was no released version of Buildbot with a 'steps' table yet. + + col = sa.Column('state_string', sa.Text, nullable=False, server_default='') + col.create(steps_table) + steps_table.c.state_strings_json.drop() diff --git a/master/buildbot/db/model.py b/master/buildbot/db/model.py index 91ebfd4be1a..78e09952893 100644 --- a/master/buildbot/db/model.py +++ b/master/buildbot/db/model.py @@ -130,8 +130,7 @@ class Model(base.DBConnectorComponent): sa.Column('buildid', sa.Integer, sa.ForeignKey('builds.id')), sa.Column('started_at', sa.Integer), sa.Column('complete_at', sa.Integer), - # a list of strings describing the step's state - sa.Column('state_strings_json', sa.Text, nullable=False), + sa.Column('state_string', sa.Text, nullable=False, server_default=''), sa.Column('results', sa.Integer), sa.Column('urls_json', sa.Text, nullable=False), ) diff --git a/master/buildbot/test/unit/test_db_migrate_versions_038_step_string.py b/master/buildbot/test/unit/test_db_migrate_versions_038_step_string.py new file mode 100644 index 00000000000..9d43aadda59 --- /dev/null +++ b/master/buildbot/test/unit/test_db_migrate_versions_038_step_string.py @@ -0,0 +1,78 @@ +# This file is part of Buildbot. Buildbot is free software: you can +# redistribute it and/or modify it under the terms of the GNU General Public +# License as published by the Free Software Foundation, version 2. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more +# details. +# +# You should have received a copy of the GNU General Public License along with +# this program; if not, write to the Free Software Foundation, Inc., 51 +# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +# +# Copyright Buildbot Team Members + +import sqlalchemy as sa + +from buildbot.test.util import migration +from twisted.trial import unittest + + +class Migration(migration.MigrateTestMixin, unittest.TestCase): + + def setUp(self): + return self.setUpMigrateTest() + + def tearDown(self): + return self.tearDownMigrateTest() + + def create_tables_thd(self, conn): + metadata = sa.MetaData() + metadata.bind = conn + + builds = sa.Table('builds', metadata, + sa.Column('id', sa.Integer, primary_key=True), + sa.Column('number', sa.Integer, nullable=False), + sa.Column('builderid', sa.Integer), + sa.Column('buildrequestid', sa.Integer, nullable=False), + sa.Column('buildslaveid', sa.Integer), + sa.Column('masterid', sa.Integer, nullable=False), + sa.Column('started_at', sa.Integer, nullable=False), + sa.Column('complete_at', sa.Integer), + sa.Column('state_strings_json', sa.Text, nullable=False), + sa.Column('results', sa.Integer), + ) + builds.create() + + steps = sa.Table('steps', metadata, + sa.Column('id', sa.Integer, primary_key=True), + sa.Column('number', sa.Integer, nullable=False), + sa.Column('name', sa.String(50), nullable=False), + sa.Column('buildid', sa.Integer, sa.ForeignKey('builds.id')), + sa.Column('started_at', sa.Integer), + sa.Column('complete_at', sa.Integer), + sa.Column('state_strings_json', sa.Text, nullable=False), + sa.Column('results', sa.Integer), + sa.Column('urls_json', sa.Text, nullable=False), + ) + steps.create() + + # tests + + def test_update(self): + def setup_thd(conn): + self.create_tables_thd(conn) + + def verify_thd(conn): + metadata = sa.MetaData() + metadata.bind = conn + + steps = sa.Table('steps', metadata, autoload=True) + self.failIf(hasattr(steps.c, 'state_strings_json')) + self.failUnless(hasattr(steps.c, 'state_string')) + self.assertIsInstance(steps.c.state_string.type, sa.Text) + + # TODO: also test that data is transferred? Or just hell with it + + return self.do_test_migration(37, 38, setup_thd, verify_thd) From 51cc13f67c72e889cc4f0c13769aabe135908990 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Thu, 25 Sep 2014 16:25:24 -0400 Subject: [PATCH 2/8] change step state_strings to state_string: DB API --- master/buildbot/db/steps.py | 12 +++--- master/buildbot/test/fake/fakedb.py | 20 ++++----- master/buildbot/test/unit/test_db_steps.py | 49 +++++++++++----------- master/buildbot/test/util/validation.py | 4 +- master/docs/developer/database.rst | 12 +++--- 5 files changed, 48 insertions(+), 49 deletions(-) diff --git a/master/buildbot/db/steps.py b/master/buildbot/db/steps.py index 4d93072974b..64cc8fece82 100644 --- a/master/buildbot/db/steps.py +++ b/master/buildbot/db/steps.py @@ -63,9 +63,7 @@ def thd(conn): return [self._stepdictFromRow(row) for row in res.fetchall()] return self.db.pool.do(thd) - def addStep(self, buildid, name, state_strings): - state_strings_json = json.dumps(state_strings) - + def addStep(self, buildid, name, state_string): def thd(conn): tbl = self.db.model.steps # get the highest current number @@ -79,7 +77,7 @@ def thd(conn): # conflict, then the name is likely already taken. insert_row = dict(buildid=buildid, number=number, started_at=None, complete_at=None, - state_strings_json=state_strings_json, + state_string=state_string, urls_json='[]', name=name) try: r = conn.execute(self.db.model.steps.insert(), insert_row) @@ -118,11 +116,11 @@ def thd(conn): conn.execute(q, started_at=started_at) return self.db.pool.do(thd) - def setStepStateStrings(self, stepid, state_strings): + def setStepStateString(self, stepid, state_string): def thd(conn): tbl = self.db.model.steps q = tbl.update(whereclause=(tbl.c.id == stepid)) - conn.execute(q, state_strings_json=json.dumps(state_strings)) + conn.execute(q, state_string=state_string) return self.db.pool.do(thd) def addURL(self, stepid, name, url, _racehook=None): @@ -176,6 +174,6 @@ def mkdt(epoch): buildid=row.buildid, started_at=mkdt(row.started_at), complete_at=mkdt(row.complete_at), - state_strings=json.loads(row.state_strings_json), + state_string=row.state_string, results=row.results, urls=json.loads(row.urls_json)) diff --git a/master/buildbot/test/fake/fakedb.py b/master/buildbot/test/fake/fakedb.py index 8bad75ee3a8..7817d2b2b3a 100644 --- a/master/buildbot/test/fake/fakedb.py +++ b/master/buildbot/test/fake/fakedb.py @@ -472,7 +472,7 @@ class Step(Row): buildid=None, started_at=1304262222, complete_at=None, - state_strings_json='[]', + state_string='', results=None, urls_json='[]') @@ -1755,7 +1755,7 @@ def _row2dict(self, row): name=row['name'], started_at=_mkdt(row['started_at']), complete_at=_mkdt(row['complete_at']), - state_strings=json.loads(row['state_strings_json']), + state_string=row['state_string'], results=row['results'], urls=json.loads(row['urls_json'])) @@ -1789,9 +1789,9 @@ def getSteps(self, buildid): ret.sort(key=lambda r: r['number']) return defer.succeed(ret) - def addStep(self, buildid, name, state_strings, _reactor=reactor): - validation.verifyType(self.t, 'state_strings', state_strings, - validation.ListValidator(validation.StringValidator())) + def addStep(self, buildid, name, state_string, _reactor=reactor): + validation.verifyType(self.t, 'state_string', state_string, + validation.StringValidator()) validation.verifyType(self.t, 'name', name, validation.IdentifierValidator(50)) # get a unique name and number @@ -1817,7 +1817,7 @@ def addStep(self, buildid, name, state_strings, _reactor=reactor): 'started_at': None, 'complete_at': None, 'results': None, - 'state_strings_json': unicode(json.dumps(state_strings)), + 'state_string': state_string, 'urls_json': '[]'} return defer.succeed((id, number, name)) @@ -1828,12 +1828,12 @@ def startStep(self, stepid, _reactor=reactor): b['started_at'] = _reactor.seconds() return defer.succeed(None) - def setStepStateStrings(self, stepid, state_strings): - validation.verifyType(self.t, 'state_strings', state_strings, - validation.ListValidator(validation.StringValidator())) + def setStepStateString(self, stepid, state_string): + validation.verifyType(self.t, 'state_string', state_string, + validation.StringValidator()) b = self.steps.get(stepid) if b: - b['state_strings_json'] = json.dumps(state_strings) + b['state_string'] = state_string return defer.succeed(None) def addURL(self, stepid, name, url, _racehook=None): diff --git a/master/buildbot/test/unit/test_db_steps.py b/master/buildbot/test/unit/test_db_steps.py index 248c3defe19..35c31f57679 100644 --- a/master/buildbot/test/unit/test_db_steps.py +++ b/master/buildbot/test/unit/test_db_steps.py @@ -50,10 +50,10 @@ class Tests(interfaces.InterfaceTests): stepRows = [ fakedb.Step(id=70, number=0, name='one', buildid=30, started_at=TIME1, complete_at=TIME2, - state_strings_json=u'["test"]', results=0), + state_string=u'test', results=0), fakedb.Step(id=71, number=1, name='two', buildid=30, started_at=TIME2, complete_at=TIME3, - state_strings_json=u'["test"]', results=2, + state_string=u'test', results=2, urls_json=u'["http://url"]'), fakedb.Step(id=72, number=2, name='three', buildid=30, started_at=TIME3), @@ -64,19 +64,19 @@ class Tests(interfaces.InterfaceTests): 'results': 0, 'started_at': epoch2datetime(TIME1), 'complete_at': epoch2datetime(TIME2), - 'state_strings': [u'test'], + 'state_string': u'test', 'urls': []}, {'id': 71, 'buildid': 30, 'number': 1, 'name': u'two', 'results': 2, 'started_at': epoch2datetime(TIME2), 'complete_at': epoch2datetime(TIME3), - 'state_strings': [u'test'], + 'state_string': u'test', 'urls': [u'http://url']}, {'id': 72, 'buildid': 30, 'number': 2, 'name': u'three', 'results': None, 'started_at': epoch2datetime(TIME3), 'complete_at': None, - 'state_strings': [], + 'state_string': u'', 'urls': []}, ] @@ -94,7 +94,7 @@ def getSteps(self, buildid): def test_signature_addStep(self): @self.assertArgSpecMatches(self.db.steps.addStep) - def addStep(self, buildid, name, state_strings): + def addStep(self, buildid, name, state_string): pass def test_signature_startStep(self): @@ -102,9 +102,9 @@ def test_signature_startStep(self): def addStep(self, stepid): pass - def test_signature_setStepStateStrings(self): - @self.assertArgSpecMatches(self.db.steps.setStepStateStrings) - def setStepStateStrings(self, stepid, state_strings): + def test_signature_setStepStateString(self): + @self.assertArgSpecMatches(self.db.steps.setStepStateString) + def setStepStateString(self, stepid, state_string): pass def test_signature_finishStep(self): @@ -177,7 +177,7 @@ def test_addStep_getStep(self): clock.advance(TIME1) yield self.insertTestData(self.backgroundData) stepid, number, name = yield self.db.steps.addStep(buildid=30, - name=u'new', state_strings=[u'new']) + name=u'new', state_string=u'new') yield self.db.steps.startStep(stepid=stepid, _reactor=clock) self.assertEqual((number, name), (0, 'new')) stepdict = yield self.db.steps.getStep(stepid=stepid) @@ -190,7 +190,7 @@ def test_addStep_getStep(self): 'started_at': epoch2datetime(TIME1), 'complete_at': None, 'results': None, - 'state_strings': [u'new'], + 'state_string': u'new', 'urls': []}) @defer.inlineCallbacks @@ -198,8 +198,8 @@ def test_addStep_getStep_existing_step(self): clock = task.Clock() clock.advance(TIME1) yield self.insertTestData(self.backgroundData + [self.stepRows[0]]) - stepid, number, name = yield self.db.steps.addStep(buildid=30, - name=u'new', state_strings=[u'new']) + stepid, number, name = yield self.db.steps.addStep( + buildid=30, name=u'new', state_string=u'new') yield self.db.steps.startStep(stepid=stepid, _reactor=clock) self.assertEqual((number, name), (1, 'new')) stepdict = yield self.db.steps.getStep(stepid=stepid) @@ -217,8 +217,8 @@ def test_addStep_getStep_name_collisions(self): fakedb.Step(id=75, number=2, name=u'new_2', buildid=30), fakedb.Step(id=76, number=3, name=u'new_step', buildid=30), ]) - stepid, number, name = yield self.db.steps.addStep(buildid=30, - name=u'new', state_strings=[u'new']) + stepid, number, name = yield self.db.steps.addStep( + buildid=30, name=u'new', state_string=u'new') yield self.db.steps.startStep(stepid=stepid, _reactor=clock) self.assertEqual((number, name), (4, u'new_3')) stepdict = yield self.db.steps.getStep(stepid=stepid) @@ -227,12 +227,12 @@ def test_addStep_getStep_name_collisions(self): self.assertEqual(stepdict['name'], name) @defer.inlineCallbacks - def test_setStepStateStrings(self): + def test_setStepStateString(self): yield self.insertTestData(self.backgroundData + [self.stepRows[2]]) - yield self.db.steps.setStepStateStrings(stepid=72, - state_strings=[u'aaa', u'bbb']) + yield self.db.steps.setStepStateString(stepid=72, + state_string=u'aaa') stepdict = yield self.db.steps.getStep(stepid=72) - self.assertEqual(stepdict['state_strings'], [u'aaa', u'bbb']) + self.assertEqual(stepdict['state_string'], u'aaa') @defer.inlineCallbacks def test_addURL(self): @@ -247,7 +247,8 @@ def test_addURL_race(self): yield self.insertTestData(self.backgroundData + [self.stepRows[2]]) yield defer.gatherResults([ # only a tiny sleep is required to see the problem. - self.db.steps.addURL(stepid=72, name=u'foo', url=u'bar', _racehook=lambda: time.sleep(.01)), + self.db.steps.addURL(stepid=72, name=u'foo', url=u'bar', + _racehook=lambda: time.sleep(.01)), self.db.steps.addURL(stepid=72, name=u'foo2', url=u'bar2')]) stepdict = yield self.db.steps.getStep(stepid=72) @@ -278,8 +279,8 @@ def test_addStep_getStep_name_collisions_too_long(self): fakedb.Step(id=73, number=0, name=u'a' * 49, buildid=30), fakedb.Step(id=74, number=1, name=u'a' * 48 + '_1', buildid=30), ]) - stepid, number, name = yield self.db.steps.addStep(buildid=30, - name=u'a' * 49, state_strings=[u'new']) + stepid, number, name = yield self.db.steps.addStep( + buildid=30, name=u'a' * 49, state_string=u'new') yield self.db.steps.startStep(stepid=stepid, _reactor=clock) self.assertEqual((number, name), (2, u'a' * 48 + '_2')) stepdict = yield self.db.steps.getStep(stepid=stepid) @@ -298,8 +299,8 @@ def test_addStep_getStep_name_collisions_too_long_extra_digits(self): ] + [fakedb.Step(id=73 + i, number=i, name=u'a' * 47 + ('_%d' % i), buildid=30) for i in range(10, 100) ]) - stepid, number, name = yield self.db.steps.addStep(buildid=30, - name=u'a' * 50, state_strings=[u'new']) + stepid, number, name = yield self.db.steps.addStep( + buildid=30, name=u'a' * 50, state_string=u'new') yield self.db.steps.startStep(stepid=stepid, _reactor=clock) self.assertEqual((number, name), (100, u'a' * 46 + '_100')) stepdict = yield self.db.steps.getStep(stepid=stepid) diff --git a/master/buildbot/test/util/validation.py b/master/buildbot/test/util/validation.py index f09297f0776..2039919998f 100644 --- a/master/buildbot/test/util/validation.py +++ b/master/buildbot/test/util/validation.py @@ -569,7 +569,7 @@ def validate(self, name, arg_object): started_at=IntValidator(), complete=BooleanValidator(), complete_at=NoneOk(IntValidator()), - state_strings=ListValidator(StringValidator()), + state_string=StringValidator(), results=NoneOk(IntValidator()), urls=ListValidator(StringValidator()), ) @@ -590,7 +590,7 @@ def validate(self, name, arg_object): buildid=IntValidator(), started_at=DateTimeValidator(), complete_at=NoneOk(DateTimeValidator()), - state_strings=ListValidator(StringValidator()), + state_string=StringValidator(), results=NoneOk(IntValidator()), urls=ListValidator(StringValidator()), ) diff --git a/master/docs/developer/database.rst b/master/docs/developer/database.rst index ca24045f47f..54935e3e0d5 100644 --- a/master/docs/developer/database.rst +++ b/master/docs/developer/database.rst @@ -342,7 +342,7 @@ steps * ``buildid`` (the ID of the build containing this step) * ``started_at`` (datetime at which this step began) * ``complete_at`` (datetime at which this step finished, or None if it is ongoing) - * ``state_strings`` (list of short strings describing the step's state) + * ``state_string`` (short string describing the step's state) * ``results`` (results of this step; see :ref:`Build-Result-Codes`) * ``urls`` (list of URLs produced by this step. Each urls is stored as a dictionary with keys `name` and `url`) @@ -369,24 +369,24 @@ steps Get all steps in the given build, in order by number. - .. py:method:: addStep(self, buildid, name, state_strings) + .. py:method:: addStep(self, buildid, name, state_string) :param integer buildid: the build to which to add the step :param name: the step name :type name: 50-character :ref:`identifier ` - :param list state_strings: the initial state of the step + :param list state_string: the initial state of the step :returns: tuple of step ID, step number, and step name, via Deferred Add a new step to a build. The given name will be used if it is unique; otherwise, a unique numerical suffix will be appended. - .. py:method:: setStepStateStrings(stepid, state_strings): + .. py:method:: setStepStateString(stepid, state_string): :param integer stepid: step ID - :param list state_strings: updated state of the step + :param unicode state_string updated state of the step :returns: Deferred - Update the state strings for the given step. + Update the state string for the given step. .. py:method:: finishStep(stepid, results) From 595a02d295173dc0346c7de25bce0493319104bd Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Sat, 18 Oct 2014 19:03:09 -0400 Subject: [PATCH 3/8] change step state_strings to state_string: data API + consumers --- master/buildbot/data/steps.py | 12 +++--- master/buildbot/process/buildstep.py | 4 +- master/buildbot/status/words.py | 2 +- master/buildbot/test/fake/fakedata.py | 10 ++--- master/buildbot/test/unit/test_data_steps.py | 42 +++++++++---------- .../test/unit/test_process_buildstep.py | 14 +++---- master/buildbot/test/util/steps.py | 14 +++---- master/docs/developer/rtype-step.rst | 8 ++-- www/base/src/app/builders/step/step.tpl.jade | 2 +- .../buildsummary/buildsummary.tpl.jade | 2 +- .../buildbot/buildbot.service.spec.coffee | 2 +- 11 files changed, 55 insertions(+), 57 deletions(-) diff --git a/master/buildbot/data/steps.py b/master/buildbot/data/steps.py index 09487d076e6..0ed3ef6c4cc 100644 --- a/master/buildbot/data/steps.py +++ b/master/buildbot/data/steps.py @@ -29,7 +29,7 @@ def db2data(self, dbdict): 'started_at': dbdict['started_at'], 'complete': dbdict['complete_at'] is not None, 'complete_at': dbdict['complete_at'], - 'state_strings': dbdict['state_strings'], + 'state_string': dbdict['state_string'], 'results': dbdict['results'], 'urls': dbdict['urls'], } @@ -117,7 +117,7 @@ class EntityType(types.Entity): complete = types.Boolean() complete_at = types.NoneOk(types.DateTime()) results = types.NoneOk(types.Integer()) - state_strings = types.List(of=types.String()) + state_string = types.String() urls = types.List( of=types.Dict( name=types.String(), @@ -134,7 +134,7 @@ def generateEvent(self, stepid, event): @defer.inlineCallbacks def newStep(self, buildid, name): stepid, num, name = yield self.master.db.steps.addStep( - buildid=buildid, name=name, state_strings=[u'pending']) + buildid=buildid, name=name, state_string=u'pending') yield self.generateEvent(stepid, 'new') defer.returnValue((stepid, num, name)) @@ -146,9 +146,9 @@ def startStep(self, stepid): @base.updateMethod @defer.inlineCallbacks - def setStepStateStrings(self, stepid, state_strings): - yield self.master.db.steps.setStepStateStrings( - stepid=stepid, state_strings=state_strings) + def setStepStateString(self, stepid, state_string): + yield self.master.db.steps.setStepStateString( + stepid=stepid, state_string=state_string) yield self.generateEvent(stepid, 'updated') @base.updateMethod diff --git a/master/buildbot/process/buildstep.py b/master/buildbot/process/buildstep.py index 65ebe5d6ce5..04710911dc2 100644 --- a/master/buildbot/process/buildstep.py +++ b/master/buildbot/process/buildstep.py @@ -390,8 +390,8 @@ def methodInfo(m): if not isinstance(stepResult, unicode): raise TypeError("step result string must be unicode (got %r)" % (stepResult,)) - yield self.master.data.updates.setStepStateStrings(self.stepid, - [stepResult]) + yield self.master.data.updates.setStepStateString(self.stepid, + stepResult) if not self._running: buildResult = summary.get('build', None) diff --git a/master/buildbot/status/words.py b/master/buildbot/status/words.py index 47e920e3fcf..80439aec5d4 100644 --- a/master/buildbot/status/words.py +++ b/master/buildbot/status/words.py @@ -779,7 +779,7 @@ def emit_status(self, which): for build in runningBuilds: step = yield self.getCurrentBuildstep(build) if step: - s = "(%s)" % " ".join(step['state_strings']) + s = "(%s)" % step['state_string'] else: s = "(no current step)" buildInfo.append("%d %s" % (build['number'], s)) diff --git a/master/buildbot/test/fake/fakedata.py b/master/buildbot/test/fake/fakedata.py index c57dcad9e3b..b24fd91235e 100644 --- a/master/buildbot/test/fake/fakedata.py +++ b/master/buildbot/test/fake/fakedata.py @@ -48,7 +48,7 @@ def __init__(self, master, testcase): # { logid : {'finished': .., 'name': .., 'type': .., 'content': [ .. ]} } self.logs = {} self.claimedBuildRequests = set([]) - self.stepStateStrings = {} # { stepid : [strings] } + self.stepStateString = {} # { stepid : string } self.stepUrls = {} # { stepid : [(name,url)] } # extra assertions @@ -297,12 +297,12 @@ def startStep(self, stepid): validation.IntValidator()) return defer.succeed(None) - def setStepStateStrings(self, stepid, state_strings): + def setStepStateString(self, stepid, state_string): validation.verifyType(self.testcase, 'stepid', stepid, validation.IntValidator()) - validation.verifyType(self.testcase, 'state_strings', state_strings, - validation.ListValidator(validation.StringValidator())) - self.stepStateStrings[stepid] = state_strings + validation.verifyType(self.testcase, 'state_string', state_string, + validation.StringValidator()) + self.stepStateString[stepid] = state_string return defer.succeed(None) def finishStep(self, stepid, results): diff --git a/master/buildbot/test/unit/test_data_steps.py b/master/buildbot/test/unit/test_data_steps.py index 5d2f8722e60..e5c7f24e453 100644 --- a/master/buildbot/test/unit/test_data_steps.py +++ b/master/buildbot/test/unit/test_data_steps.py @@ -69,7 +69,7 @@ def test_get_existing(self): 'number': 2, 'results': None, 'started_at': epoch2datetime(TIME3), - 'state_strings': [], + 'state_string': u'', 'stepid': 72, 'urls': []}) @@ -181,7 +181,7 @@ def test_newStep(self): 'number': number, 'results': None, 'started_at': None, - 'state_strings': [u'pending'], + 'state_string': u'pending', 'stepid': stepid, 'urls': [], } @@ -198,7 +198,7 @@ def test_newStep(self): 'number': number, 'results': None, 'started_at': None, - 'state_strings': [u'pending'], + 'state_string': u'pending', 'urls': [], }) @@ -220,7 +220,7 @@ def newStep(self, stepid): def test_startStep(self): self.patch(reactor, 'seconds', lambda: TIME1) yield self.master.db.steps.addStep(buildid=10, name=u'ten', - state_strings=[u'pending']) + state_string=u'pending') yield self.rtype.startStep(stepid=100) msgBody = { @@ -231,7 +231,7 @@ def test_startStep(self): 'number': 0, 'results': None, 'started_at': epoch2datetime(TIME1), - 'state_strings': [u'pending'], + 'state_string': u'pending', 'stepid': 100, 'urls': [], } @@ -248,22 +248,22 @@ def test_startStep(self): 'number': 0, 'results': None, 'started_at': epoch2datetime(TIME1), - 'state_strings': [u'pending'], + 'state_string': u'pending', 'urls': [], }) - def test_signature_setStepStateStrings(self): + def test_signature_setStepStateString(self): @self.assertArgSpecMatches( - self.master.data.updates.setStepStateStrings, # fake - self.rtype.setStepStateStrings) # real - def setStepStateStrings(self, stepid, state_strings): + self.master.data.updates.setStepStateString, # fake + self.rtype.setStepStateString) # real + def setStepStateString(self, stepid, state_string): pass @defer.inlineCallbacks - def test_setStepStateStrings(self): + def test_setStepStateString(self): yield self.master.db.steps.addStep(buildid=10, name=u'ten', - state_strings=[u'pending']) - yield self.rtype.setStepStateStrings(stepid=100, state_strings=[u'hi']) + state_string=u'pending') + yield self.rtype.setStepStateString(stepid=100, state_string=u'hi') msgBody = { 'buildid': 10, @@ -273,7 +273,7 @@ def test_setStepStateStrings(self): 'number': 0, 'results': None, 'started_at': None, - 'state_strings': [u'hi'], + 'state_string': u'hi', 'stepid': 100, 'urls': [], } @@ -290,7 +290,7 @@ def test_setStepStateStrings(self): 'number': 0, 'results': None, 'started_at': None, - 'state_strings': [u'hi'], + 'state_string': u'hi', 'urls': [], }) @@ -304,7 +304,7 @@ def finishStep(self, stepid, results): @defer.inlineCallbacks def test_finishStep(self): yield self.master.db.steps.addStep(buildid=10, name=u'ten', - state_strings=[u'pending']) + state_string=u'pending') self.patch(reactor, 'seconds', lambda: TIME1) yield self.rtype.startStep(stepid=100) self.patch(reactor, 'seconds', lambda: TIME2) @@ -319,7 +319,7 @@ def test_finishStep(self): 'number': 0, 'results': 9, 'started_at': epoch2datetime(TIME1), - 'state_strings': [u'pending'], + 'state_string': u'pending', 'stepid': 100, 'urls': [], } @@ -336,7 +336,7 @@ def test_finishStep(self): 'number': 0, 'results': 9, 'started_at': epoch2datetime(TIME1), - 'state_strings': [u'pending'], + 'state_string': u'pending', 'urls': [], }) @@ -350,7 +350,7 @@ def addStepURL(self, stepid, name, url): @defer.inlineCallbacks def test_addStepURL(self): yield self.master.db.steps.addStep(buildid=10, name=u'ten', - state_strings=[u'pending']) + state_string=u'pending') yield self.rtype.addStepURL(stepid=100, name=u"foo", url=u"bar") msgBody = { @@ -361,7 +361,7 @@ def test_addStepURL(self): 'number': 0, 'results': None, 'started_at': None, - 'state_strings': [u'pending'], + 'state_string': u'pending', 'stepid': 100, 'urls': [{u'name': u'foo', u'url': u'bar'}], } @@ -378,6 +378,6 @@ def test_addStepURL(self): 'number': 0, 'results': None, 'started_at': None, - 'state_strings': [u'pending'], + 'state_string': u'pending', 'urls': [{u'name': u'foo', u'url': u'bar'}], }) diff --git a/master/buildbot/test/unit/test_process_buildstep.py b/master/buildbot/test/unit/test_process_buildstep.py index cee7dd1a16b..a3bb508897a 100644 --- a/master/buildbot/test/unit/test_process_buildstep.py +++ b/master/buildbot/test/unit/test_process_buildstep.py @@ -316,8 +316,7 @@ def test_updateSummary_running(self): step._running = True step.updateSummary() self.clock.advance(1) - self.assertEqual(step.master.data.updates.stepStateStrings[13], - [u'C']) + self.assertEqual(step.master.data.updates.stepStateString[13], u'C') def test_updateSummary_running_empty_dict(self): step = self.setup_summary_test() @@ -325,8 +324,8 @@ def test_updateSummary_running_empty_dict(self): step._running = True step.updateSummary() self.clock.advance(1) - self.assertEqual(step.master.data.updates.stepStateStrings[13], - [u'finished']) + self.assertEqual(step.master.data.updates.stepStateString[13], + u'finished') def test_updateSummary_running_not_unicode(self): step = self.setup_summary_test() @@ -349,8 +348,7 @@ def test_updateSummary_finished(self): step._running = False step.updateSummary() self.clock.advance(1) - self.assertEqual(step.master.data.updates.stepStateStrings[13], - [u'CS']) + self.assertEqual(step.master.data.updates.stepStateString[13], u'CS') def test_updateSummary_finished_empty_dict(self): step = self.setup_summary_test() @@ -358,8 +356,8 @@ def test_updateSummary_finished_empty_dict(self): step._running = False step.updateSummary() self.clock.advance(1) - self.assertEqual(step.master.data.updates.stepStateStrings[13], - [u'finished']) + self.assertEqual(step.master.data.updates.stepStateString[13], + u'finished') def test_updateSummary_finished_not_dict(self): step = self.setup_summary_test() diff --git a/master/buildbot/test/util/steps.py b/master/buildbot/test/util/steps.py index 729924610d3..0e8d0da773c 100644 --- a/master/buildbot/test/util/steps.py +++ b/master/buildbot/test/util/steps.py @@ -155,7 +155,7 @@ def addLogObserver(logname, observer): # expectations self.exp_result = None - self.exp_state_strings = None + self.exp_state_string = None self.exp_properties = {} self.exp_missing_properties = [] self.exp_logfiles = {} @@ -184,7 +184,7 @@ def expectOutcome(self, result, state_string=None): """ self.exp_result = result if state_string: - self.exp_state_strings = [state_string] + self.exp_state_string = state_string def expectProperty(self, property, value, source=None): """ @@ -227,12 +227,12 @@ def check(result): self.assertEqual(self.expected_remote_commands, [], "assert all expected commands were run") self.assertEqual(result, self.exp_result, "expected result") - if self.exp_state_strings: - stepStateStrings = self.master.data.updates.stepStateStrings - stepids = stepStateStrings.keys() + if self.exp_state_string: + stepStateString = self.master.data.updates.stepStateString + stepids = stepStateString.keys() assert stepids, "no step state strings were set" - self.assertEqual(stepStateStrings[stepids[0]], - self.exp_state_strings, + self.assertEqual(stepStateString[stepids[0]], + self.exp_state_string, "expected step state strings") for pn, (pv, ps) in self.exp_properties.iteritems(): self.assertTrue(self.properties.hasProperty(pn), diff --git a/master/docs/developer/rtype-step.rst b/master/docs/developer/rtype-step.rst index 71aad594e84..5122ad7931c 100644 --- a/master/docs/developer/rtype-step.rst +++ b/master/docs/developer/rtype-step.rst @@ -12,7 +12,7 @@ Steps :attr boolean complete: true if this step is complete :attr timestamp complete_at: time at which this step was complete, or None if it's still running :attr integer results: the results of the step (see :ref:`Build-Result-Codes`), or None if not complete - :attr list state_strings: a list of strings giving progressively more detail on the state of the build. + :attr list state_string: a list of strings giving progressively more detail on the state of the build. The first is usually one word or phrase; the remainder are sized for one-line display. :attr urls: a list of URLs associated with this step. :type urls: list of dictionaries with keys `name` and `url` @@ -93,12 +93,12 @@ All update methods are available as attributes of ``master.data.updates``. Start the step. - .. py:method:: setStepStateStrings(stepid, state_strings) + .. py:method:: setStepStateString(stepid, state_string) :param integer stepid: the step to modify - :param list state_strings: new state strings for this step + :param unicode state_string: new state strings for this step - Replace the existing state strings for a step with a new list. + Replace the existing state string for a step with a new list. .. py:method:: addStepURL(stepid, name, url): diff --git a/www/base/src/app/builders/step/step.tpl.jade b/www/base/src/app/builders/step/step.tpl.jade index 110e5c4063f..bc334ac805a 100644 --- a/www/base/src/app/builders/step/step.tpl.jade +++ b/www/base/src/app/builders/step/step.tpl.jade @@ -1,7 +1,7 @@ .container .row .col-sm-6 - | status: {{step.state_strings.join(' ')}} + | status: {{step.state_string}} ul.unstyled li(ng-repeat="log in step.logs") a(href="#/builders/{{builder.builderid}}/build/{{build.number}}/step/{{step.number}}/log/{{log.slug}}") {{log.name}} diff --git a/www/base/src/app/common/directives/buildsummary/buildsummary.tpl.jade b/www/base/src/app/common/directives/buildsummary/buildsummary.tpl.jade index 9d5d4867d5f..462a4e8e1f9 100644 --- a/www/base/src/app/common/directives/buildsummary/buildsummary.tpl.jade +++ b/www/base/src/app/common/directives/buildsummary/buildsummary.tpl.jade @@ -16,7 +16,7 @@ ng-click="step.fulldisplay=!step.fulldisplay") span.pull-right(ng-if="step.started_at") | {{ step.duration| durationformat:'LLL' }} - |  {{step.state_strings.join(' ')}} + |  {{step.state_string}} span.badge-status(ng-class="results2class(step, 'pulse')") | {{step.number}} |   diff --git a/www/base/src/app/common/services/buildbot/buildbot.service.spec.coffee b/www/base/src/app/common/services/buildbot/buildbot.service.spec.coffee index f9799c86c69..d02281d2286 100644 --- a/www/base/src/app/common/services/buildbot/buildbot.service.spec.coffee +++ b/www/base/src/app/common/services/buildbot/buildbot.service.spec.coffee @@ -41,7 +41,7 @@ describe 'buildbot service', -> r = buildbotService.one("builds", 1).one("steps", 2) r.bind($scope) $httpBackend.flush() - expect($scope.step.state_strings).toEqual(["mystate_strings"]) + expect($scope.step.state_string).toEqual("mystate_string") it 'should query default scope_key to route key', -> $httpBackend.expectGET('api/v2/builds/1/steps/2').respond({steps:[{res: "SUCCESS"}]}) From 40b885a461395d341a45361a113f6442ce63921c Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Sun, 19 Oct 2014 15:07:06 -0400 Subject: [PATCH 4/8] remove outdated TODO --- master/buildbot/process/buildstep.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/master/buildbot/process/buildstep.py b/master/buildbot/process/buildstep.py index 04710911dc2..afbdb3b14fb 100644 --- a/master/buildbot/process/buildstep.py +++ b/master/buildbot/process/buildstep.py @@ -368,8 +368,6 @@ def getResultSummary(self): return {u'step': stepsumm} - # TODO: test those^^ - @debounce.method(wait=1) @defer.inlineCallbacks def updateSummary(self): From 6d128ed50cccfbb8ccea7784cd2a1c35a9bda910 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Sun, 19 Oct 2014 15:12:30 -0400 Subject: [PATCH 5/8] change build state_strings to state_string: DB model --- .../versions/{038_step_string.py => 038_state_string.py} | 5 +++++ master/buildbot/db/model.py | 3 +-- ...tring.py => test_db_migrate_versions_038_state_string.py} | 5 ++++- 3 files changed, 10 insertions(+), 3 deletions(-) rename master/buildbot/db/migrate/versions/{038_step_string.py => 038_state_string.py} (84%) rename master/buildbot/test/unit/{test_db_migrate_versions_038_step_string.py => test_db_migrate_versions_038_state_string.py} (92%) diff --git a/master/buildbot/db/migrate/versions/038_step_string.py b/master/buildbot/db/migrate/versions/038_state_string.py similarity index 84% rename from master/buildbot/db/migrate/versions/038_step_string.py rename to master/buildbot/db/migrate/versions/038_state_string.py index c6b78ec68e4..32175061572 100644 --- a/master/buildbot/db/migrate/versions/038_step_string.py +++ b/master/buildbot/db/migrate/versions/038_state_string.py @@ -21,6 +21,7 @@ def upgrade(migrate_engine): metadata.bind = migrate_engine steps_table = sa.Table('steps', metadata, autoload=True) + builds_table = sa.Table('builds', metadata, autoload=True) # no attempt is made here to move data from one table to the other, since # there was no released version of Buildbot with a 'steps' table yet. @@ -28,3 +29,7 @@ def upgrade(migrate_engine): col = sa.Column('state_string', sa.Text, nullable=False, server_default='') col.create(steps_table) steps_table.c.state_strings_json.drop() + + col = sa.Column('state_string', sa.Text, nullable=False, server_default='') + col.create(builds_table) + builds_table.c.state_strings_json.drop() diff --git a/master/buildbot/db/model.py b/master/buildbot/db/model.py index 78e09952893..238df76545e 100644 --- a/master/buildbot/db/model.py +++ b/master/buildbot/db/model.py @@ -116,8 +116,7 @@ class Model(base.DBConnectorComponent): # start/complete times sa.Column('started_at', sa.Integer, nullable=False), sa.Column('complete_at', sa.Integer), - # a list of strings describing the build's state - sa.Column('state_strings_json', sa.Text, nullable=False), + sa.Column('state_string', sa.Text, nullable=False, server_default=''), sa.Column('results', sa.Integer), ) diff --git a/master/buildbot/test/unit/test_db_migrate_versions_038_step_string.py b/master/buildbot/test/unit/test_db_migrate_versions_038_state_string.py similarity index 92% rename from master/buildbot/test/unit/test_db_migrate_versions_038_step_string.py rename to master/buildbot/test/unit/test_db_migrate_versions_038_state_string.py index 9d43aadda59..e560f62079e 100644 --- a/master/buildbot/test/unit/test_db_migrate_versions_038_step_string.py +++ b/master/buildbot/test/unit/test_db_migrate_versions_038_state_string.py @@ -73,6 +73,9 @@ def verify_thd(conn): self.failUnless(hasattr(steps.c, 'state_string')) self.assertIsInstance(steps.c.state_string.type, sa.Text) - # TODO: also test that data is transferred? Or just hell with it + builds = sa.Table('builds', metadata, autoload=True) + self.failIf(hasattr(builds.c, 'state_strings_json')) + self.failUnless(hasattr(builds.c, 'state_string')) + self.assertIsInstance(builds.c.state_string.type, sa.Text) return self.do_test_migration(37, 38, setup_thd, verify_thd) From e55d775c81fb6f902babdd1c0708ada1f09fc89f Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Sun, 19 Oct 2014 15:20:19 -0400 Subject: [PATCH 6/8] change build state_strings to state_string: DB API --- master/buildbot/db/builds.py | 12 ++--- master/buildbot/test/fake/fakedb.py | 20 ++++---- master/buildbot/test/unit/test_db_builds.py | 51 ++++++++++----------- master/buildbot/test/util/validation.py | 4 +- master/docs/developer/database.rst | 12 ++--- 5 files changed, 48 insertions(+), 51 deletions(-) diff --git a/master/buildbot/db/builds.py b/master/buildbot/db/builds.py index b6619b7ebf8..6386a7279ae 100644 --- a/master/buildbot/db/builds.py +++ b/master/buildbot/db/builds.py @@ -18,7 +18,6 @@ from buildbot.db import NULL from buildbot.db import base from buildbot.util import epoch2datetime -from buildbot.util import json from twisted.internet import reactor @@ -59,9 +58,8 @@ def thd(conn): return self.db.pool.do(thd) def addBuild(self, builderid, buildrequestid, buildslaveid, masterid, - state_strings, _reactor=reactor, _race_hook=None): + state_string, _reactor=reactor, _race_hook=None): started_at = _reactor.seconds() - state_strings_json = json.dumps(state_strings) def thd(conn): tbl = self.db.model.builds @@ -82,19 +80,19 @@ def thd(conn): buildrequestid=buildrequestid, buildslaveid=buildslaveid, masterid=masterid, started_at=started_at, complete_at=None, - state_strings_json=state_strings_json)) + state_string=state_string)) except (sa.exc.IntegrityError, sa.exc.ProgrammingError): new_number += 1 continue return r.inserted_primary_key[0], new_number return self.db.pool.do(thd) - def setBuildStateStrings(self, buildid, state_strings): + def setBuildStateString(self, buildid, state_string): def thd(conn): tbl = self.db.model.builds q = tbl.update(whereclause=(tbl.c.id == buildid)) - conn.execute(q, state_strings_json=json.dumps(state_strings)) + conn.execute(q, state_string=state_string) return self.db.pool.do(thd) def finishBuild(self, buildid, results, _reactor=reactor): @@ -132,5 +130,5 @@ def mkdt(epoch): masterid=row.masterid, started_at=mkdt(row.started_at), complete_at=mkdt(row.complete_at), - state_strings=json.loads(row.state_strings_json), + state_string=row.state_string, results=row.results) diff --git a/master/buildbot/test/fake/fakedb.py b/master/buildbot/test/fake/fakedb.py index 7817d2b2b3a..e4db215c725 100644 --- a/master/buildbot/test/fake/fakedb.py +++ b/master/buildbot/test/fake/fakedb.py @@ -454,7 +454,7 @@ class Build(Row): masterid=None, started_at=1304262222, complete_at=None, - state_strings_json='["test"]', + state_string=u"test", results=None) id_column = 'id' @@ -1662,7 +1662,7 @@ def _row2dict(self, row): buildslaveid=row['buildslaveid'], started_at=_mkdt(row['started_at']), complete_at=_mkdt(row['complete_at']), - state_strings=json.loads(row['state_strings_json']), + state_string=row['state_string'], results=row['results']) def getBuild(self, buildid): @@ -1690,26 +1690,26 @@ def getBuilds(self, builderid=None, buildrequestid=None): return defer.succeed(ret) def addBuild(self, builderid, buildrequestid, buildslaveid, masterid, - state_strings, _reactor=reactor): - validation.verifyType(self.t, 'state_strings', state_strings, - validation.ListValidator(validation.StringValidator())) + state_string, _reactor=reactor): + validation.verifyType(self.t, 'state_string', state_string, + validation.StringValidator()) id = self._newId() number = max([0] + [r['number'] for r in self.builds.itervalues() if r['builderid'] == builderid]) + 1 self.builds[id] = dict(id=id, number=number, buildrequestid=buildrequestid, builderid=builderid, buildslaveid=buildslaveid, masterid=masterid, - state_strings_json=unicode(json.dumps(state_strings)), + state_string=state_string, started_at=_reactor.seconds(), complete_at=None, results=None) return defer.succeed((id, number)) - def setBuildStateStrings(self, buildid, state_strings): - validation.verifyType(self.t, 'state_strings', state_strings, - validation.ListValidator(validation.StringValidator())) + def setBuildStateString(self, buildid, state_string): + validation.verifyType(self.t, 'state_string', state_string, + validation.StringValidator()) b = self.builds.get(buildid) if b: - b['state_strings_json'] = unicode(json.dumps(state_strings)) + b['state_string'] = state_string return defer.succeed(None) def finishBuild(self, buildid, results, _reactor=reactor): diff --git a/master/buildbot/test/unit/test_db_builds.py b/master/buildbot/test/unit/test_db_builds.py index c16e9dbadc6..0a556991104 100644 --- a/master/buildbot/test/unit/test_db_builds.py +++ b/master/buildbot/test/unit/test_db_builds.py @@ -47,13 +47,13 @@ class Tests(interfaces.InterfaceTests): ] threeBuilds = [ fakedb.Build(id=50, buildrequestid=42, number=5, masterid=88, - builderid=77, buildslaveid=13, state_strings_json='["test"]', + builderid=77, buildslaveid=13, state_string="test", started_at=TIME1), fakedb.Build(id=51, buildrequestid=41, number=6, masterid=88, - builderid=88, buildslaveid=13, state_strings_json='["test"]', + builderid=88, buildslaveid=13, state_string="test", started_at=TIME2), fakedb.Build(id=52, buildrequestid=42, number=7, masterid=88, - builderid=77, buildslaveid=13, state_strings_json='["test"]', + builderid=77, buildslaveid=13, state_string="test", started_at=TIME3, complete_at=TIME4, results=5), ] @@ -61,18 +61,18 @@ class Tests(interfaces.InterfaceTests): 50: {'id': 50, 'buildrequestid': 42, 'builderid': 77, 'masterid': 88, 'number': 5, 'buildslaveid': 13, 'started_at': epoch2datetime(TIME1), - 'complete_at': None, 'state_strings': [u'test'], + 'complete_at': None, 'state_string': 'test', 'results': None}, 51: {'id': 51, 'buildrequestid': 41, 'builderid': 88, 'masterid': 88, 'number': 6, 'buildslaveid': 13, 'started_at': epoch2datetime(TIME2), - 'complete_at': None, 'state_strings': [u'test'], + 'complete_at': None, 'state_string': 'test', 'results': None}, 52: {'id': 52, 'buildrequestid': 42, 'builderid': 77, 'masterid': 88, 'number': 7, 'buildslaveid': 13, 'started_at': epoch2datetime(TIME3), 'complete_at': epoch2datetime(TIME4), - 'state_strings': [u'test'], + 'state_string': 'test', 'results': 5}, } @@ -96,12 +96,12 @@ def getBuilds(self, builderid=None, buildrequestid=None): def test_signature_addBuild(self): @self.assertArgSpecMatches(self.db.builds.addBuild) def addBuild(self, builderid, buildrequestid, buildslaveid, masterid, - state_strings): + state_string): pass - def test_signature_setBuildStateStrings(self): - @self.assertArgSpecMatches(self.db.builds.setBuildStateStrings) - def setBuildStateStrings(self, buildid, state_strings): + def test_signature_setBuildStateString(self): + @self.assertArgSpecMatches(self.db.builds.setBuildStateString) + def setBuildStateString(self, buildid, state_string): pass def test_signature_finishBuild(self): @@ -124,7 +124,7 @@ def test_getBuild(self): self.assertEqual(bdict, dict(id=50, number=5, buildrequestid=42, masterid=88, builderid=77, buildslaveid=13, started_at=epoch2datetime(TIME1), complete_at=None, - state_strings=[u'test'], results=None)) + state_string=u'test', results=None)) @defer.inlineCallbacks def test_getBuild_missing(self): @@ -173,13 +173,13 @@ def test_addBuild_first(self): yield self.insertTestData(self.backgroundData) id, number = yield self.db.builds.addBuild(builderid=77, buildrequestid=41, buildslaveid=13, masterid=88, - state_strings=[u'test', u'test2'], _reactor=clock) + state_string=u'test test2', _reactor=clock) bdict = yield self.db.builds.getBuild(id) validation.verifyDbDict(self, 'builddict', bdict) self.assertEqual(bdict, {'buildrequestid': 41, 'builderid': 77, 'id': id, 'masterid': 88, 'number': number, 'buildslaveid': 13, 'started_at': epoch2datetime(TIME1), - 'complete_at': None, 'state_strings': [u'test', u'test2'], + 'complete_at': None, 'state_string': u'test test2', 'results': None}) @defer.inlineCallbacks @@ -192,28 +192,27 @@ def test_addBuild_existing(self): ]) id, number = yield self.db.builds.addBuild(builderid=77, buildrequestid=41, buildslaveid=13, masterid=88, - state_strings=[u'test', u'test2'], _reactor=clock) + state_string=u'test test2', _reactor=clock) bdict = yield self.db.builds.getBuild(id) validation.verifyDbDict(self, 'builddict', bdict) self.assertEqual(number, 11) self.assertEqual(bdict, {'buildrequestid': 41, 'builderid': 77, 'id': id, 'masterid': 88, 'number': number, 'buildslaveid': 13, 'started_at': epoch2datetime(TIME1), - 'complete_at': None, 'state_strings': [u'test', u'test2'], + 'complete_at': None, 'state_string': u'test test2', 'results': None}) @defer.inlineCallbacks - def test_setBuildStateStrings(self): + def test_setBuildStateString(self): yield self.insertTestData(self.backgroundData + [self.threeBuilds[0]]) - yield self.db.builds.setBuildStateStrings(buildid=50, - state_strings=[u'test', u'test2', u'test3']) + yield self.db.builds.setBuildStateString(buildid=50, + state_string=u'test test2') bdict = yield self.db.builds.getBuild(50) validation.verifyDbDict(self, 'builddict', bdict) self.assertEqual(bdict, dict(id=50, number=5, buildrequestid=42, masterid=88, builderid=77, buildslaveid=13, started_at=epoch2datetime(TIME1), complete_at=None, - state_strings=[u'test', u'test2', u'test3'], - results=None)) + state_string=u'test test2', results=None)) @defer.inlineCallbacks def test_finishBuild(self): @@ -227,7 +226,7 @@ def test_finishBuild(self): masterid=88, builderid=77, buildslaveid=13, started_at=epoch2datetime(TIME1), complete_at=epoch2datetime(TIME4), - state_strings=[u'test'], + state_string=u'test', results=7)) @defer.inlineCallbacks @@ -237,7 +236,7 @@ def test_finishBuildsFromMaster(self): yield self.insertTestData(self.backgroundData + self.threeBuilds + [ fakedb.Build( id=54, buildrequestid=40, number=50, masterid=89, - builderid=77, buildslaveid=13, state_strings_json='["test"]', + builderid=77, buildslaveid=13, state_string="test", started_at=TIME1) ]) yield self.db.builds.finishBuildsFromMaster(masterid=88, results=7, _reactor=clock) @@ -247,7 +246,7 @@ def test_finishBuildsFromMaster(self): masterid=88, builderid=77, buildslaveid=13, started_at=epoch2datetime(TIME1), complete_at=epoch2datetime(TIME4), - state_strings=[u'test'], + state_string=u'test', results=7)) for _id, results in [(50, 7), (51, 7), (52, 5), (54, None)]: bdict = yield self.db.builds.getBuild(_id) @@ -272,11 +271,11 @@ def raceHook(conn): conn.execute(self.db.model.builds.insert(), {'number': numbers.pop(0), 'buildrequestid': 41, 'masterid': 88, 'buildslaveid': 13, 'builderid': 77, - 'started_at': TIME1, 'state_strings_json': '["hi"]'}) + 'started_at': TIME1, 'state_string': "hi"}) id, number = yield self.db.builds.addBuild(builderid=77, buildrequestid=41, buildslaveid=13, masterid=88, - state_strings=[u'test', u'test2'], _reactor=clock, + state_string=u'test test2', _reactor=clock, _race_hook=raceHook) bdict = yield self.db.builds.getBuild(id) validation.verifyDbDict(self, 'builddict', bdict) @@ -284,7 +283,7 @@ def raceHook(conn): self.assertEqual(bdict, {'buildrequestid': 41, 'builderid': 77, 'id': id, 'masterid': 88, 'number': number, 'buildslaveid': 13, 'started_at': epoch2datetime(TIME1), - 'complete_at': None, 'state_strings': [u'test', u'test2'], + 'complete_at': None, 'state_string': u'test test2', 'results': None}) diff --git a/master/buildbot/test/util/validation.py b/master/buildbot/test/util/validation.py index 2039919998f..09d7c9331d7 100644 --- a/master/buildbot/test/util/validation.py +++ b/master/buildbot/test/util/validation.py @@ -533,7 +533,7 @@ def validate(self, name, arg_object): started_at=IntValidator(), complete=BooleanValidator(), complete_at=NoneOk(IntValidator()), - state_strings=ListValidator(StringValidator()), + state_string=StringValidator(), results=NoneOk(IntValidator()), ) _buildEvents = ['new', 'complete'] @@ -555,7 +555,7 @@ def validate(self, name, arg_object): masterid=IntValidator(), started_at=DateTimeValidator(), complete_at=NoneOk(DateTimeValidator()), - state_strings=ListValidator(StringValidator()), + state_string=StringValidator(), results=NoneOk(IntValidator()), ) diff --git a/master/docs/developer/database.rst b/master/docs/developer/database.rst index 54935e3e0d5..55fc3d82269 100644 --- a/master/docs/developer/database.rst +++ b/master/docs/developer/database.rst @@ -247,7 +247,7 @@ builds * ``masterid`` (the ID of the master on which this build was performed) * ``started_at`` (datetime at which this build began) * ``complete_at`` (datetime at which this build finished, or None if it is ongoing) - * ``state_strings`` (list of short strings describing the build's state) + * ``state_string`` (short string describing the build's state) * ``results`` (results of this build; see :ref:`Build-Result-Codes`) .. py:method:: getBuild(buildid) @@ -276,22 +276,22 @@ builds Get a list of builds, in the format described above. Each of the parameters limit the resulting set of builds. - .. py:method:: addBuild(builderid, buildrequestid, buildslaveid, masterid, state_strings) + .. py:method:: addBuild(builderid, buildrequestid, buildslaveid, masterid, state_string) :param integer builderid: builder to get builds for :param integer buildrequestid: build request id :param integer slaveid: slave performing the build :param integer masterid: master performing the build - :param list state_strings: initial state of the build + :param unicode state_string: initial state of the build :returns: tuple of build ID and build number, via Deferred Add a new build to the db, recorded as having started at the current time. This will invent a new number for the build, unique within the context of the builder. - .. py:method:: setBuildStateStrings(buildid, state_strings): + .. py:method:: setBuildStateString(buildid, state_string): :param integer buildid: build id - :param list state_strings: updated state of the build + :param list state_string: updated state of the build :returns: Deferred Update the state strings for the given build. @@ -383,7 +383,7 @@ steps .. py:method:: setStepStateString(stepid, state_string): :param integer stepid: step ID - :param unicode state_string updated state of the step + :param unicode state_string: updated state of the step :returns: Deferred Update the state string for the given step. From 532497befacdffe16c69bf51f8fcaff734cf01bd Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Sun, 19 Oct 2014 15:28:08 -0400 Subject: [PATCH 7/8] change build state_strings to state_string: Data API + consumers --- master/buildbot/data/builds.py | 12 +++++------ master/buildbot/process/build.py | 8 ++++---- master/buildbot/schedulers/trysched.py | 2 +- master/buildbot/status/words.py | 6 +++--- master/buildbot/test/fake/fakedata.py | 6 +++--- master/buildbot/test/unit/test_data_builds.py | 20 +++++++++---------- .../buildbot/test/unit/test_status_words.py | 2 +- master/docs/developer/rtype-build.rst | 9 ++++----- .../src/app/builders/builder/builder.tpl.jade | 4 ++-- .../buildsummary/buildsummary.tpl.jade | 4 ++-- 10 files changed, 36 insertions(+), 37 deletions(-) diff --git a/master/buildbot/data/builds.py b/master/buildbot/data/builds.py index 3a393fb9226..920fddb6685 100644 --- a/master/buildbot/data/builds.py +++ b/master/buildbot/data/builds.py @@ -31,7 +31,7 @@ def db2data(self, dbdict): 'started_at': dbdict['started_at'], 'complete_at': dbdict['complete_at'], 'complete': dbdict['complete_at'] is not None, - 'state_strings': dbdict['state_strings'], + 'state_string': dbdict['state_string'], 'results': dbdict['results'], } return defer.succeed(data) @@ -124,7 +124,7 @@ class EntityType(types.Entity): complete = types.Boolean() complete_at = types.NoneOk(types.DateTime()) results = types.NoneOk(types.Integer()) - state_strings = types.List(of=types.String()) + state_string = types.String() entityType = EntityType(name) @defer.inlineCallbacks @@ -141,7 +141,7 @@ def newBuild(self, builderid, buildrequestid, buildslaveid): buildrequestid=buildrequestid, buildslaveid=buildslaveid, masterid=self.master.masterid, - state_strings=[u'created']) + state_string=u'created') if res is not None: _id, number = res yield self.generateEvent(_id, "new") @@ -149,9 +149,9 @@ def newBuild(self, builderid, buildrequestid, buildslaveid): @base.updateMethod @defer.inlineCallbacks - def setBuildStateStrings(self, buildid, state_strings): - res = yield self.master.db.builds.setBuildStateStrings( - buildid=buildid, state_strings=state_strings) + def setBuildStateString(self, buildid, state_string): + res = yield self.master.db.builds.setBuildStateString( + buildid=buildid, state_string=state_string) yield self.generateEvent(buildid, "update") defer.returnValue(res) diff --git a/master/buildbot/process/build.py b/master/buildbot/process/build.py index b33e1733078..e5e67f97972 100644 --- a/master/buildbot/process/build.py +++ b/master/buildbot/process/build.py @@ -278,8 +278,8 @@ def startBuild(self, build_status, expectations, slavebuilder): self.deferred = None return - yield self.master.data.updates.setBuildStateStrings(self.buildid, - [u'starting']) + yield self.master.data.updates.setBuildStateString(self.buildid, + u'starting') self.build_status.buildStarted(self) yield self.acquireLocks() @@ -290,8 +290,8 @@ def startBuild(self, build_status, expectations, slavebuilder): finally: metrics.MetricCountEvent.log('active_builds', -1) - yield self.master.data.updates.setBuildStateStrings(self.buildid, - [u'finished']) + yield self.master.data.updates.setBuildStateString(self.buildid, + u'finished') yield self.master.data.updates.finishBuild(self.buildid, self.results) # mark the build as finished diff --git a/master/buildbot/schedulers/trysched.py b/master/buildbot/schedulers/trysched.py index 910415b9c33..dfbe19fb7bb 100644 --- a/master/buildbot/schedulers/trysched.py +++ b/master/buildbot/schedulers/trysched.py @@ -357,7 +357,7 @@ def remote_getResults(self): def remote_getText(self): buildid = self.builddict['buildid'] builddict = yield self.master.data.get(('builds', buildid)) - defer.returnValue(builddict['state_strings']) + defer.returnValue([builddict['state_string']]) class Try_Userpass_Perspective(pbutil.NewCredPerspective): diff --git a/master/buildbot/status/words.py b/master/buildbot/status/words.py index 80439aec5d4..bbe33f0ca57 100644 --- a/master/buildbot/status/words.py +++ b/master/buildbot/status/words.py @@ -622,7 +622,7 @@ def watchedBuildFinished(self, build): r = "Hey! build %s #%d is complete: %s" % \ (builder_name, buildnum, results[0]) - r += ' [%s]' % maybeColorize(" ".join(build['state_strings']), results[1], self.useColors) + r += ' [%s]' % maybeColorize(build['state_string'], results[1], self.useColors) self.send(r) # FIXME: where do we get the base_url? Then do we use the build Link to make the URL? @@ -769,7 +769,7 @@ def emit_status(self, which): ago = self.convertTime(int(util.now() - complete_at)) else: ago = "??" - status = " ".join(lastBuild['state_strings']) + status = lastBuild['state_string'] response += ' last build %s ago: %s' % (ago, status) else: response += "offline" @@ -814,7 +814,7 @@ def command_LAST(self, args, who): ago = self.convertTime(int(util.now() - complete_at)) else: ago = "??" - status = " ".join(lastBuild['state_strings']) + status = lastBuild['state_string'] status = 'last build %s ago: %s' % (ago, status) self.send("last build [%s]: %s" % (builder['name'], status)) diff --git a/master/buildbot/test/fake/fakedata.py b/master/buildbot/test/fake/fakedata.py index b24fd91235e..999ce92336b 100644 --- a/master/buildbot/test/fake/fakedata.py +++ b/master/buildbot/test/fake/fakedata.py @@ -261,11 +261,11 @@ def newBuild(self, builderid, buildrequestid, buildslaveid): validation.IntValidator()) return defer.succeed((10, 1)) - def setBuildStateStrings(self, buildid, state_strings): + def setBuildStateString(self, buildid, state_string): validation.verifyType(self.testcase, 'buildid', buildid, validation.IntValidator()) - validation.verifyType(self.testcase, 'state_strings', state_strings, - validation.ListValidator(validation.StringValidator())) + validation.verifyType(self.testcase, 'state_string', state_string, + validation.StringValidator()) return defer.succeed(None) def finishBuild(self, buildid, results): diff --git a/master/buildbot/test/unit/test_data_builds.py b/master/buildbot/test/unit/test_data_builds.py index e2882ae9ec8..feb4371a9d7 100644 --- a/master/buildbot/test/unit/test_data_builds.py +++ b/master/buildbot/test/unit/test_data_builds.py @@ -133,7 +133,7 @@ class Build(interfaces.InterfaceTests, unittest.TestCase): 'number': 1, 'results': None, 'started_at': epoch2datetime(1), - 'state_strings': [u'created']} + 'state_string': u'created'} def setUp(self): self.master = fakemaster.make_master(testcase=self, @@ -169,7 +169,7 @@ def test_newBuild(self): builderid=10, buildrequestid=13, buildslaveid=20, exp_kwargs=dict(builderid=10, buildrequestid=13, buildslaveid=20, masterid=self.master.masterid, - state_strings=['created'])) + state_string=u'created')) def test_newBuildEvent(self): return self.do_test_event(self.rtype.newBuild, @@ -177,17 +177,17 @@ def test_newBuildEvent(self): exp_events=[(('builders', '10', 'builds', '1', 'new'), self.new_build_event), (('builds', '100', 'new'), self.new_build_event)]) - def test_signature_setBuildStateStrings(self): + def test_signature_setBuildStateString(self): @self.assertArgSpecMatches( - self.master.data.updates.setBuildStateStrings, # fake - self.rtype.setBuildStateStrings) # real - def setBuildStateStrings(self, buildid, state_strings): + self.master.data.updates.setBuildStateString, # fake + self.rtype.setBuildStateString) # real + def setBuildStateString(self, buildid, state_string): pass - def test_setBuildStateStrings(self): - return self.do_test_callthrough('setBuildStateStrings', - self.rtype.setBuildStateStrings, - buildid=10, state_strings=['a', 'b']) + def test_setBuildStateString(self): + return self.do_test_callthrough('setBuildStateString', + self.rtype.setBuildStateString, + buildid=10, state_string=u'a b') def test_signature_finishBuild(self): @self.assertArgSpecMatches( diff --git a/master/buildbot/test/unit/test_status_words.py b/master/buildbot/test/unit/test_status_words.py index ab23e365368..f5f518b8e50 100644 --- a/master/buildbot/test/unit/test_status_words.py +++ b/master/buildbot/test/unit/test_status_words.py @@ -464,7 +464,7 @@ def sendBuildFinishedMessage(self, buildid, results=0): started_at=datetime2epoch(build['started_at']), complete=True, complete_at=datetime2epoch(build['complete_at']), - state_strings=[], + state_string=u'', results=results, )) diff --git a/master/docs/developer/rtype-build.rst b/master/docs/developer/rtype-build.rst index 26dadc2df4f..36ef23450db 100644 --- a/master/docs/developer/rtype-build.rst +++ b/master/docs/developer/rtype-build.rst @@ -13,8 +13,7 @@ Builds :attr boolean complete: true if this build is complete :attr timestamp complete_at: time at which this build was complete, or None if it's still running :attr integer results: the results of the build (see :ref:`Build-Result-Codes`), or None if not complete - :attr list state_strings: a list of strings giving progressively more detail on the state of the build. - The first is usually one word or phrase; the remainder are sized for one-line display. + :attr unicode state_string: a string giving detail on the state of the build. .. todo:: @@ -39,7 +38,7 @@ Builds .. bb:event:: build.$builderid.$buildid.newstate - The build's state (``state_strings``) has changed. + The build's state (``state_string``) has changed. .. bb:rpath:: /build @@ -87,10 +86,10 @@ All update methods are available as attributes of ``master.data.updates``. Create a new build resource and return its ID. The state strings for the new build will be set to 'starting'. - .. py:method:: setBuildStateStrings(buildid, state_strings) + .. py:method:: setBuildStateString(buildid, state_string) :param integer buildid: the build to modify - :param list state_strings: new state strings for this build + :param list state_string: new state string for this build Replace the existing state strings for a build with a new list. diff --git a/www/base/src/app/builders/builder/builder.tpl.jade b/www/base/src/app/builders/builder/builder.tpl.jade index 1f09b29a31b..05c89d6fed7 100644 --- a/www/base/src/app/builders/builder/builder.tpl.jade +++ b/www/base/src/app/builders/builder/builder.tpl.jade @@ -45,5 +45,5 @@ td(width='200px') td ul.list-inline - li(ng-repeat='st in build.state_strings') - | {{st}} + li + | {{build.state_string}} diff --git a/www/base/src/app/common/directives/buildsummary/buildsummary.tpl.jade b/www/base/src/app/common/directives/buildsummary/buildsummary.tpl.jade index 462a4e8e1f9..c8490ff1d17 100644 --- a/www/base/src/app/common/directives/buildsummary/buildsummary.tpl.jade +++ b/www/base/src/app/common/directives/buildsummary/buildsummary.tpl.jade @@ -7,8 +7,8 @@ | {{(build.complete_at - build.started_at)| durationformat:'LLL' }} span(ng-show="!build.complete") | {{(now - build.started_at)| durationformat:'LLL' }} - span(ng-repeat="s in build.state_strings") - |  {{s}}  + span + |  {{build.state_string}}  .label(ng-class="results2class(build)") | {{results2text(build)}} ul.list-group.no-select From 69b22d2e45ca0caca7f2babb35f6023b7d598262 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Mon, 20 Oct 2014 19:35:19 -0400 Subject: [PATCH 8/8] update docs to specify right type for state_string --- master/docs/developer/database.rst | 4 ++-- master/docs/developer/rtype-build.rst | 2 +- master/docs/developer/rtype-step.rst | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/master/docs/developer/database.rst b/master/docs/developer/database.rst index 55fc3d82269..fa484537c69 100644 --- a/master/docs/developer/database.rst +++ b/master/docs/developer/database.rst @@ -291,7 +291,7 @@ builds .. py:method:: setBuildStateString(buildid, state_string): :param integer buildid: build id - :param list state_string: updated state of the build + :param unicode state_string: updated state of the build :returns: Deferred Update the state strings for the given build. @@ -374,7 +374,7 @@ steps :param integer buildid: the build to which to add the step :param name: the step name :type name: 50-character :ref:`identifier ` - :param list state_string: the initial state of the step + :param unicode state_string: the initial state of the step :returns: tuple of step ID, step number, and step name, via Deferred Add a new step to a build. diff --git a/master/docs/developer/rtype-build.rst b/master/docs/developer/rtype-build.rst index 36ef23450db..fde97a8f620 100644 --- a/master/docs/developer/rtype-build.rst +++ b/master/docs/developer/rtype-build.rst @@ -89,7 +89,7 @@ All update methods are available as attributes of ``master.data.updates``. .. py:method:: setBuildStateString(buildid, state_string) :param integer buildid: the build to modify - :param list state_string: new state string for this build + :param unicode state_string: new state string for this build Replace the existing state strings for a build with a new list. diff --git a/master/docs/developer/rtype-step.rst b/master/docs/developer/rtype-step.rst index 5122ad7931c..f88a2c0707b 100644 --- a/master/docs/developer/rtype-step.rst +++ b/master/docs/developer/rtype-step.rst @@ -12,7 +12,7 @@ Steps :attr boolean complete: true if this step is complete :attr timestamp complete_at: time at which this step was complete, or None if it's still running :attr integer results: the results of the step (see :ref:`Build-Result-Codes`), or None if not complete - :attr list state_string: a list of strings giving progressively more detail on the state of the build. + :attr unicode state_string: a string giving detail on the state of the build. The first is usually one word or phrase; the remainder are sized for one-line display. :attr urls: a list of URLs associated with this step. :type urls: list of dictionaries with keys `name` and `url`