Added TryJobGerritScheduler

scripts/master/try_job_gerrit.py

Index: scripts/master/try_job_gerrit.py
diff --git a/scripts/master/try_job_gerrit.py b/scripts/master/try_job_gerrit.py
new file mode 100644
index 0000000000000000000000000000000000000000..60e24755a81f2bcf56cac794650e2716a898cdc7
--- /dev/null
+++ b/scripts/master/try_job_gerrit.py
@@ -0,0 +1,141 @@
+# Copyright (c) 2014 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+import json
+import re
+
+from twisted.internet import defer
+from twisted.python import log
+
+from buildbot.changes import filter
+from buildbot.schedulers.basic import SingleBranchScheduler
+
+from master.gerrit_poller import GerritPoller
+from master.builders_pools import BuildersPools
+
+
+ALWAYS_TRUE_FILTER = filter.ChangeFilter(filter_fn=lambda x: True)
+
+
+class JobDefinition(object):
+  """Describes a try job posted on Gerrit."""
+  def __init__(self, builder_names=None):
+    # Force str type and remove empty builder names.
+    self.builder_names = [str(b) for b in builder_names or []
+                          if b]
+
+  def __repr__(self):
+    return repr(self.__dict__)
+
+  @staticmethod
+  def parse(text):
+    """Parses a try job definition."""
+    text = text and text.strip()
+    if not text:
+      # Return an empty definition.
+      return JobDefinition()
+
+    # Parse as json.
+    job = json.loads(text)
+
+    # Convert to canonical form.
+    if isinstance(job, list):
+      # Treat a list as builder name list.
+      job = {'builderNames': job}
+
+    return JobDefinition(job.get('builderNames'))
+
+
+class _TryJobGerritPoller(GerritPoller):
+  """Polls issues, creates changes and calls scheduler.submitJob.
+
+  This class is a part of TryJobGerritScheduler implementation and not designed
+  to be used otherwise.
+  """
+
+  MESSAGE_REGEX_TRYJOB = re.compile('Patch set \d+:\s+\!tryjob', re.I)
+
+  def __init__(self, scheduler, gerrit_host, gerrit_projects=None,
+               pollInterval=None):
+    assert scheduler
+    GerritPoller.__init__(self, gerrit_host, gerrit_projects, pollInterval)
+    self.scheduler = scheduler

[Dan Jacques, 2014/04/26 00:38:51]

I don't think the 'change source' / 'scheduler' set is supposed to be directly
aware of each other. If the scheduler is so tightly coupled, consider just
moving the scheduler's "SubmitJob" etc. functions into this class.

[nodir, 2014/04/28 22:31:11]

In general, yes, a poller and scheduler don't know about each other. They talk
to each other through buildbot changes, but in here TryJobGerritPoller is
essentially a part of TryJobGerritScheduler because the scheduler needs the
tryjob definition, not available in a change. I would like not to put it into
properties. Anyway, the can't work without each other.

Note that addBuildbotChange method is in the poller, while addBuildsetForXXX
methods are in the scheduler, so they can't be completely moved to one of the
classes. I decided to do this way because pollers usually create changes and
schedulers usually create sourcestamps and buildsets.

+
+  def _is_interesting_comment(self, comment):
+    return self.MESSAGE_REGEX_TRYJOB.match(comment['message'])
+
+  def getChangeQuery(self):
+    query = GerritPoller.getChangeQuery(self)
+    # Request only issues with TryJob=+1 label.
+    query += '+label:TryJob=%2B1'

[Dan Jacques, 2014/04/26 00:38:51]

While you're constraining the query, you might as well also constrain by the
projects in the "projects" array.

[nodir, 2014/04/28 22:31:11]

I think I will wait for your GerritAgent

