remove properties from build_status

put it directly in build one step forward to removing the status Signed-off-by: Pierre Tardy <tardyp@gmail.com>
buildbot · Oct 28, 2015 · a010119 · a010119
1 parent 422012f
commit a010119
Show file tree

Hide file tree

Showing 8 changed files with 39 additions and 145 deletions.
diff --git a/master/buildbot/process/build.py b/master/buildbot/process/build.py
@@ -25,11 +25,11 @@
 from buildbot import interfaces
 from buildbot.process import metrics
 from buildbot.process import properties
-from buildbot.process.results import Results
 from buildbot.process.results import CANCELLED
 from buildbot.process.results import EXCEPTION
 from buildbot.process.results import FAILURE
 from buildbot.process.results import RETRY
+from buildbot.process.results import Results
 from buildbot.process.results import SUCCESS
 from buildbot.process.results import WARNINGS
 from buildbot.process.results import computeResultAndTermination
@@ -90,6 +90,7 @@ def __init__(self, requests):
 
         self._acquiringLock = None
         self.results = SUCCESS   # overall results, may downgrade after each step
+        self.properties = properties.Properties()
 
     def setBuilder(self, builder):
         """
@@ -178,8 +179,7 @@ def setupProperties(self):
         props.build = self
 
         # start with global properties from the configuration
-        master = self.builder.master
-        props.updateFromProperties(master.config.properties)
+        props.updateFromProperties(self.master.config.properties)
 
         # from the SourceStamps, which have properties via Change
         for change in self.allChanges():
@@ -586,5 +586,5 @@ def getStatus(self):
     # stopBuild is defined earlier
 
 components.registerAdapter(
-    lambda build: interfaces.IProperties(build.build_status),
+    lambda build: interfaces.IProperties(build.properties),
     Build, interfaces.IProperties)
diff --git a/master/buildbot/status/build.py b/master/buildbot/status/build.py
@@ -18,18 +18,16 @@
 
 from buildbot import interfaces
 from buildbot import util
-from buildbot.process import properties
 from buildbot.util import pickle
 from twisted.internet import defer
 from twisted.internet import reactor
 from twisted.persisted import styles
-from twisted.python import components
 from twisted.python import log
 from twisted.python import runtime
 from zope.interface import implements
 
 
-class BuildStatus(styles.Versioned, properties.PropertiesMixin):
+class BuildStatus(styles.Versioned):
     implements(interfaces.IBuildStatus, interfaces.IStatusEvent)
 
     persistenceVersion = 4
@@ -47,8 +45,6 @@ class BuildStatus(styles.Versioned, properties.PropertiesMixin):
     results = None
     slavename = "???"
 
-    set_runtime_properties = True
-
     # these lists/dicts are defined here so that unserialized instances have
     # (empty) values. They are set in __init__ to new objects to make sure
     # each instance gets its own copy.
@@ -71,7 +67,6 @@ def __init__(self, parent, master, number):
         self.finishedWatchers = []
         self.steps = []
         self.testResults = {}
-        self.properties = properties.Properties()
 
     def __repr__(self):
         return "<%s #%s>" % (self.__class__.__name__, self.number)
@@ -92,14 +87,6 @@ def getPreviousBuild(self):
             return None
         return self.builder.getBuild(self.number - 1)
 
-    def getAllGotRevisions(self):
-        all_got_revisions = self.properties.getProperty('got_revision', {})
-        # For backwards compatibility all_got_revisions is a string if codebases
-        # are not used. Convert to the default internal type (dict)
-        if not isinstance(all_got_revisions, dict):
-            all_got_revisions = {'': all_got_revisions}
-        return all_got_revisions
-
     def getSourceStamps(self, absolute=False):
         return {}
 
@@ -121,10 +108,6 @@ def getRevisions(self):
     def getResponsibleUsers(self):
         return self.blamelist
 
-    def getInterestedUsers(self):
-        # TODO: the Builder should add others: sheriffs, domain-owners
-        return self.properties.getProperty('owners', [])
-
     def getSteps(self):
         """Return a list of IBuildStepStatus objects. For invariant builds
         (those which always use the same set of Steps), this should be the
@@ -365,14 +348,9 @@ def upgradeToVersion1(self):
         self.wasUpgraded = True
 
     def upgradeToVersion2(self):
