Buildbucket: Save buildbucket build info and summary on pubsub push

Buildbucket: Save buildbucket build info and summary on pubsub push

BUG=624960
Review-Url: https://codereview.chromium.org/2964143002
Committed: https://github.com/luci/luci-go/commit/3f5ae6c4a1c17663c8113708a912fea6d8b98781

[Ryan Tseng, 2017-06-30 00:04:12]

The CQ bit was checked by hinoka@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-30 00:04:19]

Dry run: 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, 2017-06-30 00:08:31]

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

[commit-bot: I haz the power, 2017-06-30 00:08:32]

Dry run: Try jobs failed on following builders:
  Luci-go Mac Tester on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/370f21c7ff53f410)

[Ryan Tseng, 2017-06-30 00:23:58]

The CQ bit was checked by hinoka@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-30 00:24:08]

Dry run: 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, 2017-06-30 00:28:20]

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

[commit-bot: I haz the power, 2017-06-30 00:28:21]

Dry run: Try jobs failed on following builders:
  Luci-go Mac Tester on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/370f33de5a073210)

[Ryan Tseng, 2017-07-01 00:33:29]

The CQ bit was checked by hinoka@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-07-01 00:33:39]

Dry run: 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, 2017-07-01 00:38:49]

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

[commit-bot: I haz the power, 2017-07-01 00:38:50]

Dry run: Try jobs failed on following builders:
  Luci-go Linux Trusty 32-on-64 Tester on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/371462f527ff9410)

[Ryan Tseng, 2017-07-05 23:02:08]

The CQ bit was checked by hinoka@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-07-05 23:02:11]

Dry run: 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, 2017-07-05 23:08:00]

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

[commit-bot: I haz the power, 2017-07-05 23:08:01]

Dry run: Try jobs failed on following builders:
  Luci-go Linux Trusty 32-on-64 Tester on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/372dcf07482bbd10)

[Ryan Tseng, 2017-07-05 23:53:25]

The CQ bit was checked by hinoka@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-07-05 23:53:31]

Dry run: 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, 2017-07-05 23:59:01]

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

[commit-bot: I haz the power, 2017-07-05 23:59:01]

Dry run: Try jobs failed on following builders:
  Luci-go Presubmit on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/372dfdf93c22d610)

[Ryan Tseng, 2017-07-07 20:17:26]

The CQ bit was checked by hinoka@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-07-07 20:17:29]

Dry run: 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, 2017-07-07 20:22:18]

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

[commit-bot: I haz the power, 2017-07-07 20:22:19]

Dry run: Try jobs failed on following builders:
  Luci-go Linux Trusty 32-on-64 Tester on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/373784f2cee5fc10)
  Luci-go Linux Trusty 64 Tester on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/373784f27e81f710)

[Ryan Tseng, 2017-07-08 01:12:46]

The CQ bit was checked by hinoka@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-07-08 01:12:49]

Dry run: 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, 2017-07-08 01:17:19]

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

[commit-bot: I haz the power, 2017-07-08 01:17:19]

Dry run: Try jobs failed on following builders:
  Luci-go Linux Trusty 32-on-64 Tester on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/3738935114329310)
  Luci-go Linux Trusty 64 Tester on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/3738935109b43b10)
  Luci-go Mac Tester on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/373893516b4a0f10)

[Ryan Tseng, 2017-07-08 01:28:58]

The CQ bit was checked by hinoka@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-07-08 01:29:09]

Dry run: 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, 2017-07-08 01:34:18]

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

[commit-bot: I haz the power, 2017-07-08 01:34:19]

Dry run: Try jobs failed on following builders:
  Luci-go Linux Trusty 32-on-64 Tester on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/3738a2439777fc10)

[Ryan Tseng, 2017-07-10 23:46:21]

The CQ bit was checked by hinoka@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-07-10 23:46:31]

Dry run: 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, 2017-07-10 23:54:04]

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

[commit-bot: I haz the power, 2017-07-10 23:54:05]

Dry run: This issue passed the CQ dry run.

