Build delegation for perf tryjobs

This CL attempts to implement build delegation for perf tryjobs.
Currently, perf try jobs build on the same builder that runs the test.
This is bad because often we want much slower performance characteristics on 
our perfbots than on our builders, and this makes the builds very slow or even impossible. 

BUG= http://crbug.com/601699

Committed: https://chromium.googlesource.com/chromium/tools/build/+/b89e76b0fd8bebe0ccd45aabf54dca49b3cf26b9

[Ziqi Xiong, 2016-06-13 23:57:36]

zxiong@google.com changed reviewers:
+ prasadv@chromium.org, robertocn@chromium.org, robertocn@google.com

[Ziqi Xiong, 2016-06-13 23:57:37]

Tentative attempt to implement perf try build delegation

[RobertoCN, 2016-06-14 18:37:49]

By running the recipe simulation I found the causes of some of the exceptions.
See the comments below. Also, here are the changes I made locally to address
them: https://paste.googleplex.com/4716501122678784

I recommend you continue this process.

https://codereview.chromium.org/2061893003/diff/1/scripts/slave/recipe_module...
File scripts/slave/recipe_modules/perf_try/build_state.py (right):

https://codereview.chromium.org/2061893003/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/perf_try/build_state.py:20: """
This string should be a comment instead, same for below.

https://codereview.chromium.org/2061893003/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/perf_try/build_state.py:33: if 'mac' in bot_name:
What about Linux?

https://codereview.chromium.org/2061893003/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/perf_try/build_state.py:34: return
'gs://chrome-perf/Mac Builder/full-build-mac_'
There is no default return. In the case where buildername is empty this function
doesn't return anything, and that breaks other functionality that expects to be
able to concatenate the result of this func with another string.

https://codereview.chromium.org/2061893003/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/perf_try/build_state.py:54: api = self.bisector.api
bisector doesn't exist in this scope (or in this module at all)

https://codereview.chromium.org/2061893003/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/perf_try/build_state.py:76: if self.build_number is
None:
This attribute needs to be initialized to None in the ctor.

https://codereview.chromium.org/2061893003/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/perf_try/build_state.py:100: return
current_build.get('results') in [FAILURE, SKIPPED, EXCEPTION]
These constants are not defined

[prasadv, 2016-06-15 17:55:14]

https://codereview.chromium.org/2061893003/diff/20001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/api.py (right):

