swarmbucket: fix empty dimension value

swarmbucket: fix empty dimension value

Make a dimension with no value valid and meaning that it cancels a value
for this dimension defined at wider scope, e.g. in common_dimension

R=tandrii@chromium.org
BUG=629620
Committed: https://chromium.googlesource.com/infra/infra/+/60670f24bd58b986790ca440761649bb758a6753

[nodir, 2016-07-19 21:05:27]

PTAL

[tandrii(chromium), 2016-07-19 22:41:05]

Thanks, Nodir!

https://codereview.chromium.org/2160913003/diff/1/appengine/cr-buildbucket/pr...
File appengine/cr-buildbucket/proto/project_config.proto (right):

https://codereview.chromium.org/2160913003/diff/1/appengine/cr-buildbucket/pr...
appengine/cr-buildbucket/proto/project_config.proto:66: // same key specified in
common_dimensions.
good! And since proto is changed, how about changing swarmbucket tool's proto in
the same CL + updating testdata/builders.pyl with a case so that the codepath
excluding a value is exercised?

https://codereview.chromium.org/2160913003/diff/1/appengine/cr-buildbucket/sw...
File appengine/cr-buildbucket/swarming/swarmingcfg.py (left):

https://codereview.chromium.org/2160913003/diff/1/appengine/cr-buildbucket/sw...
appengine/cr-buildbucket/swarming/swarmingcfg.py:46: ctx.error('no value')
ok, so validator won't choke any more on empty value.
Am I right that now it's possible delete the non-existent key, say like this:

common_dimensions: "key:value"

builders{
  dimension: "kye:"
}

?

[nodir, 2016-07-22 18:03:37]

https://codereview.chromium.org/2160913003/diff/1/appengine/cr-buildbucket/pr...
File appengine/cr-buildbucket/proto/project_config.proto (right):

https://codereview.chromium.org/2160913003/diff/1/appengine/cr-buildbucket/pr...
appengine/cr-buildbucket/proto/project_config.proto:66: // same key specified in
common_dimensions.
On 2016/07/19 22:41:04, tandrii(chromium) wrote:
> good! And since proto is changed, how about changing swarmbucket tool's proto
in
> the same CL

the tool's proto is a symlink

>  + updating testdata/builders.pyl with a case so that the codepath
> excluding a value is exercised?

added a test

https://codereview.chromium.org/2160913003/diff/1/appengine/cr-buildbucket/sw...
File appengine/cr-buildbucket/swarming/swarmingcfg.py (left):

https://codereview.chromium.org/2160913003/diff/1/appengine/cr-buildbucket/sw...
appengine/cr-buildbucket/swarming/swarmingcfg.py:46: ctx.error('no value')
On 2016/07/19 22:41:04, tandrii(chromium) wrote:
> ok, so validator won't choke any more on empty value.
> Am I right that now it's possible delete the non-existent key, say like this:
> 
> common_dimensions: "key:value"
> 
> builders{
>   dimension: "kye:"

key

> }
> 
> ?


yes

[tandrii(chromium), 2016-07-26 11:13:16]

lgtm

[nodir, 2016-07-26 16:37:18]

The CQ bit was checked by nodir@chromium.org

[commit-bot: I haz the power, 2016-07-26 16:37:20]

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-07-26 16:50:57]

Description was changed from

==========
swarmbucket: fix empty dimension value

Make a dimension with no value valid and meaning that it cancels a value
for this dimension defined at wider scope, e.g. in common_dimension

R=tandrii@chromium.org
BUG=629620
==========

to

==========
swarmbucket: fix empty dimension value

Make a dimension with no value valid and meaning that it cancels a value
for this dimension defined at wider scope, e.g. in common_dimension

R=tandrii@chromium.org
BUG=629620

Committed:
https://chromium.googlesource.com/infra/infra/+/60670f24bd58b986790ca44076164...
==========

[commit-bot: I haz the power, 2016-07-26 16:50:57]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/infra/infra/+/60670f24bd58b986790ca44076164...