Keep track of hashes triggered instead of builds.

Keep track of hashes triggered instead of builds.

BUG=341650, 335363
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=253091

[ghost stip (do not use), 2014-02-20 02:42:19]

ptal

[iannucci, 2014-02-20 03:30:55]

use structures or something. classes are good.

https://codereview.chromium.org/172523005/diff/30001/scripts/slave/gatekeeper...
File scripts/slave/gatekeeper_ng.py (right):

https://codereview.chromium.org/172523005/diff/30001/scripts/slave/gatekeeper...
scripts/slave/gatekeeper_ng.py:73: def find_new_builds(master_url, root_json,
build_db, options):
GREAT EVIL!!!

https://codereview.chromium.org/172523005/diff/30001/scripts/slave/gatekeeper...
scripts/slave/gatekeeper_ng.py:92: b['build'] for b in builds if
b.get('finished'))
lame to iterate over builds twice, but still O(N) :P

https://codereview.chromium.org/172523005/diff/30001/scripts/slave/gatekeeper...
scripts/slave/gatekeeper_ng.py:95: candidate_builds =
set(builder['cachedBuilds'] + builder['currentBuilds'])
comment: cachedBuilds == finishedBuilds

https://codereview.chromium.org/172523005/diff/30001/scripts/slave/gatekeeper...
scripts/slave/gatekeeper_ng.py:103: new_builds[buildername] = [{'build': x} for
x in candidate_builds]
should have structs or namedtuples or classes or something for building up this
db

https://codereview.chromium.org/172523005/diff/30001/scripts/slave/gatekeeper...
scripts/slave/gatekeeper_ng.py:174: for u, m, bd, bl in builds]
zip url_list with the rest of the data so that you don't need this fixup loop...
or just use the build number as the key of the hash.

https://codereview.chromium.org/172523005/diff/30001/scripts/slave/gatekeeper...
scripts/slave/gatekeeper_ng.py:498: lambda x, y: x | set(y.get('triggered',
[])), builds, set([]))
{ t 
  for b in builds
  for t in b.get('triggered', []) 
}

https://codereview.chromium.org/172523005/diff/30001/scripts/slave/gatekeeper...
scripts/slave/gatekeeper_ng.py:503: section_hash not in build_db['sections']):
Use the sets, luke.

https://codereview.chromium.org/172523005/diff/30001/scripts/slave/unittests/...
File scripts/slave/unittests/gatekeeper_ng_test.py (right):

https://codereview.chromium.org/172523005/diff/30001/scripts/slave/unittests/...
scripts/slave/unittests/gatekeeper_ng_test.py:361:
'--process-finished-builds-on-new-builder',
nope

[ghost stip (do not use), 2014-02-22 10:03:06]

ppptttaaalll

https://codereview.chromium.org/172523005/diff/30001/scripts/slave/gatekeeper...
File scripts/slave/gatekeeper_ng.py (right):

https://codereview.chromium.org/172523005/diff/30001/scripts/slave/gatekeeper...
scripts/slave/gatekeeper_ng.py:73: def find_new_builds(master_url, root_json,
build_db, options):
On 2014/02/20 03:30:55, iannucci wrote:
> GREAT EVIL!!!

Done.

https://codereview.chromium.org/172523005/diff/30001/scripts/slave/gatekeeper...
scripts/slave/gatekeeper_ng.py:92: b['build'] for b in builds if
b.get('finished'))
On 2014/02/20 03:30:55, iannucci wrote:
> lame to iterate over builds twice, but still O(N) :P

Done.

https://codereview.chromium.org/172523005/diff/30001/scripts/slave/gatekeeper...
scripts/slave/gatekeeper_ng.py:95: candidate_builds =
set(builder['cachedBuilds'] + builder['currentBuilds'])
On 2014/02/20 03:30:55, iannucci wrote:
> comment: cachedBuilds == finishedBuilds

Done.

