scheduler: add GetJobInvocations api.

scheduler: add GetJobInvocations api.

BUG=712427
Review-Url: https://codereview.chromium.org/2948163002
Committed: https://github.com/luci/luci-go/commit/7c3b1e060dd19bad03f926f6cacc220a3182babd

[tandrii(chromium), 2017-06-21 20:44:55]

The CQ bit was checked by tandrii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-21 20:45:05]

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-21 20:50:55]

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

[commit-bot: I haz the power, 2017-06-21 20:50:56]

Dry run: This issue passed the CQ dry run.

[tandrii(chromium), 2017-06-21 20:55:22]

tandrii@chromium.org changed reviewers:
+ vadimsh@chromium.org

[tandrii(chromium), 2017-06-21 20:55:23]

ptal

[Vadim Sh., 2017-06-21 23:46:41]

https://codereview.chromium.org/2948163002/diff/1/scheduler/api/scheduler/v1/...
File scheduler/api/scheduler/v1/scheduler.proto (right):

https://codereview.chromium.org/2948163002/diff/1/scheduler/api/scheduler/v1/...
scheduler/api/scheduler/v1/scheduler.proto:58: int64 now_ts = 3;
why is this needed? We generally assume clocks and in sync everywhere, so
clients can just use their own clock.

https://codereview.chromium.org/2948163002/diff/1/scheduler/appengine/apiserv...
File scheduler/appengine/apiservers/scheduler.go (right):

https://codereview.chromium.org/2948163002/diff/1/scheduler/appengine/apiserv...
scheduler/appengine/apiservers/scheduler.go:61: return nil,
status.Errorf(codes.Internal, "datastore error: %s", err)
we don't support 'status'

use grpc.Errorf(...)

https://codereview.chromium.org/2948163002/diff/1/scheduler/appengine/apiserv...
scheduler/appengine/apiservers/scheduler.go:67: pageSize := 50
note in proto that 50 is default and maximum

[tandrii(chromium), 2017-06-22 15:47:24]

The CQ bit was checked by tandrii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-22 15:47:35]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tandrii(chromium), 2017-06-22 15:47:42]

https://codereview.chromium.org/2948163002/diff/1/scheduler/api/scheduler/v1/...
File scheduler/api/scheduler/v1/scheduler.proto (right):

https://codereview.chromium.org/2948163002/diff/1/scheduler/api/scheduler/v1/...
scheduler/api/scheduler/v1/scheduler.proto:58: int64 now_ts = 3;
On 2017/06/21 23:46:41, Vadim Sh. wrote:
> why is this needed? We generally assume clocks and in sync everywhere, so
> clients can just use their own clock.

tl;dr ok, let's remove complexity from here.

I copied this from buildbucket API for search, which returns among other
properties of a build 'utcnow_ts', cq then carefully uses that field instead of
local CQ2.golo time. I don't know why.

https://codereview.chromium.org/2948163002/diff/1/scheduler/appengine/apiserv...
File scheduler/appengine/apiservers/scheduler.go (right):

https://codereview.chromium.org/2948163002/diff/1/scheduler/appengine/apiserv...
scheduler/appengine/apiservers/scheduler.go:61: return nil,
status.Errorf(codes.Internal, "datastore error: %s", err)
On 2017/06/21 23:46:41, Vadim Sh. wrote:
> we don't support 'status'
> 
> use grpc.Errorf(...)
https://godoc.org/google.golang.org/grpc#Errorf says it is deprecated and that I
should status.Errorf instead, and in fact grpc.Errorf just calls into
status.Errorf anyway.

https://codereview.chromium.org/2948163002/diff/1/scheduler/appengine/apiserv...
scheduler/appengine/apiservers/scheduler.go:67: pageSize := 50
On 2017/06/21 23:46:41, Vadim Sh. wrote:
> note in proto that 50 is default and maximum

Done. I don't have a good way to choose the maximum; 50 is what UI uses today.

[commit-bot: I haz the power, 2017-06-22 15:53:24]

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