[Ryan Tseng, 2017-07-11 00:03:47]

Description was changed from

==========
Buildbucket: Save buildbucket build info and summary on pubsub push

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

to

==========
Buildbucket: Save buildbucket build info and summary on pubsub push

BUG=624960
==========

[Ryan Tseng, 2017-07-11 00:03:47]

hinoka@google.com changed reviewers:
+ iannucci@chromium.org

[Ryan Tseng, 2017-07-11 00:04:41]

Tests will come in a separate CL, after 2 refactors that involve moving the
swarming and logdog interfaces into the context.

https://codereview.chromium.org/2964143002/diff/160001/milo/buildsource/build...
File milo/buildsource/buildbucket/struct.go (right):

https://codereview.chromium.org/2964143002/diff/160001/milo/buildsource/build...
milo/buildsource/buildbucket/struct.go:23: buildbucketData []byte `gae:,noindex`
This would be useful as a broken out as a native struct rather than a marshalled
byte array.  It is define here:
https://cs.chromium.org/chromium/infra/go/src/github.com/luci/luci-go/common/...

it looks like it would be pretty safe to do so (no list of nested structs). 
wdyt?

[Ryan Tseng, 2017-07-11 00:07:28]

The CQ bit was checked by hinoka@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-07-11 00:07:32]

Dry run: 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, 2017-07-11 00:15:01]

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

[commit-bot: I haz the power, 2017-07-11 00:15:04]

Dry run: This issue passed the CQ dry run.

[Ryan Tseng, 2017-07-11 19:28:02]

hinoka@google.com changed reviewers:
+ dnj@chromium.org

[Ryan Tseng, 2017-07-11 19:28:03]

+dnj for more ayes (eyes?)

[iannucci, 2017-07-11 21:51:21]

lgtm for now

https://codereview.chromium.org/2964143002/diff/180001/milo/api/resp/build.go
File milo/api/resp/build.go (right):

https://codereview.chromium.org/2964143002/diff/180001/milo/api/resp/build.go...
milo/api/resp/build.go:271: func (rb *MiloBuild) SummarizeTo(bs
*model.BuildSummary) {
I'm not sure if this makes sense as a function of a MiloBuild; I suspect that
we'll want to generate and update BuildSummaries with only partial information
directly from the backends without loading the whole native build and converting
it to a MiloBuild first, right?

https://codereview.chromium.org/2964143002/diff/180001/milo/buildsource/build...
File milo/buildsource/buildbucket/pubsub.go (right):

https://codereview.chromium.org/2964143002/diff/180001/milo/buildsource/build...
milo/buildsource/buildbucket/pubsub.go:84: *resp.MiloBuild, error) {
one line?

https://codereview.chromium.org/2964143002/diff/180001/milo/buildsource/build...
milo/buildsource/buildbucket/pubsub.go:99: swarmingSvc, err :=
swarming.NewProdService(c, host)
I assume you'll change this in a subsequent CL?

https://codereview.chromium.org/2964143002/diff/180001/milo/buildsource/build...
milo/buildsource/buildbucket/pubsub.go:145: // createBuildSummary creates or
updates a build summary based off a buildbucket
this is actually putting the build summary, not just creating it. maybe
putBuildSummary? summarizeBuild?

https://codereview.chromium.org/2964143002/diff/180001/milo/buildsource/build...
File milo/buildsource/buildbucket/struct.go (right):

https://codereview.chromium.org/2964143002/diff/180001/milo/buildsource/build...
milo/buildsource/buildbucket/struct.go:28: respBuild *resp.MiloBuild
`gae:,noindex`
this doesn't have any nested stuff? Note that if this was a proto, luci/gae has
a tool to generate to/from datastore binary serializer methods, which would be
more efficient

[Ryan Tseng, 2017-07-11 22:53:39]

The CQ bit was checked by hinoka@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-07-11 22:53:42]

Dry run: 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, 2017-07-11 22:59:12]

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

[commit-bot: I haz the power, 2017-07-11 22:59:15]

Dry run: This issue passed the CQ dry run.