https://codereview.chromium.org/172523005/diff/30001/scripts/slave/gatekeeper...
scripts/slave/gatekeeper_ng.py:103: new_builds[buildername] = [{'build': x} for
x in candidate_builds]
On 2014/02/20 03:30:55, iannucci wrote:
> should have structs or namedtuples or classes or something for building up
this
> db

Done.

https://codereview.chromium.org/172523005/diff/30001/scripts/slave/gatekeeper...
scripts/slave/gatekeeper_ng.py:174: for u, m, bd, bl in builds]
On 2014/02/20 03:30:55, iannucci wrote:
> zip url_list with the rest of the data so that you don't need this fixup
loop...
> or just use the build number as the key of the hash.

Done.

https://codereview.chromium.org/172523005/diff/30001/scripts/slave/gatekeeper...
scripts/slave/gatekeeper_ng.py:498: lambda x, y: x | set(y.get('triggered',
[])), builds, set([]))
On 2014/02/20 03:30:55, iannucci wrote:
> { t 
>   for b in builds
>   for t in b.get('triggered', []) 
> }

Done.

https://codereview.chromium.org/172523005/diff/30001/scripts/slave/gatekeeper...
scripts/slave/gatekeeper_ng.py:503: section_hash not in build_db['sections']):
On 2014/02/20 03:30:55, iannucci wrote:
> Use the sets, luke.

dude you just unlocked an entire set of lgtm sagas for luqui@ (see
https://chromiumcodereview.appspot.com/71553003/#msg11)

https://codereview.chromium.org/172523005/diff/30001/scripts/slave/unittests/...
File scripts/slave/unittests/gatekeeper_ng_test.py (right):

https://codereview.chromium.org/172523005/diff/30001/scripts/slave/unittests/...
scripts/slave/unittests/gatekeeper_ng_test.py:361:
'--process-finished-builds-on-new-builder',
On 2014/02/20 03:30:55, iannucci wrote:
> nope

Done.

https://codereview.chromium.org/172523005/diff/80001/scripts/slave/gatekeeper...
File scripts/slave/gatekeeper_ng.py (right):

https://codereview.chromium.org/172523005/diff/80001/scripts/slave/gatekeeper...
scripts/slave/gatekeeper_ng.py:553: if options.set_status:
moved from line 549

https://codereview.chromium.org/172523005/diff/80001/scripts/slave/gatekeeper...
File scripts/slave/gatekeeper_ng_db.py (right):

https://codereview.chromium.org/172523005/diff/80001/scripts/slave/gatekeeper...
scripts/slave/gatekeeper_ng_db.py:70: def gen_db(**kwargs):
hm, might be able to merge this into __init__

https://codereview.chromium.org/172523005/diff/80001/scripts/slave/unittests/...
File scripts/slave/unittests/gatekeeper_ng_test.py (left):

https://codereview.chromium.org/172523005/diff/80001/scripts/slave/unittests/...
scripts/slave/unittests/gatekeeper_ng_test.py:330: def testBasicURLQuery(self):
this check didn't do much and got in the way of moving --json into
call_gatekeeper

https://codereview.chromium.org/172523005/diff/80001/scripts/slave/unittests/...
scripts/slave/unittests/gatekeeper_ng_test.py:355: def
testIgnoreSuccessfulBuildNoGatekeeperSteps(self):
this check didn't do much and got in the way of moving --json into
call_gatekeeper

[iannucci, 2014-02-24 20:20:23]

looks a lot better. lgtm + other comments

https://codereview.chromium.org/172523005/diff/190001/scripts/slave/gatekeepe...
File scripts/slave/gatekeeper_ng.py (right):

https://codereview.chromium.org/172523005/diff/190001/scripts/slave/gatekeepe...
scripts/slave/gatekeeper_ng.py:164: build =
build_db.masters[master].setdefault(builder, {}).get(buildnum)
TBH, this should be done with defaultdict's in your db file.

https://codereview.chromium.org/172523005/diff/190001/scripts/slave/gatekeepe...
scripts/slave/gatekeeper_ng.py:253: def debounce_failures(failed_builds,
build_db):
"debounce"  hardware nerd :P

https://codereview.chromium.org/172523005/diff/190001/scripts/slave/gatekeepe...
File scripts/slave/gatekeeper_ng_db.py (right):

https://codereview.chromium.org/172523005/diff/190001/scripts/slave/gatekeepe...
scripts/slave/gatekeeper_ng_db.py:31: 'masters',
Add comment after these like

'masters'  # {<mastername>: {<buildername>: {<buildnumber>: BuildDBBuild}}}

https://codereview.chromium.org/172523005/diff/190001/scripts/slave/gatekeepe...
scripts/slave/gatekeeper_ng_db.py:50: self._asdict().iteritems())
Probably shorter as a list comp:

