Merge branch '9/use-data-api-1' of git://github.com/djmitche/buildbot…

… into nine (PR #1023)

Conflicts:
	master/buildbot/process/build.py
	master/buildbot/process/builder.py

+autopep8, docs fixes, etc.

README.md

@@ -74,6 +74,8 @@ The "process" part of Buildbot is the part that coordinates all of the other par
   Builds for merged requests currently only refer to one of the merged requests.
 * In the Trigger step, add links from the triggering step to the triggered builds
 * Create builds, steps, logs, and log chunks during execution of a build.
+* Simplify LogLineObserver - this class is designed to handle arbitrary chunks of log data, but now chunks always end on a line boundary.
+  It can be substantially simplified to just split each chunk into lines and call {err,out}LineReceived for each line. :runner:
 
 ## Documentation ##
 

master/buildbot/config.py

@@ -72,8 +72,9 @@ def __init__(self):
         self.buildHorizon = None
         self.logCompressionLimit = 4 * 1024
         self.logCompressionMethod = 'bz2'
-        self.logMaxTailSize = None
+        self.logEncoding = 'utf-8'
         self.logMaxSize = None
+        self.logMaxTailSize = None
         self.properties = properties.Properties()
         self.mergeRequests = None
         self.codebaseGenerator = None
@@ -117,12 +118,12 @@ def __init__(self):
         "buildbotURL", "buildCacheSize", "builders", "buildHorizon", "caches",
         "change_source", "codebaseGenerator", "changeCacheSize", "changeHorizon",
         'db', "db_poll_interval", "db_url", "debugPassword", "eventHorizon",
-        "logCompressionLimit", "logCompressionMethod", "logHorizon",
-        "logMaxSize", "logMaxTailSize", "manhole", "mergeRequests", "metrics",
-        "mq", "multiMaster", "prioritizeBuilders", "projectName", "projectURL",
-        "properties", "protocols", "revlink", "schedulers", "slavePortnum",
-        "slaves", "status", "title", "titleURL", "user_managers", "validation",
-        'www'
+        "logCompressionLimit", "logCompressionMethod", "logEncoding",
+        "logHorizon", "logMaxSize", "logMaxTailSize", "manhole",
+        "mergeRequests", "metrics", "mq", "multiMaster", "prioritizeBuilders",
+        "projectName", "projectURL", "properties", "protocols", "revlink",
+        "schedulers", "slavePortnum", "slaves", "status", "title", "titleURL",
+        "user_managers", "validation", 'www'
     ])
 
     @classmethod
@@ -273,6 +274,7 @@ def copy_str_param(name, alt_key=None):
 
         copy_int_param('logMaxSize')
         copy_int_param('logMaxTailSize')
+        copy_param('logEncoding')
 
         properties = config_dict.get('properties', {})
         if not isinstance(properties, dict):

master/buildbot/data/logchunks.py

@@ -25,11 +25,11 @@ class LogChunkEndpoint(base.BuildNestingMixin, base.Endpoint):
     isCollection = False
     pathPatterns = """
         /log/n:logid/content
-        /step/n:stepid/log/i:log_name/content
-        /build/n:buildid/step/i:step_name/log/i:log_name/content
-        /build/n:buildid/step/n:step_number/log/i:log_name/content
-        /builder/n:builderid/build/n:build_number/step/i:step_name/log/i:log_name/content
-        /builder/n:builderid/build/n:build_number/step/n:step_number/log/i:log_name/content
+        /step/n:stepid/log/i:log_slug/content
+        /build/n:buildid/step/i:step_name/log/i:log_slug/content
+        /build/n:buildid/step/n:step_number/log/i:log_slug/content
+        /builder/n:builderid/build/n:build_number/step/i:step_name/log/i:log_slug/content
+        /builder/n:builderid/build/n:build_number/step/n:step_number/log/i:log_slug/content
     """
 
     @defer.inlineCallbacks
@@ -42,8 +42,8 @@ def get(self, resultSpec, kwargs):
             stepid = yield self.getStepid(kwargs)
             if stepid is None:
                 return
-            dbdict = yield self.master.db.logs.getLogByName(stepid,
-                                                            kwargs.get('log_name'))
+            dbdict = yield self.master.db.logs.getLogBySlug(stepid,
+                                                            kwargs.get('log_slug'))
             if not dbdict:
                 return
             logid = dbdict['id']

master/buildbot/data/logs.py

@@ -15,6 +15,7 @@
 
 from buildbot.data import base
 from buildbot.data import types
+from buildbot.util import identifiers
 from twisted.internet import defer
 
 