https://codereview.chromium.org/2061893003/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:247:
self.m.buildbucket.put([build_details],
I think we also should capture buildbucket id, so that we can check the build
status while waiting for it.

https://codereview.chromium.org/2061893003/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:265: #Duplicate code from
auto_bisect.api.start_test_run_for_bisect
space after #, do this where ever applicable

https://codereview.chromium.org/2061893003/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:274: # if not run_locally:
If these lines are not needed, then we can remove them.

https://codereview.chromium.org/2061893003/diff/20001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/build_state.py (right):

https://codereview.chromium.org/2061893003/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/build_state.py:24: def
_get_platform_gs_prefix(self):
I don't think we should store these perf tryjob builds here because the archives
built by perf tryjobs are for uncommitted codes.

Possible approaches:
1. Create a separate bucket to archive perf try job builds with and without
patch.

or
2. Only archive builds with patches in separate bucket, and use chrome-per
bucket for builds without patches. (before posting a build request for revision
without patch, we should check if the build is already cached if so use that
otherwise post the build request)

https://codereview.chromium.org/2061893003/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/build_state.py:65: def
is_build_failed(self):
I think we should use buildbucket apis to determine the status of builds.

[Ziqi Xiong, 2016-06-15 23:01:25]

Description was changed from

==========
Currently, perf try jobs build on the same builder that runs the test. This is
bad because often we want much slower performance characteristics on our
perfbots than on our builders, and this makes the builds very slow or even
impossible. This CL attempts to implement build delegation for perf tryjobs
BUG= http://crbug.com/601699
==========

to

==========
Currently, perf try jobs build on the same builder that runs the test. This is
bad because often we want much slower performance characteristics on our
perfbots than on our builders, and this makes the builds very slow or even
impossible. This CL attempts to implement build delegation for perf tryjobs
BUG= http://crbug.com/601699
==========

[Ziqi Xiong, 2016-06-16 21:58:09]

Description was changed from

==========
Currently, perf try jobs build on the same builder that runs the test. This is
bad because often we want much slower performance characteristics on our
perfbots than on our builders, and this makes the builds very slow or even
impossible. This CL attempts to implement build delegation for perf tryjobs
BUG= http://crbug.com/601699
==========

to

==========
This CL attempts to implement build delegation for perf tryjobs.
Currently, perf try jobs build on the same builder that runs the test.
This is bad because often we want much slower performance characteristics on our
perfbots than on our builders, 
and this makes the builds very slow or even impossible. 

BUG= http://crbug.com/601699
==========

[Ziqi Xiong, 2016-06-16 21:58:31]

Description was changed from

==========
This CL attempts to implement build delegation for perf tryjobs.
Currently, perf try jobs build on the same builder that runs the test.
This is bad because often we want much slower performance characteristics on our
perfbots than on our builders, 
and this makes the builds very slow or even impossible. 

BUG= http://crbug.com/601699
==========

to

==========
This CL attempts to implement build delegation for perf tryjobs.
Currently, perf try jobs build on the same builder that runs the test.
This is bad because often we want much slower performance characteristics on 
our perfbots than on our builders, and this makes the builds very slow or even
impossible. 

BUG= http://crbug.com/601699
==========

[Ziqi Xiong, 2016-06-17 18:41:31]

Thanks for your reviews! Sorry I just figured out that I need to publish the
drafts for email notification.

https://codereview.chromium.org/2061893003/diff/1/scripts/slave/recipe_module...
File scripts/slave/recipe_modules/perf_try/build_state.py (right):

https://codereview.chromium.org/2061893003/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/perf_try/build_state.py:20: """
On 2016/06/14 18:37:49, RobertoCN wrote:
> This string should be a comment instead, same for below.

Fixed in patchset: 20001

https://codereview.chromium.org/2061893003/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/perf_try/build_state.py:33: if 'mac' in bot_name:
On 2016/06/14 18:37:48, RobertoCN wrote:
> What about Linux?

Fixed in patchset: 20001

https://codereview.chromium.org/2061893003/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/perf_try/build_state.py:34: return
'gs://chrome-perf/Mac Builder/full-build-mac_'
On 2016/06/14 18:37:49, RobertoCN wrote:
> There is no default return. In the case where buildername is empty this
function
> doesn't return anything, and that breaks other functionality that expects to
be
> able to concatenate the result of this func with another string.

Fixed in patchset: 20001

https://codereview.chromium.org/2061893003/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/perf_try/build_state.py:54: api = self.bisector.api
On 2016/06/14 18:37:48, RobertoCN wrote:
> bisector doesn't exist in this scope (or in this module at all)

Fixed in patchset: 20001

https://codereview.chromium.org/2061893003/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/perf_try/build_state.py:76: if self.build_number is
None:
On 2016/06/14 18:37:49, RobertoCN wrote:
> This attribute needs to be initialized to None in the ctor.

Fixed in patchset: 20001

https://codereview.chromium.org/2061893003/diff/1/scripts/slave/recipe_module...
scripts/slave/recipe_modules/perf_try/build_state.py:100: return
current_build.get('results') in [FAILURE, SKIPPED, EXCEPTION]
On 2016/06/14 18:37:48, RobertoCN wrote:
> These constants are not defined
Fixed in patchset: 20001

https://codereview.chromium.org/2061893003/diff/20001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/api.py (right):

https://codereview.chromium.org/2061893003/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:247:
self.m.buildbucket.put([build_details],
On 2016/06/15 17:55:14, prasadv wrote:
> I think we also should capture buildbucket id, so that we can check the build
> status while waiting for it.

Done.

https://codereview.chromium.org/2061893003/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:265: #Duplicate code from
auto_bisect.api.start_test_run_for_bisect
On 2016/06/15 17:55:14, prasadv wrote:
> space after #, do this where ever applicable

Done.

https://codereview.chromium.org/2061893003/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:274: # if not run_locally:
On 2016/06/15 17:55:13, prasadv wrote:
> If these lines are not needed, then we can remove them.

Done.

https://codereview.chromium.org/2061893003/diff/20001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/build_state.py (right):

https://codereview.chromium.org/2061893003/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/build_state.py:65: def
is_build_failed(self):
On 2016/06/15 17:55:14, prasadv wrote:
> I think we should use buildbucket apis to determine the status of builds.

Done.

[prasadv, 2016-06-20 18:31:47]

I was expecting changes to expectation files here,
https://cs.chromium.org/chromium/build/scripts/slave/recipes/bisection/deskto...
https://cs.chromium.org/chromium/build/scripts/slave/recipes/bisection/androi...

Did you run /scripts/slave/unittests/recipe_simulation_test.py to generate
expectation file.

https://codereview.chromium.org/2061893003/diff/20001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/build_state.py (right):

https://codereview.chromium.org/2061893003/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/build_state.py:65: def
is_build_failed(self):
Can you please check the feasibility of using buildcuket recipe module to check
the status of the build.
tryjob should bailout quickly in case the requested build is failed.

https://codereview.chromium.org/2061893003/diff/40001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/api.py (right):

https://codereview.chromium.org/2061893003/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:223: self._request_build(revision)
When this method is called for revision without a patch, we should check for
existence of binary for the revision in chrome-perf, if not request a build by
setting the revision.build_file_path pointing to chrome-perf.
Note: This is applicale only for WITHOUT patch.

https://codereview.chromium.org/2061893003/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:248:
self.m.service_account.get_json_path\
4 space indentation, and also per python coding styles, avoid '\' to continue
line.

Recommended: 
result = self.m.buildbucket.put(
    [build_details],
    self.m.service_account.get_json_path(SERVICE_ACCOUNT))

https://codereview.chromium.org/2061893003/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:250: revision.build_id =
result.stdout['results'][0]['build']['id']
Is this the buildbucket id? where is this used?

https://codereview.chromium.org/2061893003/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:293: gs_bucket = 'gs://%s/' %
bot_config['bucket']
The bucket in bot_config is set to chrome-perf.
https://cs.chromium.org/chromium/build/scripts/slave/recipes/bisection/androi...

We should make sure we are downloading archive from proper location.
Both for with and without patch.

https://codereview.chromium.org/2061893003/diff/40001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/build_state.py (right):

https://codereview.chromium.org/2061893003/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/build_state.py:24: def
_get_platform_gs_prefix(self):
I think we can add bucket as argument to this method.

[prasadv, 2016-06-21 16:52:56]

https://codereview.chromium.org/2061893003/diff/40001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/api.py (right):

https://codereview.chromium.org/2061893003/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:255: if revision.is_completed():
I think we should also check results of buildbucket job along with status. 
https://docs.google.com/document/d/10BH6oSq_UEtdSdhrguEuV4hx-JxY7jzFcgs1OCHcn...

Because if the build fails on the builder for some reason, we should not
continue perf tryjob instead display a meaningful information to the user.

[Ziqi Xiong, 2016-06-21 22:21:27]

Make sure the builds with and without patches are stored in the right locations;
Add expected files.

https://codereview.chromium.org/2061893003/diff/20001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/build_state.py (right):

https://codereview.chromium.org/2061893003/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/build_state.py:24: def
_get_platform_gs_prefix(self):
On 2016/06/15 17:55:14, prasadv wrote:
> I don't think we should store these perf tryjob builds here because the
archives
> built by perf tryjobs are for uncommitted codes.
> 
> Possible approaches:
> 1. Create a separate bucket to archive perf try job builds with and without
> patch.
> 
> or
> 2. Only archive builds with patches in separate bucket, and use chrome-per
> bucket for builds without patches. (before posting a build request for
revision
> without patch, we should check if the build is already cached if so use that
> otherwise post the build request)
> 

Done.

https://codereview.chromium.org/2061893003/diff/20001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/build_state.py:65: def
is_build_failed(self):
On 2016/06/20 18:31:47, prasadv wrote:
> Can you please check the feasibility of using buildcuket recipe module to
check
> the status of the build.
> tryjob should bailout quickly in case the requested build is failed.

Done.

https://codereview.chromium.org/2061893003/diff/40001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/api.py (right):

https://codereview.chromium.org/2061893003/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:223: self._request_build(revision)
On 2016/06/20 18:31:47, prasadv wrote:
> When this method is called for revision without a patch, we should check for
> existence of binary for the revision in chrome-perf, if not request a build by
> setting the revision.build_file_path pointing to chrome-perf.
> Note: This is applicale only for WITHOUT patch.

Done.

https://codereview.chromium.org/2061893003/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:248:
self.m.service_account.get_json_path\
On 2016/06/20 18:31:47, prasadv wrote:
> 4 space indentation, and also per python coding styles, avoid '\' to continue
> line.
> 
> Recommended: 
> result = self.m.buildbucket.put(
>     [build_details],
>     self.m.service_account.get_json_path(SERVICE_ACCOUNT))

Done.

https://codereview.chromium.org/2061893003/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:250: revision.build_id =
result.stdout['results'][0]['build']['id']
On 2016/06/20 18:31:47, prasadv wrote:
> Is this the buildbucket id? where is this used?

Done.

https://codereview.chromium.org/2061893003/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:255: if revision.is_completed():
On 2016/06/21 16:52:56, prasadv wrote:
> I think we should also check results of buildbucket job along with status. 
>
https://docs.google.com/document/d/10BH6oSq_UEtdSdhrguEuV4hx-JxY7jzFcgs1OCHcn...
> 
> Because if the build fails on the builder for some reason, we should not
> continue perf tryjob instead display a meaningful information to the user.

If the build fails, then the status should be 'completed' and it won't continue?

https://codereview.chromium.org/2061893003/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:293: gs_bucket = 'gs://%s/' %
bot_config['bucket']
On 2016/06/20 18:31:47, prasadv wrote:
> The bucket in bot_config is set to chrome-perf.
>
https://cs.chromium.org/chromium/build/scripts/slave/recipes/bisection/androi...
> 
> We should make sure we are downloading archive from proper location.
> Both for with and without patch.

Done.

https://codereview.chromium.org/2061893003/diff/40001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/build_state.py (right):

https://codereview.chromium.org/2061893003/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/build_state.py:24: def
_get_platform_gs_prefix(self):
On 2016/06/20 18:31:47, prasadv wrote:
> I think we can add bucket as argument to this method.

Done.

[prasadv, 2016-06-21 22:32:56]

Current expectation files are generated only for android. You should also modify
desktop_bisect.py recipe inorder to generate expectation files for desktop try
jobs.

https://codereview.chromium.org/2061893003/diff/60001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/api.py (right):

https://codereview.chromium.org/2061893003/diff/60001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:226:
self._download_build(update_step, bot_db, revision)
Also in the test expectation files, I don't see lines 226-230 being executed.

https://codereview.chromium.org/2061893003/diff/60001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:240: 'parent_got_revision':
revision.commit_hash,
How the builder know what to patch. Since we are just passing the revision hash
and there is no way the builder can build the revision with patch.

[RobertoCN, 2016-06-21 22:39:43]

Here are some comments. It seems we are not going all the way in the simulation.

https://codereview.chromium.org/2061893003/diff/60001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/api.py (right):

https://codereview.chromium.org/2061893003/diff/60001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:226:
self._download_build(update_step, bot_db, revision)
It doesn't seem from the expectations that we are getting to this point.

https://codereview.chromium.org/2061893003/diff/60001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:272: if not
revision.build_archived:
This should be set by build_state.is_build_archived(), but I don't see where
this is called.

https://codereview.chromium.org/2061893003/diff/60001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/build_state.py (right):

https://codereview.chromium.org/2061893003/diff/60001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/build_state.py:47: def
is_build_archived(self):
Maybe merge is_build_archived and is_completed?

https://codereview.chromium.org/2061893003/diff/60001/scripts/slave/recipes/b...
File
scripts/slave/recipes/bisection/android_bisect.expected/basic_perf_tryjob_android_fyi_perf_bisect.json
(left):

https://codereview.chromium.org/2061893003/diff/60001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/android_bisect.expected/basic_perf_tryjob_android_fyi_perf_bisect.json:754:
},
up until here the diff seems to make sense, the steps following this line it
seems to me shouldn't have been removed. (After the build is complete, we should
download it unzip, install apk, test, etc..)

