Milo: Store console defs as their own entities

Milo: Store console defs as their own entities

BUG=468053
Review-Url: https://codereview.chromium.org/2982183002
Committed: https://github.com/luci/luci-go/commit/09620964f484b1ba31711085e071f0f629531084

[Ryan Tseng, 2017-07-18 23:14:55]

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

[commit-bot: I haz the power, 2017-07-18 23:15: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-07-18 23:19:16]

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

[commit-bot: I haz the power, 2017-07-18 23:19:17]

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/3770cd7cf2bf3d10)

[Ryan Tseng, 2017-07-19 18:50:55]

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

[commit-bot: I haz the power, 2017-07-19 18:51:07]

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-19 18:56:16]

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

[commit-bot: I haz the power, 2017-07-19 18:56:17]

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/37750228d8110b10)

[Ryan Tseng, 2017-07-19 19:08:10]

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

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

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-19 19:13:26]

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

[commit-bot: I haz the power, 2017-07-19 19:13:28]

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/377511db6a89d410)

[Ryan Tseng, 2017-07-19 19:45:01]

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

[commit-bot: I haz the power, 2017-07-19 19:45:05]

Dry run: CQ is trying da patch.

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

[Ryan Tseng, 2017-07-19 19:51:01]

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

[Ryan Tseng, 2017-07-19 19:51:02]

Missing tests will come to a future CL near you

https://codereview.chromium.org/2982183002/diff/60001/milo/buildsource/buildb...
File milo/buildsource/buildbot/pubsub_test.go (left):

https://codereview.chromium.org/2982183002/diff/60001/milo/buildsource/buildb...
milo/buildsource/buildbot/pubsub_test.go:475: err :=
common.UpdateProjectConfigs(c)
This actually hasn't done anything useful for a while since we switch to using
cfgclient caching.

[commit-bot: I haz the power, 2017-07-19 19:52:37]

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

[commit-bot: I haz the power, 2017-07-19 19:52:38]

Dry run: This issue passed the CQ dry run.

[Ryan Tseng, 2017-07-19 19:57:43]

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

[commit-bot: I haz the power, 2017-07-19 19:57:46]

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-19 20:03:56]

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

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

Dry run: This issue passed the CQ dry run.