@@ -24,6 +25,7 @@ def db2data(self, dbdict):
         data = {
             'logid': dbdict['id'],
             'name': dbdict['name'],
+            'slug': dbdict['slug'],
             'stepid': dbdict['stepid'],
             'step_link': base.Link(('step', str(dbdict['stepid']))),
             'complete': dbdict['complete'],
@@ -39,11 +41,11 @@ class LogEndpoint(EndpointMixin, base.BuildNestingMixin, base.Endpoint):
     isCollection = False
     pathPatterns = """
         /log/n:logid
-        /step/n:stepid/log/i:log_name
-        /build/n:buildid/step/i:step_name/log/i:log_name
-        /build/n:buildid/step/n:step_number/log/i:log_name
-        /builder/n:builderid/build/n:build_number/step/i:step_name/log/i:log_name
-        /builder/n:builderid/build/n:build_number/step/n:step_number/log/i:log_name
+        /step/n:stepid/log/i:log_slug
+        /build/n:buildid/step/i:step_name/log/i:log_slug
+        /build/n:buildid/step/n:step_number/log/i:log_slug
+        /builder/n:builderid/build/n:build_number/step/i:step_name/log/i:log_slug
+        /builder/n:builderid/build/n:build_number/step/n:step_number/log/i:log_slug
     """
 
     @defer.inlineCallbacks
@@ -58,8 +60,8 @@ def get(self, resultSpec, kwargs):
         if stepid is None:
             return
 
-        dbdict = yield self.master.db.logs.getLogByName(stepid,
-                                                        kwargs.get('log_name'))
+        dbdict = yield self.master.db.logs.getLogBySlug(stepid,
+                                                        kwargs.get('log_slug'))
         defer.returnValue((yield self.db2data(dbdict))
                           if dbdict else None)
 
@@ -94,7 +96,8 @@ class Log(base.ResourceType):
 
     class EntityType(types.Entity):
         logid = types.Integer()
-        name = types.Identifier(50)
+        name = types.String()
+        slug = types.Identifier(50)
         stepid = types.Integer()
         step_link = types.Link()
         complete = types.Boolean()
@@ -104,9 +107,17 @@ class EntityType(types.Entity):
     entityType = EntityType(name)
 
     @base.updateMethod
+    @defer.inlineCallbacks
     def newLog(self, stepid, name, type):
-        return self.master.db.logs.addLog(
-            stepid=stepid, name=name, type=type)
+        slug = name
+        while True:
+            try:
+                logid = yield self.master.db.logs.addLog(
+                    stepid=stepid, name=name, slug=slug, type=type)
+            except KeyError:
+                slug = identifiers.incrementIdentifier(50, slug)
+                continue
+            defer.returnValue(logid)
 
     @base.updateMethod
     def finishLog(self, logid):

master/buildbot/data/steps.py

@@ -65,8 +65,6 @@ def get(self, resultSpec, kwargs):
         defer.returnValue((yield self.db2data(dbdict))
                           if dbdict else None)
 
-# FIXME: need unit tests!
-
 
 class StepsEndpoint(Db2DataMixin, base.BuildNestingMixin, base.Endpoint):
 

master/buildbot/db/buildslaves.py

@@ -16,7 +16,7 @@
 import sqlalchemy as sa
 
 from buildbot.db import base
-from buildbot.util import typechecks
+from buildbot.util import identifiers
 from twisted.internet import defer
 
 
@@ -26,7 +26,7 @@ class BuildslavesConnectorComponent(base.DBConnectorComponent):
     def findBuildslaveId(self, name):
         tbl = self.db.model.buildslaves
         # callers should verify this and give good user error messages
-        assert typechecks.isIdentifier(50, name)
+        assert identifiers.isIdentifier(50, name)
         return self.findSomethingId(
             tbl=tbl,
             whereclause=(tbl.c.name == name),

master/buildbot/db/logs.py

@@ -42,9 +42,9 @@ def thd(conn):
     def getLog(self, logid):
         return self._getLog(self.db.model.logs.c.id == logid)
 
-    def getLogByName(self, stepid, name):
+    def getLogBySlug(self, stepid, slug):
         tbl = self.db.model.logs
-        return self._getLog((tbl.c.name == name) & (tbl.c.stepid == stepid))
+        return self._getLog((tbl.c.slug == slug) & (tbl.c.stepid == stepid))
 
     def getLogs(self, stepid):
         def thd(conn):
@@ -86,17 +86,18 @@ def thd(conn):
             return u'\n'.join(rv) + u'\n' if rv else u''
         return self.db.pool.do(thd)
 
-    def addLog(self, stepid, name, type):
+    def addLog(self, stepid, name, slug, type):
         assert type in 'tsh', "Log type must be one of t, s, or h"
 
         def thd(conn):
             try:
                 r = conn.execute(self.db.model.logs.insert(),
-                                 dict(name=name, stepid=stepid, complete=0,
-                                      num_lines=0, type=type))
+                                 dict(name=name, slug=slug, stepid=stepid,
+                                      complete=0, num_lines=0, type=type))
                 return r.inserted_primary_key[0]
             except (sa.exc.IntegrityError, sa.exc.ProgrammingError):
-                raise KeyError("log with name '%r' already exists in this step" % (name,))
+                raise KeyError(
+                    "log with slug '%r' already exists in this step" % (slug,))
         return self.db.pool.do(thd)
 
     def appendLog(self, logid, content):

master/buildbot/db/migrate/versions/034_log_slug.py

@@ -0,0 +1,67 @@
+# 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
+
+    logs = sa.Table('logs', metadata, autoload=True)
+    logs.drop()
+    logchunks = sa.Table('logchunks', metadata, autoload=True)
+    logchunks.drop()
+
+    metadata = sa.MetaData()
+    metadata.bind = migrate_engine
+
+    sa.Table('steps', metadata, autoload=True)
+    logs = sa.Table(
+        'logs', metadata,
+        sa.Column('id', sa.Integer, primary_key=True),
+        sa.Column('name', sa.Text, nullable=False),
+        sa.Column('slug', sa.String(50), nullable=False),
+        sa.Column('stepid', sa.Integer, sa.ForeignKey('steps.id')),
+        sa.Column('complete', sa.SmallInteger, nullable=False),
+        sa.Column('num_lines', sa.Integer, nullable=False),
+        # 's' = stdio, 't' = text, 'h' = html
+        sa.Column('type', sa.String(1), nullable=False),
+    )
+    logs.create()
+
+    logchunks = sa.Table(
+        'logchunks', metadata,
+        sa.Column('logid', sa.Integer, sa.ForeignKey('logs.id')),
+        # 0-based line number range in this chunk (inclusive); note that for
+        # HTML logs, this counts lines of HTML, not lines of rendered output
+        sa.Column('first_line', sa.Integer, nullable=False),
+        sa.Column('last_line', sa.Integer, nullable=False),
+        # log contents, including a terminating newline, encoded in utf-8 or,
+        # if 'compressed' is true, compressed with gzip
+        sa.Column('content', sa.LargeBinary(65536)),
+        sa.Column('compressed', sa.SmallInteger, nullable=False),
+    )
+    logchunks.create()
+
+    idx = sa.Index('logs_slug',
+                   logs.c.stepid, logs.c.slug, unique=True)
+    idx.create()
+    idx = sa.Index('logchunks_firstline',
+                   logchunks.c.logid, logchunks.c.first_line)
+    idx.create()
+    idx = sa.Index('logchunks_lastline',
+                   logchunks.c.logid, logchunks.c.last_line)
+    idx.create()

master/buildbot/db/model.py

@@ -131,7 +131,8 @@ class Model(base.DBConnectorComponent):
 
     logs = sa.Table('logs', metadata,
                     sa.Column('id', sa.Integer, primary_key=True),
-                    sa.Column('name', sa.String(50), nullable=False),
+                    sa.Column('name', sa.Text, nullable=False),
+                    sa.Column('slug', sa.String(50), nullable=False),
                     sa.Column('stepid', sa.Integer, sa.ForeignKey('steps.id')),
                     sa.Column('complete', sa.SmallInteger, nullable=False),
                     sa.Column('num_lines', sa.Integer, nullable=False),
@@ -599,7 +600,7 @@ class Model(base.DBConnectorComponent):
              unique=True)
     sa.Index('steps_name', steps.c.buildid, steps.c.name,
              unique=True)
-    sa.Index('logs_name', logs.c.stepid, logs.c.name, unique=True)
+    sa.Index('logs_slug', logs.c.stepid, logs.c.slug, unique=True)
     sa.Index('logchunks_firstline', logchunks.c.logid, logchunks.c.first_line)
     sa.Index('logchunks_lastline', logchunks.c.logid, logchunks.c.last_line)
 

master/buildbot/process/build.py

@@ -278,7 +278,8 @@ def startBuild(self, build_status, expectations, slavebuilder):
             self.deferred = None
             return
 
-        self.master.data.updates.setBuildStateStrings(self.buildid, [u'starting'])
+        yield self.master.data.updates.setBuildStateStrings(self.buildid,
+                                                            [u'starting'])
         self.build_status.buildStarted(self)
         yield self.acquireLocks()
 
@@ -289,7 +290,9 @@ def startBuild(self, build_status, expectations, slavebuilder):
         finally:
             metrics.MetricCountEvent.log('active_builds', -1)
 
-        yield self.master.db.builds.finishBuild(self.buildid, self.results)
+        yield self.master.data.updates.finishBuild(self.buildid, self.results)
+        yield self.master.data.updates.setBuildStateStrings(self.buildid,
+                                                            [u'finished'])
 
         # mark the build as finished
         self.slavebuilder.buildFinished()