-        self.properties = {}
         self.wasUpgraded = True
 
     def upgradeToVersion3(self):
-        # in version 3, self.properties became a Properties object
-        propdict = self.properties
-        self.properties = properties.Properties()
-        self.properties.update(propdict, "Upgrade from previous version")
         self.wasUpgraded = True
 
     def upgradeToVersion4(self):
@@ -420,7 +398,6 @@ def asDict(self):
         result['blame'] = self.getResponsibleUsers()
 
         # Transient
-        result['properties'] = self.getProperties().asList()
         result['times'] = self.getTimes()
         result['text'] = self.getText()
         result['results'] = self.getResults()
@@ -436,6 +413,3 @@ def asDict(self):
         else:
             result['currentStep'] = None
         return result
-
-components.registerAdapter(lambda build_status: build_status.properties,
-                           BuildStatus, interfaces.IProperties)
diff --git a/master/buildbot/steps/trigger.py b/master/buildbot/steps/trigger.py
@@ -147,13 +147,21 @@ def prepareSourcestampListForTrigger(self):
 
         # overrule revision in sourcestamps with got revision
         if self.updateSourceStamp:
-            got = self.build.build_status.getAllGotRevisions()
+            got = self.getAllGotRevisions()
             for codebase in ss_for_trigger:
                 if codebase in got:
                     ss_for_trigger[codebase]['revision'] = got[codebase]
 
         return list(itervalues(ss_for_trigger))
 
+    def getAllGotRevisions(self):
+        all_got_revisions = self.getProperty('got_revision', {})
+        # For backwards compatibility all_got_revisions is a string if codebases
+        # are not used. Convert to the default internal type (dict)
+        if not isinstance(all_got_revisions, dict):
+            all_got_revisions = {'': all_got_revisions}
+        return all_got_revisions
+
     @defer.inlineCallbacks
     def worstStatus(self, overall_results, rclist):
         for was_cb, results in rclist:

diff --git a/master/buildbot/test/unit/test_process_build.py b/master/buildbot/test/unit/test_process_build.py
@@ -634,17 +634,17 @@ def test_getSummaryStatistic(self):
     def testflushProperties(self):
         b = self.build
 
-        class FakeBuildStatus(Mock):
+        class FakeProperties(Mock):
             implements(interfaces.IProperties)
-        b.build_status = FakeBuildStatus()
+        b.properties = FakeProperties()
 
         class Properties(Mock):
 
             def asList(self):
                 return [(u'p', 5, u'fake'),
                         (u'p2', ['abc', 9], u'mock')]
         b.master.data.updates.setBuildProperty = Mock()
-        b.build_status.getProperties.return_value = Properties()
+        b.properties.getProperties.return_value = Properties()
         b.buildid = 42
         result = 'SUCCESS'
         res = yield b._flushProperties(result)
@@ -863,7 +863,7 @@ def setUp(self):
         self.build.setBuilder(self.builder)
         self.build.build_status = FakeBuildStatus()
         # record properties that will be set
-        self.build.build_status.properties.setProperty = self.setProperty
+        self.build.properties.setProperty = self.setProperty
 
     def setProperty(self, n, v, s, runtime=False):
         if s not in self.props:
@@ -905,7 +905,7 @@ def setUp(self):
         self.build.setBuilder(self.builder)
         self.build.build_status = FakeBuildStatus()
         # record properties that will be set
-        self.build.build_status.properties.setProperty = self.setProperty
+        self.build.properties.setProperty = self.setProperty
 
     def setProperty(self, n, v, s, runtime=False):
         if s not in self.props:
