milo: Export LogDog annotation stream, remove host

milo: Export LogDog annotation stream, remove host

Update the LogDog annotation stream and associated LogDog methods to be
public. This is in anticipation of other Milo modules using it to load
annotatons.

Update URL generation to use the "viewer" package.

Remove arbitrary host specification from the LogDog endpoint (security
bug).

BUG=chromium:687625
TEST=None

Review-Url: https://codereview.chromium.org/2674603002
Committed: https://github.com/luci/luci-go/commit/6126eaafade03c4ff12640b1602c78fdbf0c09fb

[dnj, 2017-02-02 02:57:07]

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

[dnj, 2017-02-02 02:57:08]

PTAL. This set of CLs adds a pRPC service to Milo called "BuildInfo". This
service offers a one-stop shop for users to query Milo about a specific build,
referencing it using familiar terms, and receive canonical build data (mostly
LogDog annotation protobufs) and metadata. It is envisioned as an entry point
for services that wish to learn about builds or view their logs.

CL chain:
https://codereview.chromium.org/2671633002 - (prep) Export host in LogDog
client.
https://codereview.chromium.org/2674603002 - (prep) Milo logdog module exports
AnnotationStream loading.
https://codereview.chromium.org/2675493003 - (prep) Refactor Swarming a bit, add
parallel fetcher.
https://codereview.chromium.org/2667353002 - Barebones (Unimplemented) BuildInfo
service addition.
https://codereview.chromium.org/2676503002 - (prep) Use strict for BuildBot
properties instead of []interface{}
https://codereview.chromium.org/2674513003 - BuildInfo implementation for
BuildBot builds.
https://codereview.chromium.org/2667363002 - BuildInfo implementation for
Swarming Kitchen builds.

[hinoka, 2017-02-02 22:09:26]

lgtm

https://codereview.chromium.org/2674603002/diff/1/milo/appengine/logdog/build.go
File milo/appengine/logdog/build.go (right):

https://codereview.chromium.org/2674603002/diff/1/milo/appengine/logdog/build...
milo/appengine/logdog/build.go:57: return fmt.Errorf("Invalid project name: %s",
as.Project)
This makes it a 500 instead of a 400, is that intentional?

https://codereview.chromium.org/2674603002/diff/1/milo/appengine/logdog/build...
milo/appengine/logdog/build.go:61: return fmt.Errorf("Invalid log stream path
%q: %s", as.Path, err)
Same here

https://codereview.chromium.org/2674603002/diff/1/milo/appengine/logdog/build...
milo/appengine/logdog/build.go:231: func (as *AnnotationStream) Step()
*miloProto.Step { return as.cs.Step }
Can't you just do
if as.cs.Step == nil {
  err := as.Load()
  if err != nil .....
}

As an interface, Load then access is kind of a confusing interface (esp if the
access doesn't error out)

[dnj, 2017-02-02 22:29:00]

https://codereview.chromium.org/2674603002/diff/1/milo/appengine/logdog/build.go
File milo/appengine/logdog/build.go (right):

https://codereview.chromium.org/2674603002/diff/1/milo/appengine/logdog/build...
milo/appengine/logdog/build.go:57: return fmt.Errorf("Invalid project name: %s",
as.Project)
On 2017/02/02 22:09:25, hinoka wrote:
> This makes it a 500 instead of a 400, is that intentional?

oops - I moved that logic into "html.go" now.

https://codereview.chromium.org/2674603002/diff/1/milo/appengine/logdog/build...
milo/appengine/logdog/build.go:61: return fmt.Errorf("Invalid log stream path
%q: %s", as.Path, err)
On 2017/02/02 22:09:25, hinoka wrote:
> Same here

Done.

https://codereview.chromium.org/2674603002/diff/1/milo/appengine/logdog/build...
milo/appengine/logdog/build.go:231: func (as *AnnotationStream) Step()
*miloProto.Step { return as.cs.Step }
On 2017/02/02 22:09:25, hinoka wrote:
> Can't you just do
> if as.cs.Step == nil {
>   err := as.Load()
>   if err != nil .....
> }
> 
> As an interface, Load then access is kind of a confusing interface (esp if the
> access doesn't error out)

Well, this way we can reload if we want. I suppose that if we ever needed that,
we could add an "Invalidate()" method that nils it though, so ok.

[dnj, 2017-02-02 22:29:37]

The CQ bit was checked by dnj@chromium.org

[dnj, 2017-02-02 22:29:37]

The patchset sent to the CQ was uploaded after l-g-t-m from hinoka@chromium.org
Link to the patchset: https://codereview.chromium.org/2674603002/#ps20001
(title: "Comments")

[commit-bot: I haz the power, 2017-02-02 22:29:41]

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-02-02 22:37:31]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1486074577284150,
"parent_rev": "149de756b08fe70a72945361d12c8353bc8656fa", "commit_rev":
"6126eaafade03c4ff12640b1602c78fdbf0c09fb"}

[commit-bot: I haz the power, 2017-02-02 22:37:35]

Description was changed from

==========
milo: Export LogDog annotation stream, remove host

Update the LogDog annotation stream and associated LogDog methods to be
public. This is in anticipation of other Milo modules using it to load
annotatons.

Update URL generation to use the "viewer" package.

Remove arbitrary host specification from the LogDog endpoint (security
bug).

BUG=chromium:687625
TEST=None
==========

to

==========
milo: Export LogDog annotation stream, remove host

Update the LogDog annotation stream and associated LogDog methods to be
public. This is in anticipation of other Milo modules using it to load
annotatons.

Update URL generation to use the "viewer" package.

Remove arbitrary host specification from the LogDog endpoint (security
bug).

BUG=chromium:687625
TEST=None

Review-Url: https://codereview.chromium.org/2674603002
Committed:
https://github.com/luci/luci-go/commit/6126eaafade03c4ff12640b1602c78fdbf0c09fb
==========

[commit-bot: I haz the power, 2017-02-02 22:37:36]

Committed patchset #2 (id:20001) as
https://github.com/luci/luci-go/commit/6126eaafade03c4ff12640b1602c78fdbf0c09fb