[commit-bot: I haz the power, 2017-06-22 15:53:25]

Dry run: This issue passed the CQ dry run.

[Vadim Sh., 2017-06-22 17:06:26]

https://codereview.chromium.org/2948163002/diff/1/scheduler/appengine/apiserv...
File scheduler/appengine/apiservers/scheduler.go (right):

https://codereview.chromium.org/2948163002/diff/1/scheduler/appengine/apiserv...
scheduler/appengine/apiservers/scheduler.go:61: return nil,
status.Errorf(codes.Internal, "datastore error: %s", err)
On 2017/06/22 15:47:42, tandrii(chromium) wrote:
> On 2017/06/21 23:46:41, Vadim Sh. wrote:
> > we don't support 'status'
> > 
> > use grpc.Errorf(...)
> https://godoc.org/google.golang.org/grpc#Errorf says it is deprecated and that
I
> should status.Errorf instead, and in fact grpc.Errorf just calls into
> status.Errorf anyway.

well.. I now for sure that pRPC implementation ignores any rich structure of
'status' replies, so we should just use grpc.Errorf for now, before anyone tries
to use unsupported status fields.

[tandrii(chromium), 2017-06-22 17:43:27]

The CQ bit was checked by tandrii@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-22 17:43:37]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tandrii(chromium), 2017-06-22 17:44:18]

https://codereview.chromium.org/2948163002/diff/1/scheduler/appengine/apiserv...
File scheduler/appengine/apiservers/scheduler.go (right):

https://codereview.chromium.org/2948163002/diff/1/scheduler/appengine/apiserv...
scheduler/appengine/apiservers/scheduler.go:61: return nil,
status.Errorf(codes.Internal, "datastore error: %s", err)
On 2017/06/22 17:06:25, Vadim Sh. wrote:
> On 2017/06/22 15:47:42, tandrii(chromium) wrote:
> > On 2017/06/21 23:46:41, Vadim Sh. wrote:
> > > we don't support 'status'
> > > 
> > > use grpc.Errorf(...)
> > https://godoc.org/google.golang.org/grpc#Errorf says it is deprecated and
that
> I
> > should status.Errorf instead, and in fact grpc.Errorf just calls into
> > status.Errorf anyway.
> 
> well.. I now for sure that pRPC implementation ignores any rich structure of
> 'status' replies, so we should just use grpc.Errorf for now, before anyone
tries
> to use unsupported status fields.

OK, changed back. Now I wonder: is returning codes.NotFound (below) OK or should
I have a separate field in reply message for this case?

[commit-bot: I haz the power, 2017-06-22 17:49:47]

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

[commit-bot: I haz the power, 2017-06-22 17:49:48]

Dry run: This issue passed the CQ dry run.

[Vadim Sh., 2017-06-22 17:50:34]

> OK, changed back. Now I wonder: is returning codes.NotFound (below) OK or
should
> I have a separate field in reply message for this case?

It's fine as is.

lgtm

[tandrii(chromium), 2017-06-22 17:51:00]

The CQ bit was checked by tandrii@chromium.org

[commit-bot: I haz the power, 2017-06-22 17:51:07]

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-22 17:55:08]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1498153860681590,
"parent_rev": "e69c6247f7a1fe7469ef61cd72bc30b609e8c06a", "commit_rev":
"7c3b1e060dd19bad03f926f6cacc220a3182babd"}

[commit-bot: I haz the power, 2017-06-22 17:55:12]

Description was changed from

==========
scheduler: add GetJobInvocations api.

BUG=712427
==========

to

==========
scheduler: add GetJobInvocations api.

BUG=712427

Review-Url: https://codereview.chromium.org/2948163002
Committed:
https://github.com/luci/luci-go/commit/7c3b1e060dd19bad03f926f6cacc220a3182babd
==========

[commit-bot: I haz the power, 2017-06-22 17:55:13]

Committed patchset #3 (id:40001) as
https://github.com/luci/luci-go/commit/7c3b1e060dd19bad03f926f6cacc220a3182babd