[(k, v) for k, v in self._asdict().iteritems() if hasattr(v, 'asJson')]

https://codereview.chromium.org/172523005/diff/190001/scripts/slave/gatekeepe...
scripts/slave/gatekeeper_ng_db.py:52: self._asdict().iteritems())
Though, you could also combine these to by just doing one loop over
self._asdict()

https://codereview.chromium.org/172523005/diff/190001/scripts/slave/gatekeepe...
scripts/slave/gatekeeper_ng_db.py:98: if json_build_db.get('build_db_version',
0) != BUILD_DB_VERSION:
could just let this default to None I think?

https://codereview.chromium.org/172523005/diff/190001/scripts/slave/gatekeepe...
scripts/slave/gatekeeper_ng_db.py:99: raise BadConf('file is an older db
version: %d (expecting %d)' % (
%r would correctly format anything (int, str, None)

https://codereview.chromium.org/172523005/diff/190001/scripts/slave/gatekeepe...
scripts/slave/gatekeeper_ng_db.py:148: filter(lambda x: x[1].finished,
builders[builder].iteritems()))
ok, seriously, you should really use list comps :p

[ghost stip (do not use), 2014-02-25 07:12:39]

https://codereview.chromium.org/172523005/diff/190001/scripts/slave/gatekeepe...
File scripts/slave/gatekeeper_ng_db.py (right):

https://codereview.chromium.org/172523005/diff/190001/scripts/slave/gatekeepe...
scripts/slave/gatekeeper_ng_db.py:31: 'masters',
On 2014/02/24 20:20:23, iannucci wrote:
> Add comment after these like
> 
> 'masters'  # {<mastername>: {<buildername>: {<buildnumber>: BuildDBBuild}}} 

Done.

https://codereview.chromium.org/172523005/diff/190001/scripts/slave/gatekeepe...
scripts/slave/gatekeeper_ng_db.py:50: self._asdict().iteritems())
On 2014/02/24 20:20:23, iannucci wrote:
> Probably shorter as a list comp:
> 
> [(k, v) for k, v in self._asdict().iteritems() if hasattr(v, 'asJson')]

Done.

https://codereview.chromium.org/172523005/diff/190001/scripts/slave/gatekeepe...
scripts/slave/gatekeeper_ng_db.py:98: if json_build_db.get('build_db_version',
0) != BUILD_DB_VERSION:
On 2014/02/24 20:20:23, iannucci wrote:
> could just let this default to None I think?

Done.

https://codereview.chromium.org/172523005/diff/190001/scripts/slave/gatekeepe...
scripts/slave/gatekeeper_ng_db.py:99: raise BadConf('file is an older db
version: %d (expecting %d)' % (
On 2014/02/24 20:20:23, iannucci wrote:
> %r would correctly format anything (int, str, None)

Done.

https://codereview.chromium.org/172523005/diff/190001/scripts/slave/gatekeepe...
scripts/slave/gatekeeper_ng_db.py:148: filter(lambda x: x[1].finished,
builders[builder].iteritems()))
On 2014/02/24 20:20:23, iannucci wrote:
> ok, seriously, you should really use list comps :p

laaaaame

[ghost stip (do not use), 2014-02-25 07:13:03]

The CQ bit was checked by stip@chromium.org

[commit-bot: I haz the power, 2014-02-25 07:13:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stip@chromium.org/172523005/270001

[commit-bot: I haz the power, 2014-02-25 07:13:57]

Change committed as 253091