Permalink
Browse files

Merge commit '50fa054' into pull385

* commit '50fa054':
  Move method to base scheduler
  Should use username not owner
  Change the reason text to not be web specific
  Move webstatus specific code to b.s.w.builder
  Use a specific error message instead of ValueError
  get_from_post -> getFromKwargs
  Remove Python2.4ism
  Refactor so that ForceScheduler isn't as tightly coupled to HTML
  Use a dict() for r.args (the real one is a dict anyway)
  Break out property gathering and scheduling into seperate functions
  • Loading branch information...
2 parents 6473eb7 + 50fa054 commit 3333c0c5850c8db4a8286e1eaff2cf96f7e3fc11 @djmitche djmitche committed May 5, 2012
View
39 master/buildbot/schedulers/base.py
@@ -254,7 +254,6 @@ def gotChange(self, change, important):
## starting bulids
- @defer.deferredGenerator
def addBuildsetForLatest(self, reason='', external_idstring=None,
branch=None, repository='', project='',
builderNames=None, properties=None):
@@ -263,16 +262,45 @@ def addBuildsetForLatest(self, reason='', external_idstring=None,
repository, and project. This will create a relative sourcestamp for
the buildset.
- This method will add any properties provided to the scheduler
- constructor to the buildset, and will call the master's addBuildset
- method with the appropriate parameters.
+ @param reason: reason for this buildset
+ @type reason: unicode string
+ @param external_idstring: external identifier for this buildset, or None
+ @param branch: branch to build (note that None often has a special meaning)
+ @param repository: repository name for sourcestamp
+ @param project: project name for sourcestamp
+ @param builderNames: builders to name in the buildset (defaults to
+ C{self.builderNames})
+ @param properties: a properties object containing initial properties for
+ the buildset
+ @type properties: L{buildbot.process.properties.Properties}
+ @returns: (buildset ID, buildrequest IDs) via Deferred
+ """
+
+ return self.addBuildSetForSourceStampDetails(
+ reason = reason,
+ external_idstring = external_idstring,
+ branch = branch,
+ repository = repository,
+ project = project,
+ builderNames = builderNames,
+ properties = properties
+ )
+
+ @defer.deferredGenerator
+ def addBuildSetForSourceStampDetails(self, reason='', external_idstring=None,
+ branch=None, repository='', project='', revision=None,
+ builderNames=None, properties=None):
+ """
+ Given details about the source code to build, create a source stamp and
+ then add a buildset for it.
@param reason: reason for this buildset
@type reason: unicode string
@param external_idstring: external identifier for this buildset, or None
@param branch: branch to build (note that None often has a special meaning)
@param repository: repository name for sourcestamp
@param project: project name for sourcestamp
+ @param revision: revision to build - default is latest
@param builderNames: builders to name in the buildset (defaults to
C{self.builderNames})
@param properties: a properties object containing initial properties for
@@ -286,7 +314,7 @@ def addBuildsetForLatest(self, reason='', external_idstring=None,
setid = wfd.getResult()
wfd = defer.waitForDeferred(self.master.db.sourcestamps.addSourceStamp(
- branch=branch, revision=None, repository=repository,
+ branch=branch, revision=revision, repository=repository,
project=project, sourcestampsetid=setid))
yield wfd
wfd.getResult()
@@ -415,3 +443,4 @@ def addBuildsetForSourceStamp(self, ssid=None, setid=None, reason='', external_i
external_idstring=external_idstring))
yield wfd
yield wfd.getResult()
+
View
160 master/buildbot/schedulers/forcesched.py
@@ -16,16 +16,14 @@
import traceback
import re
from twisted.internet import defer
-try:
- import email.utils as email_utils
- email_utils = email_utils
-except ImportError:
- # Python-2.4 capitalization
- import email.Utils as email_utils
+import email.utils as email_utils
from buildbot.process.properties import Properties
from buildbot.schedulers import base
+class ValidationError(ValueError):
+ pass
+
class BaseParameter(object):
name = ""
label = ""
@@ -45,19 +43,19 @@ def __init__(self, name, label=None, regex=None, **kw):
# all other properties are generically passed via **kw
self.__dict__.update(kw)
- def update_from_post(self, master, properties, changes, req):
- args = req.args.get(self.name, [])
+ def getFromKwargs(self, kwargs):
+ args = kwargs.get(self.name, [])
if len(args) == 0:
if self.required:
- raise ValueError("'%s' needs to be specified" % (self.label))
+ raise ValidationError("'%s' needs to be specified" % (self.label))
if self.multiple:
args = self.default
else:
args = [self.default]
if self.regex:
for arg in args:
if not self.regex.match(arg):
- raise ValueError("%s:'%s' does not match pattern '%s'"
+ raise ValidationError("%s:'%s' does not match pattern '%s'"
% (self.label, arg, self.regex.pattern))
try:
arg = self.parse_from_args(args)
@@ -68,9 +66,12 @@ def update_from_post(self, master, properties, changes, req):
traceback.print_exc()
raise e
if arg == None:
- raise ValueError("need %s: no default provided by config"
+ raise ValidationError("need %s: no default provided by config"
% (self.name,))
- properties[self.name] = arg
+ return arg
+
+ def updateFromKwargs(self, master, properties, changes, kwargs):
+ properties[self.name] = self.getFromKwargs(kwargs)
def parse_from_args(self, l):
if self.multiple:
@@ -117,10 +118,8 @@ class IntParameter(StringParameter):
class BooleanParameter(BaseParameter):
type = "bool"
- def update_from_post(self, master, properties, changes, req):
- # damn html's ungeneric checkbox implementation...
- checkbox = req.args.get("checkbox", [""])
- properties[self.name] = self.name in checkbox
+ def getFromKwargs(self, kwargs):
+ return self.name in kwargs and kwargs[self.name] == [True]
class UserNameParameter(StringParameter):
@@ -138,7 +137,7 @@ def parse_from_arg(self, s):
if self.need_email:
e = email_utils.parseaddr(s)
if e[0]=='' or e[1] == '':
- raise ValueError("%s: please fill in email address in the "
+ raise ValidationError("%s: please fill in email address in the "
" form User <email@email.com>" % (self.label,))
return s
@@ -150,26 +149,29 @@ class ChoiceStringParameter(BaseParameter):
def parse_from_arg(self, s):
if self.strict and not s in self.choices:
- raise ValueError("'%s' does not belongs to list of available choices '%s'"%(s, self.choices))
+ raise ValidationError("'%s' does not belongs to list of available choices '%s'"%(s, self.choices))
return s
class InheritBuildParameter(ChoiceStringParameter):
name = "inherit"
compatible_builds = None
- def update_from_post(self, master, properties, changes, req):
- arg = req.args.get(self.name, [""])[0]
+ def getFromKwargs(self, kwargs):
+ raise ValidationError("InheritBuildParameter can only be used by properties")
+
+ def updateFromKwargs(self, master, properties, changes, kwargs):
+ arg = kwargs.get(self.name, [""])[0]
splitted_arg = arg.split(" ")[0].split("/")
if len(splitted_arg) != 2:
- raise ValueError("bad build: %s"%(arg))
+ raise ValidationError("bad build: %s"%(arg))
builder, num = splitted_arg
builder_status = master.status.getBuilder(builder)
if not builder_status:
- raise ValueError("unknown builder: %s in %s"%(builder, arg))
+ raise ValidationError("unknown builder: %s in %s"%(builder, arg))
b = builder_status.getBuild(int(num))
if not b:
- raise ValueError("unknown build: %d in %s"%(num, arg))
+ raise ValidationError("unknown build: %d in %s"%(num, arg))
props = {self.name:(arg.split(" ")[0])}
for name, value, source in b.getProperties().asList():
if source == "Force Build Form":
@@ -183,17 +185,20 @@ def update_from_post(self, master, properties, changes, req):
class AnyPropertyParameter(BaseParameter):
type = "anyproperty"
- def update_from_post(self, master, properties, changes, req):
+ def getFromKwargs(self, kwargs):
+ raise ValidationError("AnyPropertyParameter can only be used by properties")
+
+ def updateFromKwargs(self, master, properties, changes, kwargs):
validation = master.config.validation
- pname = req.args.get("%sname" % self.name, [""])[0]
- pvalue = req.args.get("%svalue" % self.name, [""])[0]
+ pname = kwargs.get("%sname" % self.name, [""])[0]
+ pvalue = kwargs.get("%svalue" % self.name, [""])[0]
if not pname:
return
pname_validate = validation['property_name']
pval_validate = validation['property_value']
if not pname_validate.match(pname) \
or not pval_validate.match(pvalue):
- raise ValueError("bad property name='%s', value='%s'" % (pname, pvalue))
+ raise ValidationError("bad property name='%s', value='%s'" % (pname, pvalue))
properties[pname] = pvalue
@@ -236,69 +241,72 @@ def startService(self):
def stopService(self):
pass
- def forceWithWebRequest(self, owner, builder_name, req):
- """Called by the web UI.
- Authentication is already done, thus owner is passed as argument
+ @defer.inlineCallbacks
+ def gatherPropertiesAndChanges(self, **kwargs):
+ properties = {}
+ changeids = []
+
+ for param in self.forcedProperties:
+ yield defer.maybeDeferred(param.updateFromKwargs, self.master, properties, changeids, kwargs)
+
+ changeids = map(lambda a: type(a)==int and a or a.number, changeids)
+
+ real_properties = Properties()
+ for pname, pvalue in properties.items():
+ real_properties.setProperty(pname, pvalue, "Force Build Form")
+
+ defer.returnValue((real_properties, changeids))
+
+ @defer.inlineCallbacks
+ def force(self, owner, builder_name, **kwargs):
+ """
We check the parameters, and launch the build, if everything is correct
"""
if not builder_name in self.builderNames:
# in the case of buildAll, this method will be called several times
# for all the builders
# we just do nothing on a builder that is not in our builderNames
- return defer.succeed(None)
- master = self.master
- properties = {}
- changeids = []
+ defer.returnValue(None)
+
+ # Currently the validation code expects all kwargs to be lists
+ # I don't want to refactor that now so much sure we comply...
+ kwargs = dict((k, [v]) if not isinstance(v, list) else (k,v) for k,v in kwargs.items())
+
# probably need to clean that out later as the IProperty is already a
# validation mechanism
- validation = master.config.validation
+ validation = self.master.config.validation
if self.branch.regex == None:
self.branch.regex = validation['branch']
if self.revision.regex == None:
self.revision.regex = validation['revision']
- for param in self.all_fields:
- if owner and param==self.username:
- continue # dont enforce username if auth
- param.update_from_post(master, properties, changeids, req)
+ reason = self.reason.getFromKwargs(kwargs)
+ branch = self.branch.getFromKwargs(kwargs)
+ revision = self.revision.getFromKwargs(kwargs)
+ repository = self.repository.getFromKwargs(kwargs)
+ project = self.project.getFromKwargs(kwargs)
- changeids = map(lambda a: type(a)==int and a or a.number, changeids)
- # everything is validated, we can create our source stamp, and buildrequest
- reason = properties[self.reason.name]
- branch = properties[self.branch.name]
- revision = properties[self.revision.name]
- repository = properties[self.repository.name]
- project = properties[self.project.name]
if owner is None:
- owner = properties[self.username.name]
+ owner = self.username.getFromKwargs(kwargs)
+
+ properties, changeids = yield self.gatherPropertiesAndChanges(**kwargs)
+
+ properties.setProperty("reason", reason, "Force Build Form")
+ properties.setProperty("owner", owner, "Force Build Form")
+
+ r = ("A build was forced by '%s': %s" % (owner, reason))
+
+ # everything is validated, we can create our source stamp, and buildrequest
+ res = yield self.addBuildSetForSourceStampDetails(
+ reason = r,
+ branch = branch,
+ repository = repository,
+ revision = revision,
+ project = project,
+ builderNames = [builder_name],
+ properties = properties,
+ )
+
+ defer.returnValue(res)
- std_prop_names = [self.branch.name,
- self.revision.name, self.repository.name,
- self.project.name, self.username.name]
- real_properties = Properties()
- for pname, pvalue in properties.items():
- if not pname in std_prop_names:
- real_properties.setProperty(pname, pvalue, "Force Build Form")
-
- real_properties.setProperty("owner", owner, "Force Build Form")
-
- r = ("The web-page 'force build' button was pressed by '%s': %s"
- % (owner, reason))
-
- d = master.db.sourcestampsets.addSourceStampSet()
- def add_master_with_setid(sourcestampsetid):
- master.db.sourcestamps.addSourceStamp(
- sourcestampsetid = sourcestampsetid,
- branch=branch,
- revision=revision, project=project,
- repository=repository,changeids=changeids)
- return sourcestampsetid
-
- d.addCallback(add_master_with_setid)
- def got_setid(sourcestampsetid):
- return self.addBuildsetForSourceStamp(builderNames=[builder_name],
- setid=sourcestampsetid, reason=r,
- properties=real_properties)
- d.addCallback(got_setid)
- return d
View
14 master/buildbot/status/web/builder.py
@@ -138,13 +138,21 @@ def performAction(self, req):
defer.returnValue((path_to_builder(req, self.builder_status),
"forcescheduler arg not found"))
return
+
+ args = {}
+ # damn html's ungeneric checkbox implementation...
+ for cb in req.args.get("checkbox", []):
+ args[cb] = True
+ args.update(req.args)
+
+ builder_name = self.builder_status.getName()
+
for sch in master.allSchedulers():
if schedulername == sch.name:
try:
- yield sch.forceWithWebRequest(owner,
- self.builder_status.getName(), req)
+ yield self.force(owner, builder_name, **args)
msg = ""
- except Exception, e:
+ except ValidationError, e:
msg = html.escape(e.message.encode('ascii','ignore'))
break
View
90 master/buildbot/test/unit/test_schedulers_forcesched.py
@@ -15,11 +15,12 @@
import mock
from twisted.trial import unittest
+from twisted.internet import defer
from buildbot import config
from buildbot.schedulers.forcesched import ForceScheduler, StringParameter
from buildbot.schedulers.forcesched import IntParameter, FixedParameter
from buildbot.schedulers.forcesched import BooleanParameter, UserNameParameter
-from buildbot.schedulers.forcesched import ChoiceStringParameter
+from buildbot.schedulers.forcesched import ChoiceStringParameter, ValidationError
from buildbot.test.util import scheduler
class TestForceScheduler(scheduler.SchedulerMixin, unittest.TestCase):
@@ -40,20 +41,6 @@ def makeScheduler(self, name='testsched', builderNames=['a', 'b'],
sched.master.config = config.MasterConfig()
return sched
- def makeRequest(self, **args):
- r = mock.Mock()
- def get(key, default):
- if key in args:
- a = args[key]
- if type(a)==list:
- return a
- else:
- return [a]
- else:
- return default
- r.args.get = get
- return r
-
# tests
def test_compare_branch(self):
@@ -112,19 +99,18 @@ def test_compare_properties(self):
def test_basicForce(self):
sched = self.makeScheduler()
- req = self.makeRequest(branch='a',reason='because',revision='c',
- repository='d', project='p',
- property1name='p1',property1value='e',
- property2name='p2',property2value='f',
- property3name='p3',property3value='g',
- property4name='p4',property4value='h')
- d = sched.forceWithWebRequest('user', 'a', req)
+ d = sched.force('user', 'a', branch='a', reason='because',revision='c',
+ repository='d', project='p',
+ property1name='p1',property1value='e',
+ property2name='p2',property2value='f',
+ property3name='p3',property3value='g',
+ property4name='p4',property4value='h'
+ )
def check(res):
bsid,brids = res
self.db.buildsets.assertBuildset\
(bsid,
- dict(reason="The web-page 'force build' button was pressed by"
- " 'user': because",
+ dict(reason="A build was forced by 'user': because",
brids=brids,
external_idstring=None,
properties=[ ('owner', ('user', 'Force Build Form')),
@@ -144,39 +130,35 @@ def check(res):
return d
+ @defer.inlineCallbacks
def do_ParameterTest(self, value, expect, klass, owner='user', req=None,
**kwargs):
sched = self.makeScheduler(properties=[klass(name="p1",**kwargs)])
if not req:
- req = self.makeRequest(p1=value,reason='because')
+ req = dict(p1=value, reason='because')
try:
- d = sched.forceWithWebRequest(owner, 'a', req)
+ bsid, brids = yield sched.force(owner, 'a', **req)
except Exception,e:
if not isinstance(e, expect):
raise
- return # success
- def check(res):
- bsid,brids = res
- self.db.buildsets.assertBuildset\
- (bsid,
- dict(reason="The web-page 'force build' button was pressed "
- "by 'user': because",
- brids=brids,
- external_idstring=None,
- properties=[
- ('owner', ('user', 'Force Build Form')),
- ('p1', (expect, 'Force Build Form')),
- ('reason', ('because', 'Force Build Form')),
- ('scheduler', ('testsched', 'Scheduler')),
- ],
- sourcestampsetid=100),
- {"":
- dict(branch="", revision="", repository="", codebase='',
- project="", sourcestampsetid=100)
- })
- d.addCallback(check)
- return d
-
+ defer.returnValue(None) # success
+
+ self.db.buildsets.assertBuildset\
+ (bsid,
+ dict(reason="A build was forced by 'user': because",
+ brids=brids,
+ external_idstring=None,
+ properties=[
+ ('owner', ('user', 'Force Build Form')),
+ ('p1', (expect, 'Force Build Form')),
+ ('reason', ('because', 'Force Build Form')),
+ ('scheduler', ('testsched', 'Scheduler')),
+ ],
+ sourcestampsetid=100),
+ {"":
+ dict(branch="", revision="", repository="", codebase='',
+ project="", sourcestampsetid=100)
+ })
def test_StringParameter(self):
self.do_ParameterTest(value="testedvalue", expect="testedvalue",
@@ -192,13 +174,13 @@ def test_FixedParameter(self):
def test_BooleanParameter_True(self):
- req = self.makeRequest(checkbox=["p1"],reason='because')
+ req = dict(p1=True,reason='because')
self.do_ParameterTest(value="123", expect=True, klass=BooleanParameter,
req=req)
def test_BooleanParameter_False(self):
- req = self.makeRequest(checkbox=["p2"],reason='because')
+ req = dict(p2=True,reason='because')
self.do_ParameterTest(value="123", expect=False,
klass=BooleanParameter, req=req)
@@ -211,7 +193,7 @@ def test_UserNameParameter(self):
def test_UserNameParameterError(self):
for value in ["test","test@buildbot.net","<test@buildbot.net>"]:
- self.do_ParameterTest(value=value, expect=ValueError,
+ self.do_ParameterTest(value=value, expect=ValidationError,
klass=UserNameParameter, debug=False)
@@ -221,7 +203,7 @@ def test_ChoiceParameter(self):
def test_ChoiceParameterError(self):
- self.do_ParameterTest(value='t3', expect=ValueError,
+ self.do_ParameterTest(value='t3', expect=ValidationError,
klass=ChoiceStringParameter, choices=['t1','t2'],
debug=False)
@@ -232,6 +214,6 @@ def test_ChoiceParameterMultiple(self):
def test_ChoiceParameterMultipleError(self):
- self.do_ParameterTest(value=['t1','t3'], expect=ValueError,
+ self.do_ParameterTest(value=['t1','t3'], expect=ValidationError,
klass=ChoiceStringParameter, choices=['t1','t2'],
multiple=True, debug=False)

0 comments on commit 3333c0c

Please sign in to comment.