[Ryan Tseng, 2017-07-11 23:09:22]

https://codereview.chromium.org/2964143002/diff/180001/milo/api/resp/build.go
File milo/api/resp/build.go (right):

https://codereview.chromium.org/2964143002/diff/180001/milo/api/resp/build.go...
milo/api/resp/build.go:271: func (rb *MiloBuild) SummarizeTo(bs
*model.BuildSummary) {
On 2017/07/11 21:51:20, iannucci wrote:
> I'm not sure if this makes sense as a function of a MiloBuild; I suspect that
> we'll want to generate and update BuildSummaries with only partial information
> directly from the backends without loading the whole native build and
converting
> it to a MiloBuild first, right?

Yeah, this is meant to fill in extra information if the full build is available.
 That's why it fills in a buildSummary instead of returning one.

https://codereview.chromium.org/2964143002/diff/180001/milo/buildsource/build...
File milo/buildsource/buildbucket/pubsub.go (right):

https://codereview.chromium.org/2964143002/diff/180001/milo/buildsource/build...
milo/buildsource/buildbucket/pubsub.go:84: *resp.MiloBuild, error) {
On 2017/07/11 21:51:20, iannucci wrote:
> one line?

Done.

https://codereview.chromium.org/2964143002/diff/180001/milo/buildsource/build...
milo/buildsource/buildbucket/pubsub.go:99: swarmingSvc, err :=
swarming.NewProdService(c, host)
On 2017/07/11 21:51:20, iannucci wrote:
> I assume you'll change this in a subsequent CL?

Yep

https://codereview.chromium.org/2964143002/diff/180001/milo/buildsource/build...
milo/buildsource/buildbucket/pubsub.go:145: // createBuildSummary creates or
updates a build summary based off a buildbucket
On 2017/07/11 21:51:20, iannucci wrote:
> this is actually putting the build summary, not just creating it. maybe
> putBuildSummary? summarizeBuild?

renamed.

https://codereview.chromium.org/2964143002/diff/180001/milo/buildsource/build...
File milo/buildsource/buildbucket/struct.go (right):

https://codereview.chromium.org/2964143002/diff/180001/milo/buildsource/build...
milo/buildsource/buildbucket/struct.go:28: respBuild *resp.MiloBuild
`gae:,noindex`
On 2017/07/11 21:51:20, iannucci wrote:
> this doesn't have any nested stuff? Note that if this was a proto, luci/gae
has
> a tool to generate to/from datastore binary serializer methods, which would be
> more efficient

It's actually not nested, not even nested builds are nested ovo.

proto is good to know, thought about using the annotation proto at first but
then would also need to save swarming info, and then duplicate mixing logic...
this was a simpler approach.

[Ryan Tseng, 2017-07-11 23:09:26]

The CQ bit was checked by hinoka@google.com

[Ryan Tseng, 2017-07-11 23:09:27]

The patchset sent to the CQ was uploaded after l-g-t-m from
iannucci@chromium.org
Link to the patchset: https://codereview.chromium.org/2964143002/#ps200001
(title: "nits")

[commit-bot: I haz the power, 2017-07-11 23:09:33]

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, 2017-07-11 23:15:34]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1499814566921490,
"parent_rev": "16fa9642b663a5cc49b116b9c94d654372b934da", "commit_rev":
"3f5ae6c4a1c17663c8113708a912fea6d8b98781"}

[commit-bot: I haz the power, 2017-07-11 23:15:37]

Description was changed from

==========
Buildbucket: Save buildbucket build info and summary on pubsub push

BUG=624960
==========

to

==========
Buildbucket: Save buildbucket build info and summary on pubsub push

BUG=624960

Review-Url: https://codereview.chromium.org/2964143002
Committed:
https://github.com/luci/luci-go/commit/3f5ae6c4a1c17663c8113708a912fea6d8b98781
==========

[commit-bot: I haz the power, 2017-07-11 23:15:38]

Committed patchset #11 (id:200001) as
https://github.com/luci/luci-go/commit/3f5ae6c4a1c17663c8113708a912fea6d8b98781