[Findit] Adding new fields to try job metadata and updating as soon as possible

[Findit] Adding new fields to try job metadata and updating as soon as possible

BUG=606617
Committed: https://chromium.googlesource.com/infra/infra/+/c81c5f7b19f8f145ffeee9e275f5e6162095f1d0

[lijeffrey, 2016-04-26 23:17:23]

Description was changed from

==========
[Findit] Adding new fields to try job metadata and updating as soon as possible

BUG=
==========

to

==========
[Findit] Adding new fields to try job metadata and updating as soon as possible

BUG=606617
==========

[lijeffrey, 2016-04-26 23:17:24]

lijeffrey@chromium.org changed reviewers:
+ chanli@chromium.org, stgao@chromium.org

[lijeffrey, 2016-04-26 23:20:49]

ptal

[chanli, 2016-04-26 23:52:13]

https://codereview.chromium.org/1926473002/diff/1/appengine/findit/waterfall/...
File appengine/findit/waterfall/schedule_try_job_pipeline.py (right):

https://codereview.chromium.org/1926473002/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/schedule_try_job_pipeline.py:77: try_job_data =
WfTryJobData.Create(build_id)
Just double check: is it possible that build.id is None?

https://codereview.chromium.org/1926473002/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/schedule_try_job_pipeline.py:86: return build.id
So we can use build_id here as well.

https://codereview.chromium.org/1926473002/diff/1/appengine/findit/waterfall/...
File appengine/findit/waterfall/test/schedule_try_job_pipeline_test.py (right):

https://codereview.chromium.org/1926473002/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/test/schedule_try_job_pipeline_test.py:106:
try_job_data.put()
This line can be deleted, right?

[lijeffrey, 2016-04-27 00:59:58]

https://codereview.chromium.org/1926473002/diff/1/appengine/findit/waterfall/...
File appengine/findit/waterfall/schedule_try_job_pipeline.py (right):

https://codereview.chromium.org/1926473002/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/schedule_try_job_pipeline.py:77: try_job_data =
WfTryJobData.Create(build_id)
On 2016/04/26 23:52:13, chanli wrote:
> Just double check: is it possible that build.id is None?

Not sure, but I added an assert just like in monitor_try_job_pipeline

https://codereview.chromium.org/1926473002/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/schedule_try_job_pipeline.py:86: return build.id
On 2016/04/26 23:52:13, chanli wrote:
> So we can use build_id here as well.

Done.

https://codereview.chromium.org/1926473002/diff/1/appengine/findit/waterfall/...
File appengine/findit/waterfall/test/schedule_try_job_pipeline_test.py (right):

https://codereview.chromium.org/1926473002/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/test/schedule_try_job_pipeline_test.py:106:
try_job_data.put()
On 2016/04/26 23:52:13, chanli wrote:
> This line can be deleted, right?

Oops good catch!

[stgao, 2016-04-27 17:34:33]

https://codereview.chromium.org/1926473002/diff/20001/appengine/findit/waterf...
File appengine/findit/waterfall/monitor_try_job_pipeline.py (right):

https://codereview.chromium.org/1926473002/diff/20001/appengine/findit/waterf...
appengine/findit/waterfall/monitor_try_job_pipeline.py:128:
try_job_data.try_job_url or buildbucket_build.url)
Why we don't use the one from buildbucket?

https://codereview.chromium.org/1926473002/diff/20001/appengine/findit/waterf...
File appengine/findit/waterfall/schedule_try_job_pipeline.py (right):

https://codereview.chromium.org/1926473002/diff/20001/appengine/findit/waterf...
appengine/findit/waterfall/schedule_try_job_pipeline.py:77: assert build_id
No need to assert here. If no error, build id will always be set by buildbucket.

[lijeffrey, 2016-04-27 22:21:03]

https://codereview.chromium.org/1926473002/diff/20001/appengine/findit/waterf...
File appengine/findit/waterfall/monitor_try_job_pipeline.py (right):

https://codereview.chromium.org/1926473002/diff/20001/appengine/findit/waterf...
appengine/findit/waterfall/monitor_try_job_pipeline.py:128:
try_job_data.try_job_url or buildbucket_build.url)
On 2016/04/27 17:34:33, stgao wrote:
> Why we don't use the one from buildbucket?

In case it's already set from line 229 when the build status first changes to
STARTED, though it shouldn't make a difference. I'll change it back to just
buildbucket_build.url

https://codereview.chromium.org/1926473002/diff/20001/appengine/findit/waterf...
File appengine/findit/waterfall/schedule_try_job_pipeline.py (right):

https://codereview.chromium.org/1926473002/diff/20001/appengine/findit/waterf...
appengine/findit/waterfall/schedule_try_job_pipeline.py:77: assert build_id
On 2016/04/27 17:34:33, stgao wrote:
> No need to assert here. If no error, build id will always be set by
buildbucket.

Done.

[chanli, 2016-04-27 23:07:01]

On 2016/04/27 22:21:03, lijeffrey wrote:
>
https://codereview.chromium.org/1926473002/diff/20001/appengine/findit/waterf...
> File appengine/findit/waterfall/monitor_try_job_pipeline.py (right):
> 
>
https://codereview.chromium.org/1926473002/diff/20001/appengine/findit/waterf...
> appengine/findit/waterfall/monitor_try_job_pipeline.py:128:
> try_job_data.try_job_url or buildbucket_build.url)
> On 2016/04/27 17:34:33, stgao wrote:
> > Why we don't use the one from buildbucket?
> 
> In case it's already set from line 229 when the build status first changes to
> STARTED, though it shouldn't make a difference. I'll change it back to just
> buildbucket_build.url
> 
>
https://codereview.chromium.org/1926473002/diff/20001/appengine/findit/waterf...
> File appengine/findit/waterfall/schedule_try_job_pipeline.py (right):
> 
>
https://codereview.chromium.org/1926473002/diff/20001/appengine/findit/waterf...
> appengine/findit/waterfall/schedule_try_job_pipeline.py:77: assert build_id
> On 2016/04/27 17:34:33, stgao wrote:
> > No need to assert here. If no error, build id will always be set by
> buildbucket.
> 
> Done.

lgtm

[lijeffrey, 2016-04-27 23:11:22]

The CQ bit was checked by lijeffrey@chromium.org

[commit-bot: I haz the power, 2016-04-27 23:11:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1926473002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1926473002/40001

[commit-bot: I haz the power, 2016-04-27 23:16:35]

Description was changed from

==========
[Findit] Adding new fields to try job metadata and updating as soon as possible

BUG=606617
==========

to

==========
[Findit] Adding new fields to try job metadata and updating as soon as possible

BUG=606617

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

[commit-bot: I haz the power, 2016-04-27 23:16:35]

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