+    return query
+
+  def parseJob(self, comment):
+    """Parses a JobDefinition from a Gerrit comment."""
+    msg = comment['message']
+    tryjob_match = self.MESSAGE_REGEX_TRYJOB.match(msg)
+    assert tryjob_match
+    job_def_str = msg[tryjob_match.end():]
+    return JobDefinition.parse(job_def_str)
+
+  @defer.inlineCallbacks
+  def addChange(self, (change, comment)):
+    """Parses a job, adds a change and calls self.scheduler.submitJob."""
+    try:
+      job = self.parseJob(comment)
+      buildbotChange = yield self.addBuildbotChange(change, comment)
+      self.scheduler.submitJob(buildbotChange, job)

[Dan Jacques, 2014/04/26 00:38:51]

This needs to be yielded, doesn't it?

[nodir, 2014/04/28 22:31:11]

Done.

+    except Exception as e:
+      log.err('TryJobGerritPoller failed: %s' % e)
+
+
+class TryJobGerritScheduler(SingleBranchScheduler):

[Dan Jacques, 2014/04/26 00:38:51]

It would be nice if you could use a less heavy-handed scheduler. I recommend
setting the 'category' field in the 'addChange' to something like 'tryjob' and
have a simple scheduler that filters on "category='tryjob'". That way, you don't
have to implement all of this plumbing.

[nodir, 2014/04/28 22:31:11]

This scheduler takes the builder names from the job description. Standard
schedulers do have a mechanism to dynamically specify the list of builders. 

Still, I inherited TryJobGerritScheduler from BaseScheduler (the simplest one),
added a category to a buildbot change and used addBuildsetForChanges, so the
source stamp is created by the BaseScheduler.

+  """Polls try jobs on Gerrit and creates buildsets."""
+  def __init__(self, name, default_builder_names, gerrit_host,
+               gerrit_projects=None, pollInterval=None):
+    """Creates a new TryJobGerritScheduler.
+
+    Args:
+        name: name of the scheduler.
+        default_builder_names: a list of builder names used in case a job didn't
+            specify any.
+        gerrit_host: URL to the Gerrit instance
+        gerrit_projects: Gerrit projects to filter issues.
+        pollInterval: frequency of polling.
+    """
+    SingleBranchScheduler.__init__(self, name,
+                                   builderNames=default_builder_names,

[Kevin Graney, 2014/04/26 02:45:11]

Are you somehow running PRESUBMIT.py for the jobs scheduled off of Gerrit? 
Being able to have PRESUBMIT.py generate a preset list of default builders and
steps to run is pretty useful.

[nodir, 2014/04/28 22:31:11]

This code lives on server. The git-try will run PRESUBMIT and post a job
definition in a Gerrit comment, which will be parsed by this code

+                                   change_filter=ALWAYS_TRUE_FILTER)
+    self.poller = _TryJobGerritPoller(self, gerrit_host, gerrit_projects,
+                                      pollInterval)
+
+  def setServiceParent(self, parent):
+    SingleBranchScheduler.setServiceParent(self, parent)
+    self.poller.master = self.master
+    self.poller.setServiceParent(self)
+
+  def gotChange(self, *args, **kwargs):
+    """Do nothing because changes are processed by submitJob."""
+
+  def addSourcestamp(self, change):
+    return self.master.db.sourcestamps.addSourceStamp(
+        project=change.project,
+        repository=change.repository,
+        branch=change.branch,
+        revision=change.revision)
+
+  def addBuildset(self, change, ssid, job):
+    return self.addBuildsetForSourceStamp(
+        ssid=ssid,
+        reason='tryjob',
+        properties=change.properties,
+        builderNames=job.builder_names)
+
+  @defer.inlineCallbacks
+  def submitJob(self, change, job):
+    ssid = yield self.addSourcestamp(change)
+    bsid = yield self.addBuildset(change, ssid, job)
+    log.msg('Successfully submitted a Gerrit try job for %s: %s.' % (change.who,
+                                                                     job))
+    defer.returnValue(bsid)