[iannucci, 2017-07-19 22:54:10]

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go
File milo/common/config.go (right):

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go#n...
milo/common/config.go:45: func maybePutProject(c context.Context, id string)
error {
The Project only has the ID... why bother putting it all?

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go#n...
milo/common/config.go:58: // Parent is a key to the parent project entity where
this console was
capital Project (since its a type)

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go#n...
milo/common/config.go:258: // Ensure that the project exists as an entity.
but y?

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go#n...
milo/common/config.go:268: knownConsoles := make(map[string]bool,
len(proj.Consoles))
stringset.New is handy (in luci-go/common/data/stringset)

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go#n...
milo/common/config.go:270: // known ones if needed.  If any consoles errors out,
then the whole project fails.
'whole project fails' but not really; this isn't in a transaction

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go#n...
milo/common/config.go:298: q = q.KeysOnly(true)
you can string these together btw

q := datastore.NewQuery("Console").Ancestor(parentKey).KeysOnly(true)

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go#n...
milo/common/config.go:299: existingConsoles := []*Console{}
make this []*datastore.Key{}

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go#n...
milo/common/config.go:304: if _, ok := knownConsoles[ec.ID]; !ok {
make a new []*datastore.Key{}, and append to-be-deleted keys to it

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go#n...
milo/common/config.go:312: return nil
do a single `datastore.Delete(c, keysToDelete)`

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go#n...
milo/common/config.go:348: q := datastore.NewQuery("Project")
projectKey := datastore.MakeKey("Project", projName)
keys := []*datastore.Key{}
datastore.GetAll(c, datastore.NewQuery("").Ancestor(projectKey), &keys)
datastore.Delete(c, keys)

[Ryan Tseng, 2017-07-20 00:12:33]

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

[commit-bot: I haz the power, 2017-07-20 00:12:36]

Dry run: CQ is trying da patch.

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

[Ryan Tseng, 2017-07-20 00:15:16]

Latest CL also fixes config.html

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go
File milo/common/config.go (right):

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go#n...
milo/common/config.go:45: func maybePutProject(c context.Context, id string)
error {
On 2017/07/19 22:54:10, iannucci wrote:
> The Project only has the ID... why bother putting it all?

Removed.

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go#n...
milo/common/config.go:58: // Parent is a key to the parent project entity where
this console was
On 2017/07/19 22:54:10, iannucci wrote:
> capital Project (since its a type)

Done.

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go#n...
milo/common/config.go:258: // Ensure that the project exists as an entity.
On 2017/07/19 22:54:10, iannucci wrote:
> but y?

Removed

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go#n...
milo/common/config.go:268: knownConsoles := make(map[string]bool,
len(proj.Consoles))
On 2017/07/19 22:54:09, iannucci wrote:
> stringset.New is handy (in luci-go/common/data/stringset)

Done.

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go#n...
milo/common/config.go:270: // known ones if needed.  If any consoles errors out,
then the whole project fails.
On 2017/07/19 22:54:09, iannucci wrote:
> 'whole project fails' but not really; this isn't in a transaction

Done.

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go#n...
milo/common/config.go:298: q = q.KeysOnly(true)
On 2017/07/19 22:54:10, iannucci wrote:
> you can string these together btw
> 
> q := datastore.NewQuery("Console").Ancestor(parentKey).KeysOnly(true)

Yeah, I like doing them line by line tho, i thought it looked nicer this way.

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go#n...
milo/common/config.go:299: existingConsoles := []*Console{}
On 2017/07/19 22:54:10, iannucci wrote:
> make this []*datastore.Key{}

Done.

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go#n...
milo/common/config.go:304: if _, ok := knownConsoles[ec.ID]; !ok {
On 2017/07/19 22:54:10, iannucci wrote:
> make a new []*datastore.Key{}, and append to-be-deleted keys to it

Done.

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go#n...
milo/common/config.go:312: return nil
On 2017/07/19 22:54:10, iannucci wrote:
> do a single `datastore.Delete(c, keysToDelete)`

Done.

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go#n...
milo/common/config.go:348: q := datastore.NewQuery("Project")
On 2017/07/19 22:54:09, iannucci wrote:
> projectKey := datastore.MakeKey("Project", projName)
> keys := []*datastore.Key{}
> datastore.GetAll(c, datastore.NewQuery("").Ancestor(projectKey), &keys)
> datastore.Delete(c, keys)

Project removed.

https://codereview.chromium.org/2982183002/diff/80001/milo/common/config.go#n...
milo/common/config.go:407: cons, err := GetAllConsoles(c)
do the thing

Done

[commit-bot: I haz the power, 2017-07-20 00:18:47]

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

[commit-bot: I haz the power, 2017-07-20 00:18:48]

Dry run: This issue passed the CQ dry run.

[iannucci, 2017-07-20 00:21:47]

lgtm

https://codereview.chromium.org/2982183002/diff/100001/milo/common/config.go
File milo/common/config.go (right):

https://codereview.chromium.org/2982183002/diff/100001/milo/common/config.go#...
milo/common/config.go:312: existingConsoles, err := GetAllConsoles(c, "")
err := datastore.Run(c, datastore.NewQuery("Console"), func(k *datastore.Key) {
  // do stuff with the Console key, Project name will be k.Parent().StringID()
})

[Ryan Tseng, 2017-07-20 00:41:59]

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

[commit-bot: I haz the power, 2017-07-20 00:42: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-07-20 00:48:16]

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

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

Dry run: This issue passed the CQ dry run.

[Ryan Tseng, 2017-07-20 00:58:52]

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

[commit-bot: I haz the power, 2017-07-20 00:58:56]

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-20 01:02:26]

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

[commit-bot: I haz the power, 2017-07-20 01:02:27]

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/377652e49f949410)
  Luci-go Linux Trusty 64 Tester on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/377652e4d3b2e710)

[Ryan Tseng, 2017-07-20 01:17:16]

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

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

Dry run: CQ is trying da patch.

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

[Ryan Tseng, 2017-07-20 01:17:43]

The CQ bit was unchecked by hinoka@google.com

[Ryan Tseng, 2017-07-20 01:17:46]

The CQ bit was checked by hinoka@google.com

[Ryan Tseng, 2017-07-20 01:17:47]

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/2982183002/#ps160001
(title: "Rebase fixes and Working")

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

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-20 01:24:11]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1500513466504350,
"parent_rev": "52e6f7699476bd5518331dbe9600f211763be68a", "commit_rev":
"09620964f484b1ba31711085e071f0f629531084"}

[commit-bot: I haz the power, 2017-07-20 01:24:14]

Description was changed from

==========
Milo: Store console defs as their own entities

BUG=468053
==========

to

==========
Milo: Store console defs as their own entities

BUG=468053

Review-Url: https://codereview.chromium.org/2982183002
Committed:
https://github.com/luci/luci-go/commit/09620964f484b1ba31711085e071f0f629531084
==========

[commit-bot: I haz the power, 2017-07-20 01:24:15]

Committed patchset #9 (id:160001) as
https://github.com/luci/luci-go/commit/09620964f484b1ba31711085e071f0f629531084