@@ -949,8 +949,11 @@ class TestBuildProperties(unittest.TestCase):
     """
 
     def setUp(self):
-        class FakeBuildStatus(Mock):
+        class FakeProperties(Mock):
             implements(interfaces.IProperties)
+
+        class FakeBuildStatus(Mock):
+            pass
         r = FakeRequest()
         r.sources = [FakeSource()]
         r.sources[0].changes = [FakeChange()]
@@ -965,34 +968,35 @@ class FakeBuildStatus(Mock):
         self.builder = FakeBuilder(
             fakemaster.make_master(wantData=True, testcase=self))
         self.build.setBuilder(self.builder)
+        self.properties = self.build.properties = FakeProperties()
         self.build_status = FakeBuildStatus()
         self.build.startBuild(self.build_status, None, self.slavebuilder)
 
     def test_getProperty(self):
         self.build.getProperty('x')
-        self.build_status.getProperty.assert_called_with('x', None)
+        self.properties.getProperty.assert_called_with('x', None)
 
     def test_getProperty_default(self):
         self.build.getProperty('x', 'nox')
-        self.build_status.getProperty.assert_called_with('x', 'nox')
+        self.properties.getProperty.assert_called_with('x', 'nox')
 
     def test_setProperty(self):
         self.build.setProperty('n', 'v', 's')
-        self.build_status.setProperty.assert_called_with('n', 'v', 's',
-                                                         runtime=True)
+        self.properties.setProperty.assert_called_with('n', 'v', 's',
+                                                       runtime=True)
 
     def test_hasProperty(self):
-        self.build_status.hasProperty.return_value = True
+        self.properties.hasProperty.return_value = True
         self.assertTrue(self.build.hasProperty('p'))
-        self.build_status.hasProperty.assert_called_with('p')
+        self.properties.hasProperty.assert_called_with('p')
 
     def test_has_key(self):
-        self.build_status.has_key.return_value = True
+        self.properties.has_key.return_value = True
         # getattr because pep8 doesn't like calls to has_key
         self.assertTrue(getattr(self.build, 'has_key')('p'))
         # has_key calls through to hasProperty
-        self.build_status.hasProperty.assert_called_with('p')
+        self.properties.hasProperty.assert_called_with('p')
 
     def test_render(self):
         self.build.render("xyz")
-        self.build_status.render.assert_called_with("xyz")
+        self.properties.render.assert_called_with("xyz")
diff --git a/master/buildbot/test/unit/test_status_build.py b/master/buildbot/test/unit/test_status_build.py
diff --git a/master/buildbot/test/unit/test_status_builder_cache.py b/master/buildbot/test/unit/test_status_builder_cache.py
@@ -54,23 +54,18 @@ def testBuildCache(self):
         builds = []
         for i in xrange(5):
             build = b.newBuild()
-            build.setProperty('propkey', 'propval%d' % i, 'test')
             builds.append(build)
             build.buildStarted(build)
             build.buildFinished()
         for build in builds:
             build2 = b.getBuild(build.number)
             self.assertTrue(build2)
             self.assertEqual(build2.number, build.number)
-            self.assertEqual(build2.getProperty('propkey'),
-                             'propval%d' % build.number)
         # Do another round, to make sure we're hitting the cache
         hits = b.buildCache.hits
         for build in builds:
             build2 = b.getBuild(build.number)
             self.assertTrue(build2)
             self.assertEqual(build2.number, build.number)
-            self.assertEqual(build2.getProperty('propkey'),
-                             'propval%d' % build.number)
             self.assertEqual(b.buildCache.hits, hits + 1)
             hits = hits + 1
diff --git a/master/buildbot/test/unit/test_steps_trigger.py b/master/buildbot/test/unit/test_steps_trigger.py
@@ -18,11 +18,11 @@
 from buildbot import config
 from buildbot import interfaces
 from buildbot.process import properties
-from buildbot.status import master
 from buildbot.process.results import CANCELLED
 from buildbot.process.results import EXCEPTION
 from buildbot.process.results import FAILURE
 from buildbot.process.results import SUCCESS
+from buildbot.status import master
 from buildbot.steps import trigger
 from buildbot.test.fake import fakedb
 from buildbot.test.util import steps
@@ -144,7 +144,7 @@ def getAllSourceStamps():
 
         def getAllGotRevisions():
             return got_revisions
-        self.build.build_status.getAllGotRevisions = getAllGotRevisions
+        self.step.getAllGotRevisions = getAllGotRevisions
 
         self.exp_add_sourcestamp = None
         self.exp_a_trigger = None

diff --git a/master/docs/relnotes/index.rst b/master/docs/relnotes/index.rst
@@ -29,6 +29,10 @@ Deprecations, Removals, and Non-Compatible Changes
 Changes for Developers
 ~~~~~~~~~~~~~~~~~~~~~~
 
+* properties object is now directly present in build, and not in build_status.
+  This should not change much unless you try to access your properties via step.build.build_status.
+  Remember that with PropertiesMixin, you can access properties via getProperties on the steps, and on the builds objects.
+
 Slave
 -----