swarmbucket: refactor builder defaults

swarmbucket: refactor builder defaults

Replace common_* fields with one message builder_defaults of type
Builder. Teach swarmbucket to merge builder configs.

Next CL will allow a build to override a builder config using the same
merging algorithm.

R=vadimsh@chromium.org, borenet@google.com
BUG=

Committed: https://chromium.googlesource.com/infra/infra/+/dfd87124fa28c46081f367214e23d008204831a0

[nodir, 2016-08-03 20:40:33]

PTAL

Good test coverage.
Deployed to cr-buildbucket-dev.appspot.com
and updated personal config:
https://chromium.googlesource.com/infra/experimental/+/refs/heads/infra/confi...
    builder_defaults {
      swarming_tags: "allow_milo:1"
      swarming_tags: "master:master.nodir"
      dimensions: "cores:8"
      dimensions: "pool:default"
      recipe {
        repository: "https://chromium.googlesource.com/chromium/tools/build"
        name: "bar"
        properties: "a:b"
        properties: "x:y"
      }
    }
    builders {
      name: "foo"
      dimensions: "os:Ubuntu"
      recipe {
        properties_j: "a:"
      }
    }
 

scheduled a build it got properties x and not a (because a is cancelled out):
https://chromium-swarm-dev.appspot.com/user/task/306ae97231d98710

[Vadim Sh., 2016-08-03 20:58:12]

lgtm

but why?

https://codereview.chromium.org/2209583003/diff/20001/appengine/cr-buildbucke...
File appengine/cr-buildbucket/swarming/swarmingcfg.py (right):

https://codereview.chromium.org/2209583003/diff/20001/appengine/cr-buildbucke...
appengine/cr-buildbucket/swarming/swarmingcfg.py:16: """Clones a protobuf
message."""
how does it handle fields that are protos themselves (i.e. is it shallow or deep
clone)? Add a note in the comment.

https://codereview.chromium.org/2209583003/diff/20001/appengine/cr-buildbucke...
appengine/cr-buildbucket/swarming/swarmingcfg.py:32: if not v:
I find it a bit sad that 'properties: "a:"' and 'properties_j: "a:"' mean very
different things :(

Can 'properties' be eliminated completely in favor of 'properties_j'?

https://codereview.chromium.org/2209583003/diff/20001/appengine/cr-buildbucke...
appengine/cr-buildbucket/swarming/swarmingcfg.py:219: subctx = ctx.proxy(ctx)
huh? what is 'proxy'?

[nodir, 2016-08-03 21:38:11]

> why?
next CL will allow to override builder config at the build level. Now with
merge_builder it is easier.

also builder_defaults { ... } is nicer than adhoc common_* fields

https://codereview.chromium.org/2209583003/diff/20001/appengine/cr-buildbucke...
File appengine/cr-buildbucket/swarming/swarmingcfg.py (right):

https://codereview.chromium.org/2209583003/diff/20001/appengine/cr-buildbucke...
appengine/cr-buildbucket/swarming/swarmingcfg.py:16: """Clones a protobuf
message."""
On 2016/08/03 20:58:11, Vadim Sh. wrote:
> how does it handle fields that are protos themselves (i.e. is it shallow or
deep
> clone)? Add a note in the comment.

removed in favor of copy.deepcopy because this thing does the same
see also 
https://developers.google.com/protocol-buffers/docs/reference/python/google.p...

https://codereview.chromium.org/2209583003/diff/20001/appengine/cr-buildbucke...
appengine/cr-buildbucket/swarming/swarmingcfg.py:32: if not v:
On 2016/08/03 20:58:12, Vadim Sh. wrote:
> I find it a bit sad that 'properties: "a:"' and 'properties_j: "a:"' mean very
> different things :(
> 
> Can 'properties' be eliminated completely in favor of 'properties_j'?

unfortunately no, because

properties_j: "a:\"b\""

is worse :(

https://codereview.chromium.org/2209583003/diff/20001/appengine/cr-buildbucke...
appengine/cr-buildbucket/swarming/swarmingcfg.py:219: subctx = ctx.proxy(ctx)
On 2016/08/03 20:58:11, Vadim Sh. wrote:
> huh? what is 'proxy'?

ugh, I forgot that validation context is in a different repo. Anyway, moved that
function here

[nodir, 2016-08-03 21:38:23]

The CQ bit was checked by nodir@chromium.org

[nodir, 2016-08-03 21:38:23]

The patchset sent to the CQ was uploaded after l-g-t-m from vadimsh@chromium.org
Link to the patchset: https://codereview.chromium.org/2209583003/#ps40001
(title: "deepcopy and subctx")

[commit-bot: I haz the power, 2016-08-03 21:38:25]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-03 21:55:44]

Description was changed from

==========
swarmbucket: refactor builder defaults

Replace common_* fields with one message builder_defaults of type
Builder. Teach swarmbucket to merge builder configs.

Next CL will allow a build to override a builder config using the same
merging algorithm.

R=vadimsh@chromium.org, borenet@google.com
BUG=
==========

to

==========
swarmbucket: refactor builder defaults

Replace common_* fields with one message builder_defaults of type
Builder. Teach swarmbucket to merge builder configs.

Next CL will allow a build to override a builder config using the same
merging algorithm.

R=vadimsh@chromium.org, borenet@google.com
BUG=

Committed:
https://chromium.googlesource.com/infra/infra/+/dfd87124fa28c46081f367214e23d...
==========

[commit-bot: I haz the power, 2016-08-03 21:55:45]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/infra/infra/+/dfd87124fa28c46081f367214e23d...