[prasadv, 2016-06-21 23:11:04]

https://codereview.chromium.org/2061893003/diff/60001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/api.py (right):

https://codereview.chromium.org/2061893003/diff/60001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:272: if not
revision.build_archived:
This is the reason why we are not able to get other steps in simulation.

[RobertoCN, 2016-06-22 19:01:34]

https://codereview.chromium.org/2061893003/diff/60001/scripts/slave/recipes/b...
File scripts/slave/recipes/bisection/android_bisect.py (left):

https://codereview.chromium.org/2061893003/diff/60001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/android_bisect.py:159: buildername=buildername)
+ api.override_step_data(
patch_storage='rietveld', patchset='20001', issue='12345'

[Ziqi Xiong, 2016-06-22 21:41:26]

Modified desktop_bisect and android_bisect files and reached more steps in
simulation.

https://codereview.chromium.org/2061893003/diff/60001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/api.py (right):

https://codereview.chromium.org/2061893003/diff/60001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:240: 'parent_got_revision':
revision.commit_hash,
On 2016/06/21 22:32:56, prasadv wrote:
> How the builder know what to patch. Since we are just passing the revision
hash
> and there is no way the builder can build the revision with patch.

Done.

https://codereview.chromium.org/2061893003/diff/60001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:272: if not
revision.build_archived:
On 2016/06/21 22:39:43, RobertoCN wrote:
> This should be set by build_state.is_build_archived(), but I don't see where
> this is called.

Done.

https://codereview.chromium.org/2061893003/diff/60001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:272: if not
revision.build_archived:
On 2016/06/21 23:11:04, prasadv wrote:
> This is the reason why we are not able to get other steps in simulation.

Done.

https://codereview.chromium.org/2061893003/diff/60001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/build_state.py (right):

https://codereview.chromium.org/2061893003/diff/60001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/build_state.py:47: def
is_build_archived(self):
On 2016/06/21 22:39:43, RobertoCN wrote:
> Maybe merge is_build_archived and is_completed?

I am not exactly sure why they should be merged. is_completed checks status
while is_build_archived checks result. They are also used differently. The
former is called in _wait_for, the latter in _download_build

[prasadv, 2016-06-22 21:56:23]

https://codereview.chromium.org/2061893003/diff/70001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/api.py (right):

https://codereview.chromium.org/2061893003/diff/70001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:225: self._wait_for(revision)
Here we should capture if any error occurred while building the revision and
same should be displayed in the form of a step.

https://codereview.chromium.org/2061893003/diff/70001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:247: 'rietveld':
self.m.properties['rietveld']
rietveld url is missing.

https://codereview.chromium.org/2061893003/diff/70001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:250: properties = {
Check the build archive before posting build request.

https://codereview.chromium.org/2061893003/diff/70001/scripts/slave/recipes/b...
File
scripts/slave/recipes/bisection/desktop_bisect.expected/basic_perf_tryjob.json
(right):

https://codereview.chromium.org/2061893003/diff/70001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/desktop_bisect.expected/basic_perf_tryjob.json:238:
"{\"bucket\": \"master.tryserver.chromium.perf\", \"client_operation_id\":
\"57b122e4ab4c4b32865a0a441e4badd0\", \"parameters_json\":
\"{\\\"buildername\\\": \\\"linux_perf_bisect_builder\\\", \\\"properties\\\":
{\\\"build_archive_url\\\": \\\"gs://chrome-perf-tryjob/Linux
Builder/full-build-linuxNone.zip\\\", \\\"clobber\\\": true, \\\"issue\\\":
\\\"12345\\\", \\\"parent_got_revision\\\": \\\"None\\\", \\\"patch_storage\\\":
\\\"rietveld\\\", \\\"patchset\\\": \\\"20001\\\", \\\"rietveld\\\": true}}\",
\"tags\": [\"master:tryserver.chromium.perf\", \"user_agent:recipe\"]}"
" \\\"gs://chrome-perf-tryjob/Linux Builder/full-build-linuxNone.zip"
We should generate a git_hash for the patch we are testing and archive with
unique hash.

https://codereview.chromium.org/2061893003/diff/70001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/desktop_bisect.expected/basic_perf_tryjob.json:488:
"{\"bucket\": \"master.tryserver.chromium.perf\", \"client_operation_id\":
\"a3bc63df0f75472793774f2f9b555b8d\", \"parameters_json\":
\"{\\\"buildername\\\": \\\"linux_perf_bisect_builder\\\", \\\"properties\\\":
{\\\"build_archive_url\\\": \\\"gs://chrome-perf/Linux
Builder/full-build-linuxNone.zip\\\", \\\"clobber\\\": true,
\\\"parent_got_revision\\\": \\\"None\\\"}}\", \"tags\":
[\"master:tryserver.chromium.perf\", \"user_agent:recipe\"]}"
"\\\"gs://chrome-perf/Linux Builder/full-build-linuxNone.zip"
zip file should be prefixed with the revision under test.

https://codereview.chromium.org/2061893003/diff/70001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/desktop_bisect.expected/basic_perf_tryjob.json:490:
"name": "buildbucket.put (2)",
I think we before posting build request, we should check if the build exists in
chrome-perf when processing build without patch.

https://codereview.chromium.org/2061893003/diff/70001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/desktop_bisect.expected/basic_perf_tryjob.json:619:
"name": "Post bisect results",
This step is not required. we don't post this for perf try jobs

[Ziqi Xiong, 2016-06-23 20:58:58]

Fix the errors found by Prasad

https://codereview.chromium.org/2061893003/diff/70001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/api.py (right):

https://codereview.chromium.org/2061893003/diff/70001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:225: self._wait_for(revision)
On 2016/06/22 21:56:23, prasadv wrote:
> Here we should capture if any error occurred while building the revision and
> same should be displayed in the form of a step.

Done.

https://codereview.chromium.org/2061893003/diff/70001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:247: 'rietveld':
self.m.properties['rietveld']
On 2016/06/22 21:56:23, prasadv wrote:
> rietveld url is missing.

Done.

https://codereview.chromium.org/2061893003/diff/70001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:250: properties = {
On 2016/06/22 21:56:23, prasadv wrote:
> Check the build archive before posting build request.

Done.

https://codereview.chromium.org/2061893003/diff/70001/scripts/slave/recipes/b...
File
scripts/slave/recipes/bisection/desktop_bisect.expected/basic_perf_tryjob.json
(right):

https://codereview.chromium.org/2061893003/diff/70001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/desktop_bisect.expected/basic_perf_tryjob.json:238:
"{\"bucket\": \"master.tryserver.chromium.perf\", \"client_operation_id\":
\"57b122e4ab4c4b32865a0a441e4badd0\", \"parameters_json\":
\"{\\\"buildername\\\": \\\"linux_perf_bisect_builder\\\", \\\"properties\\\":
{\\\"build_archive_url\\\": \\\"gs://chrome-perf-tryjob/Linux
Builder/full-build-linuxNone.zip\\\", \\\"clobber\\\": true, \\\"issue\\\":
\\\"12345\\\", \\\"parent_got_revision\\\": \\\"None\\\", \\\"patch_storage\\\":
\\\"rietveld\\\", \\\"patchset\\\": \\\"20001\\\", \\\"rietveld\\\": true}}\",
\"tags\": [\"master:tryserver.chromium.perf\", \"user_agent:recipe\"]}"
On 2016/06/22 21:56:23, prasadv wrote:
> " \\\"gs://chrome-perf-tryjob/Linux Builder/full-build-linuxNone.zip"
> We should generate a git_hash for the patch we are testing and archive with
> unique hash.

Done.

https://codereview.chromium.org/2061893003/diff/70001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/desktop_bisect.expected/basic_perf_tryjob.json:488:
"{\"bucket\": \"master.tryserver.chromium.perf\", \"client_operation_id\":
\"a3bc63df0f75472793774f2f9b555b8d\", \"parameters_json\":
\"{\\\"buildername\\\": \\\"linux_perf_bisect_builder\\\", \\\"properties\\\":
{\\\"build_archive_url\\\": \\\"gs://chrome-perf/Linux
Builder/full-build-linuxNone.zip\\\", \\\"clobber\\\": true,
\\\"parent_got_revision\\\": \\\"None\\\"}}\", \"tags\":
[\"master:tryserver.chromium.perf\", \"user_agent:recipe\"]}"
On 2016/06/22 21:56:23, prasadv wrote:
> "\\\"gs://chrome-perf/Linux Builder/full-build-linuxNone.zip"
> zip file should be prefixed with the revision under test.

Done.

https://codereview.chromium.org/2061893003/diff/70001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/desktop_bisect.expected/basic_perf_tryjob.json:490:
"name": "buildbucket.put (2)",
On 2016/06/22 21:56:23, prasadv wrote:
> I think we before posting build request, we should check if the build exists
in
> chrome-perf when processing build without patch.

Done.

[RobertoCN, 2016-06-24 18:46:37]

Please take a look at the comments I made.

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium_tests/api.py (right):

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_tests/api.py:508: 'parent_got_revision')
or update_step.presentation.properties[
This change is ok, but doesn't belong in this cl.

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/api.py (right):

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:225: revision =
build_state.BuildState(self, revision, with_patch)
consider renaming the variables. is revision a hash string, or a build_state
object? this is a bit hard to follow.

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/build_state.py (right):

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/build_state.py:1: """
Nit: add copy right header.

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/build_state.py:2: Most code in this class
are copied or adapted from revision_state in
This docstring looks more like a comment.

Replace the docstring with a brief desciption of what this does and why it's
needed.
Then add a comment saying that the code was adapted from somewhere else.

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipes/b...
File
scripts/slave/recipes/bisection/android_bisect.expected/perf_tryjob_failed_test_android_s5_perf_cq.json
(right):

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/android_bisect.expected/perf_tryjob_failed_test_android_s5_perf_cq.json:838:
"reason": "Error occurs when building None: 'NoneType' object has no attribute
'__getitem__'",
This seems fishy, any idea where this may come from?

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipes/b...
File
scripts/slave/recipes/bisection/desktop_bisect.expected/basic_perf_tryjob.json
(left):

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/desktop_bisect.expected/basic_perf_tryjob.json:570:
"@@@STEP_LOG_LINE@Captured Output@*RESULT dummy: dummy=
[5.83,6.013,5.573]ms@@@",
Do you know why these are missing after training?

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/desktop_bisect.expected/basic_perf_tryjob.json:583:
"~followup_annotations": [
Same as above.

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipes/b...
File scripts/slave/recipes/bisection/desktop_bisect.py (right):

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/desktop_bisect.py:183: + api.step_data('gsutil
check.file.exist', retcode=1)
Consider changing the label of the step to a format more consistent with
existing steps.

[Ziqi Xiong, 2016-06-27 18:47:55]

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium_tests/api.py (right):

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_tests/api.py:508: 'parent_got_revision')
or update_step.presentation.properties[
On 2016/06/24 18:46:37, RobertoCN wrote:
> This change is ok, but doesn't belong in this cl.

Acknowledged.

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/api.py (right):

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/api.py:225: revision =
build_state.BuildState(self, revision, with_patch)
On 2016/06/24 18:46:37, RobertoCN wrote:
> consider renaming the variables. is revision a hash string, or a build_state
> object? this is a bit hard to follow.

Done.

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/perf_try/build_state.py (right):

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/build_state.py:1: """
On 2016/06/24 18:46:37, RobertoCN wrote:
> Nit: add copy right header.

Done.

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/perf_try/build_state.py:2: Most code in this class
are copied or adapted from revision_state in
On 2016/06/24 18:46:37, RobertoCN wrote:
> This docstring looks more like a comment.
> 
> Replace the docstring with a brief desciption of what this does and why it's
> needed.
> Then add a comment saying that the code was adapted from somewhere else.

Done.

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipes/b...
File
scripts/slave/recipes/bisection/android_bisect.expected/perf_tryjob_failed_test_android_s5_perf_cq.json
(right):

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/android_bisect.expected/perf_tryjob_failed_test_android_s5_perf_cq.json:838:
"reason": "Error occurs when building None: 'NoneType' object has no attribute
'__getitem__'",
On 2016/06/24 18:46:37, RobertoCN wrote:
> This seems fishy, any idea where this may come from?
The error message is generated by the try-catch block in _build_and_run_tests
method.

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipes/b...
File
scripts/slave/recipes/bisection/desktop_bisect.expected/basic_perf_tryjob.json
(left):

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/desktop_bisect.expected/basic_perf_tryjob.json:570:
"@@@STEP_LOG_LINE@Captured Output@*RESULT dummy: dummy=
[5.83,6.013,5.573]ms@@@",
On 2016/06/24 18:46:37, RobertoCN wrote:
> Do you know why these are missing after training?
Done.

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/desktop_bisect.expected/basic_perf_tryjob.json:583:
"~followup_annotations": [
On 2016/06/24 18:46:37, RobertoCN wrote:
> Same as above.

Done.

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipes/b...
File scripts/slave/recipes/bisection/desktop_bisect.py (right):

https://codereview.chromium.org/2061893003/diff/80001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/desktop_bisect.py:183: + api.step_data('gsutil
check.file.exist', retcode=1)
On 2016/06/24 18:46:37, RobertoCN wrote:
> Consider changing the label of the step to a format more consistent with
> existing steps.

I changed it to 'gsutil exists'. Does that sound better?

[RobertoCN, 2016-06-27 19:36:22]

LGTM with some style fixes. Please address them before committing  this patch.

https://codereview.chromium.org/2061893003/diff/90001/scripts/slave/recipes/b...
File scripts/slave/recipes/bisection/android_bisect.py (right):

https://codereview.chromium.org/2061893003/diff/90001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/android_bisect.py:242:
mastername='tryserver.chromium.perf',
Nit: Alignment

https://codereview.chromium.org/2061893003/diff/90001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/android_bisect.py:249:
api.raw_io.stream_output('tools/run-perf-test.cfg')) +
Nit: let's make this consistent. Either plus at the beginning of the line
(preferred as per styleguide) or always at the end (some other files in recipes
use this style).

https://codereview.chromium.org/2061893003/diff/90001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/android_bisect.py:265: buildername=buildername,
alignment

https://codereview.chromium.org/2061893003/diff/90001/scripts/slave/recipes/b...
File scripts/slave/recipes/bisection/desktop_bisect.py (right):

https://codereview.chromium.org/2061893003/diff/90001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/desktop_bisect.py:142:
api.step_data('buildbucket.put (2)',
Nit: let's cleanup the alignment and operator placement (at the end of breaking
line, or at the beginning of continuation) for this whole file.

[Ziqi Xiong, 2016-06-27 20:40:35]

https://codereview.chromium.org/2061893003/diff/90001/scripts/slave/recipes/b...
File scripts/slave/recipes/bisection/android_bisect.py (right):

https://codereview.chromium.org/2061893003/diff/90001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/android_bisect.py:242:
mastername='tryserver.chromium.perf',
On 2016/06/27 19:36:21, RobertoCN wrote:
> Nit: Alignment

Done.

https://codereview.chromium.org/2061893003/diff/90001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/android_bisect.py:249:
api.raw_io.stream_output('tools/run-perf-test.cfg')) +
On 2016/06/27 19:36:21, RobertoCN wrote:
> Nit: let's make this consistent. Either plus at the beginning of the line
> (preferred as per styleguide) or always at the end (some other files in
recipes
> use this style).

Done.

https://codereview.chromium.org/2061893003/diff/90001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/android_bisect.py:265: buildername=buildername,
On 2016/06/27 19:36:21, RobertoCN wrote:
> alignment

Done.

https://codereview.chromium.org/2061893003/diff/90001/scripts/slave/recipes/b...
File scripts/slave/recipes/bisection/desktop_bisect.py (right):

https://codereview.chromium.org/2061893003/diff/90001/scripts/slave/recipes/b...
scripts/slave/recipes/bisection/desktop_bisect.py:142:
api.step_data('buildbucket.put (2)',
On 2016/06/27 19:36:21, RobertoCN wrote:
> Nit: let's cleanup the alignment and operator placement (at the end of
breaking
> line, or at the beginning of continuation) for this whole file.

Done.

[Ziqi Xiong, 2016-06-27 21:27:23]

The CQ bit was checked by zxiong@google.com

[Ziqi Xiong, 2016-06-27 21:27:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertocn@chromium.org
Link to the patchset: https://codereview.chromium.org/2061893003/#ps130001
(title: "Rebase against master")

[commit-bot: I haz the power, 2016-06-27 21:27:29]

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-06-27 21:39:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-27 21:39:41]

Try jobs failed on following builders:
  Build Presubmit on master.tryserver.infra (JOB_FAILED,
https://build.chromium.org/p/tryserver.infra/builders/Build%20Presubmit/build...)

[Ziqi Xiong, 2016-06-28 17:36:22]

The CQ bit was checked by zxiong@google.com

[Ziqi Xiong, 2016-06-28 17:36:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertocn@chromium.org
Link to the patchset: https://codereview.chromium.org/2061893003/#ps170001
(title: "Add coverage")

[commit-bot: I haz the power, 2016-06-28 17:36:28]

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-06-28 17:44:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-28 17:45:01]

Try jobs failed on following builders:
  Build Presubmit on master.tryserver.infra (JOB_FAILED,
https://build.chromium.org/p/tryserver.infra/builders/Build%20Presubmit/build...)

[Ziqi Xiong, 2016-06-28 18:05:47]

The CQ bit was checked by zxiong@google.com

[Ziqi Xiong, 2016-06-28 18:05:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertocn@chromium.org
Link to the patchset: https://codereview.chromium.org/2061893003/#ps180001
(title: "Rebase against master")

[commit-bot: I haz the power, 2016-06-28 18:05:53]

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-06-28 18:15:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-28 18:15:03]

Try jobs failed on following builders:
  Build Presubmit on master.tryserver.infra (JOB_FAILED,
https://build.chromium.org/p/tryserver.infra/builders/Build%20Presubmit/build...)

[Ziqi Xiong, 2016-06-28 18:20:54]

The CQ bit was checked by zxiong@google.com

[Ziqi Xiong, 2016-06-28 18:20:55]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertocn@chromium.org
Link to the patchset: https://codereview.chromium.org/2061893003/#ps190001
(title: "Rebase again")

[commit-bot: I haz the power, 2016-06-28 18:21:02]

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-06-28 18:29:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-28 18:29:52]

Try jobs failed on following builders:
  Build Presubmit on master.tryserver.infra (JOB_FAILED,
https://build.chromium.org/p/tryserver.infra/builders/Build%20Presubmit/build...)

[RobertoCN, 2016-06-28 18:59:17]

robertocn@chromium.org changed reviewers:
+ dnj@chromium.org, hinoka@chromium.org, iannucci@chromium.org
- robertocn@google.com

[RobertoCN, 2016-06-28 18:59:18]

+Infra folks
We need someone in infra to LGTM due to:


** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    scripts/slave/recipe_modules/chromium_tests/api.py

[dnj (Google), 2016-06-28 19:31:02]

dnj@google.com changed reviewers:
+ dnj@google.com

[dnj (Google), 2016-06-28 19:31:04]

rs-lgtm since this only changes perf behavior.

[RobertoCN, 2016-06-28 19:33:11]

The CQ bit was checked by robertocn@chromium.org

[commit-bot: I haz the power, 2016-06-28 19:33:24]

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-06-28 19:42:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-28 19:42:27]

Try jobs failed on following builders:
  Build Presubmit on master.tryserver.infra (JOB_FAILED,
https://build.chromium.org/p/tryserver.infra/builders/Build%20Presubmit/build...)

[RobertoCN, 2016-06-28 19:46:31]

I think we need that rs-lgtm from your chromium account.

[dnj, 2016-06-28 21:04:49]

On 2016/06/28 19:46:31, RobertoCN wrote:
> I think we need that rs-lgtm from your chromium account.

rs-lgtm well that's dumb

[Ziqi Xiong, 2016-06-28 21:10:11]

The CQ bit was checked by zxiong@google.com

[commit-bot: I haz the power, 2016-06-28 21:10:23]

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-06-28 21:19:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-28 21:19:16]

Try jobs failed on following builders:
  Build Presubmit on master.tryserver.infra (JOB_FAILED,
https://build.chromium.org/p/tryserver.infra/builders/Build%20Presubmit/build...)

[Ziqi Xiong, 2016-06-28 22:15:57]

zxiong@google.com changed reviewers:
+ sergeyberezin@chromium.org

[Ziqi Xiong, 2016-06-28 22:15:58]

Hi Sergey,

My CL involves a minor change in
	scripts/slave/recipe_modules/chromium_tests/api.py

Let me know if the change looks good to you.

Ziqi

[Sergey Berezin, 2016-06-28 22:23:08]

On 2016/06/28 22:15:58, Ziqi Xiong wrote:
> Hi Sergey,
> 
> My CL involves a minor change in
> 	scripts/slave/recipe_modules/chromium_tests/api.py

rs lgtm

[Ziqi Xiong, 2016-06-28 22:24:05]

The CQ bit was checked by zxiong@google.com

[commit-bot: I haz the power, 2016-06-28 22:24:15]

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-06-28 22:29:34]

Description was changed from

==========
This CL attempts to implement build delegation for perf tryjobs.
Currently, perf try jobs build on the same builder that runs the test.
This is bad because often we want much slower performance characteristics on 
our perfbots than on our builders, and this makes the builds very slow or even
impossible. 

BUG= http://crbug.com/601699
==========

to

==========
This CL attempts to implement build delegation for perf tryjobs.
Currently, perf try jobs build on the same builder that runs the test.
This is bad because often we want much slower performance characteristics on 
our perfbots than on our builders, and this makes the builds very slow or even
impossible. 

BUG= http://crbug.com/601699

Committed:
https://chromium.googlesource.com/chromium/tools/build/+/b89e76b0fd8bebe0ccd4...
==========

[commit-bot: I haz the power, 2016-06-28 22:29:38]

Committed patchset #15 (id:190001) as
https://chromium.googlesource.com/chromium/tools/build/+/b89e76b0fd8bebe0ccd4...

[RobertoCN, 2016-06-28 23:16:18]

I am rolling back this patchset because it's broken try jobs:

It seems like a small typo on the account name, see comment below.

Please create a new branch where you make this fix and upload again from there,
this will create a new CL, where we'll try to land this again.

https://codereview.chromium.org/2061893003/diff/190001/scripts/slave/recipe_m...
File scripts/slave/recipe_modules/perf_try/api.py (right):

https://codereview.chromium.org/2061893003/diff/190001/scripts/slave/recipe_m...
scripts/slave/recipe_modules/perf_try/api.py:39: SERVICE_ACCOUNT =
'chromium_bisect'
this should be chromium-bisect.

[RobertoCN, 2016-06-28 23:17:12]

A revert of this CL (patchset #15 id:190001) has been created in
https://codereview.chromium.org/2103393002/ by robertocn@chromium.org.

The reason for reverting is: Broken tryjobs:
https://build.chromium.org/p/tryserver.chromium.perf/builders/linux_perf_bise....

[Ziqi Xiong, 2016-06-28 23:47:39]

https://codereview.chromium.org/2061893003/diff/190001/scripts/slave/recipe_m...
File scripts/slave/recipe_modules/perf_try/api.py (right):

https://codereview.chromium.org/2061893003/diff/190001/scripts/slave/recipe_m...
scripts/slave/recipe_modules/perf_try/api.py:39: SERVICE_ACCOUNT =
'chromium_bisect'
On 2016/06/28 23:16:17, RobertoCN wrote:
> this should be chromium-bisect.

Opened a new CL to fix this:
https://codereview.chromium.org/2103133005/