Adds initial Tricium service GAE configs

Adds initial Tricium service GAE configs

This change has been deployed to chrome-infra-tricim-dev and tested manually. To re-run the test, go to the landing page and click the dummy analysis request button. The expected result is a task that goes through the three queues of the app (verified on the task queue overview page).

BUG=625781
Committed: https://chromium.googlesource.com/infra/infra/+/269063afbcba7c7f870e9522668d789673555f08

[emso, 2016-07-05 10:28:48]

Description was changed from

==========
Adds initial Tricium service GAE configs

BUG=625781
==========

to

==========
Adds initial Tricium service GAE configs

This change has been deployed to chrome-infra-tricim-dev and tested manually. To
re-run the test, go to the landing page and click the dummy analysis request
button. The expected result is a task that goes through the three queues of the
app (verified on the task queue overview page).

BUG=625781
==========

[emso, 2016-07-05 10:28:48]

emso@chromium.org changed reviewers:
+ phajdan.jr@chromium.org, sergiyb@chromium.org, tandrii@chromium.org

[Paweł Hajdan Jr., 2016-07-05 12:16:00]

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
File go/src/infra/tricium/service/default/handler.go (right):

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
go/src/infra/tricium/service/default/handler.go:16: var landingPage = `
Please consider using https://golang.org/pkg/html/template/

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
go/src/infra/tricium/service/default/handler.go:53: t :=
taskqueue.NewPOSTTask("/workflow-launcher/queue-handler",
map[string][]string{"name": {"Analyze Request"}})
Please consider using https://cloud.google.com/appengine/docs/go/taskqueue/delay
.

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
File go/src/infra/tricium/service/dispatch.yaml (right):

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
go/src/infra/tricium/service/dispatch.yaml:5: module: workflow-launcher
FWIW, I'm not sure how useful is it to have each of these parts as a different
module technically.

I remember your earlier question about how to run the modules locally. Is there
a good way to do that?

[Sergiy Byelozyorov, 2016-07-05 14:41:48]

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
File go/src/infra/tricium/service/dispatch.yaml (right):

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
go/src/infra/tricium/service/dispatch.yaml:7: - url:
"*/_ah/queue/analysis-requests*"
Why is this necessary? Looks like workflow-launcher is either triggered from
default module and from gerrit-poller and in both cases you use and explicit
POST request to /workflow-launcher/queue-handler.

Which other app or page do you plan to call it from? I'm not even sure which
function if any will handle these requests as it's not explicitly described in
workflow-launcher/workflow-launcher.yaml file. Is there some default handler
that is provided by AppEngine Go for this path?

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
go/src/infra/tricium/service/dispatch.yaml:13: - url: "*/gerrit-poller/status"
Why is it just .../status here? There is a handler for .../poll in the
gerrit-poller/handlers.go in this CL.

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
File go/src/infra/tricium/service/queue.yaml (right):

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
go/src/infra/tricium/service/queue.yaml:4: rate: 1/s
why limit the rate?

[emso, 2016-07-06 06:21:28]

Thanks for the comments so far. I've uploaded an update.

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
File go/src/infra/tricium/service/default/handler.go (right):

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
go/src/infra/tricium/service/default/handler.go:16: var landingPage = `
On 2016/07/05 12:16:00, Paweł Hajdan Jr. wrote:
> Please consider using https://golang.org/pkg/html/template/

Done. Thanks, much better.

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
go/src/infra/tricium/service/default/handler.go:53: t :=
taskqueue.NewPOSTTask("/workflow-launcher/queue-handler",
map[string][]string{"name": {"Analyze Request"}})
On 2016/07/05 12:16:00, Paweł Hajdan Jr. wrote:
> Please consider using
https://cloud.google.com/appengine/docs/go/taskqueue/delay
> .

I have considered this alternative and don't think it fits the design I'm
proposing which tries to separate the concerns of the service into separate
modules (to update and scale them separately). Using the delay package would
require the specification of how workflows are launched to be added in the
default module and the gerrit poller. That is, if there was a problem with how
workflows are launched this would require an update of the gerrit poller and the
default module, not the workflow launcher module. This seems counter-intuitive
to me.

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
File go/src/infra/tricium/service/dispatch.yaml (right):

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
go/src/infra/tricium/service/dispatch.yaml:5: module: workflow-launcher
On 2016/07/05 12:16:00, Paweł Hajdan Jr. wrote:
> FWIW, I'm not sure how useful is it to have each of these parts as a different
> module technically.
> 
> I remember your earlier question about how to run the modules locally. Is
there
> a good way to do that?

AFAIK, the benefit of having these as separate modules is the option of updating
and scaling them separately.

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
go/src/infra/tricium/service/dispatch.yaml:7: - url:
"*/_ah/queue/analysis-requests*"
On 2016/07/05 14:41:48, Sergiy Byelozyorov wrote:
> Why is this necessary? Looks like workflow-launcher is either triggered from
> default module and from gerrit-poller and in both cases you use and explicit
> POST request to /workflow-launcher/queue-handler.
> 
> Which other app or page do you plan to call it from? I'm not even sure which
> function if any will handle these requests as it's not explicitly described in
> workflow-launcher/workflow-launcher.yaml file. Is there some default handler
> that is provided by AppEngine Go for this path?

Sorry, I forgot clean up this file. I was experimenting with the queue setup and
tried to use the default queue handler URL, but as you can see I'm not using it
now. Removing this line.

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
go/src/infra/tricium/service/dispatch.yaml:13: - url: "*/gerrit-poller/status"
On 2016/07/05 14:41:48, Sergiy Byelozyorov wrote:
> Why is it just .../status here? There is a handler for .../poll in the
> gerrit-poller/handlers.go in this CL.

More garbage - sorry. Removing.

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
File go/src/infra/tricium/service/queue.yaml (right):

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
go/src/infra/tricium/service/queue.yaml:4: rate: 1/s
On 2016/07/05 14:41:48, Sergiy Byelozyorov wrote:
> why limit the rate?

I tried to not add a rate but got an error saying that I needed to add a rate. I
could increase this. This is just a starting point. Once I add the gerrit poller
it will provide an indication of actual load and this will allow for some
initial tuning (even though initially there isn't much happening at the
different stages).

[Sergiy Byelozyorov, 2016-07-06 08:46:35]

lgtm w/ suggestion

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
File go/src/infra/tricium/service/queue.yaml (right):

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
go/src/infra/tricium/service/queue.yaml:4: rate: 1/s
On 2016/07/06 06:21:28, emso wrote:
> On 2016/07/05 14:41:48, Sergiy Byelozyorov wrote:
> > why limit the rate?
> 
> I tried to not add a rate but got an error saying that I needed to add a rate.
I
> could increase this. This is just a starting point. Once I add the gerrit
poller
> it will provide an indication of actual load and this will allow for some
> initial tuning (even though initially there isn't much happening at the
> different stages).

Acknowledged.

https://codereview.chromium.org/2125603002/diff/20001/go/src/infra/tricium/se...
File go/src/infra/tricium/service/templates/base.html (right):

https://codereview.chromium.org/2125603002/diff/20001/go/src/infra/tricium/se...
go/src/infra/tricium/service/templates/base.html:14: <div><input type="submit"
value="Dummy Analysis Request"></div>
Do you want this button on each page or only on the landing page in default
module? If the latter, then please add a conditional block around it:

{{if .ShowRequestForm}}
<form action="/analyze" method="post">
  <div><input type="submit" value="Dummy Analysis Request"></div>
</form>
{{end}}

and set 'ShowRequestForm' to true in the landing page handler.

[emso, 2016-07-06 09:05:16]

https://codereview.chromium.org/2125603002/diff/20001/go/src/infra/tricium/se...
File go/src/infra/tricium/service/templates/base.html (right):

https://codereview.chromium.org/2125603002/diff/20001/go/src/infra/tricium/se...
go/src/infra/tricium/service/templates/base.html:14: <div><input type="submit"
value="Dummy Analysis Request"></div>
On 2016/07/06 08:46:34, Sergiy Byelozyorov wrote:
> Do you want this button on each page or only on the landing page in default
> module? If the latter, then please add a conditional block around it:
> 
> {{if .ShowRequestForm}}
> <form action="/analyze" method="post">
>   <div><input type="submit" value="Dummy Analysis Request"></div>
> </form>
> {{end}}
> 
> and set 'ShowRequestForm' to true in the landing page handler.

Thanks! As it's mostly for testing at this stage I didn't mind to much having
the button appear on all pages, but this is better.

[Paweł Hajdan Jr., 2016-07-06 09:49:38]

LGTM

https://codereview.chromium.org/2125603002/diff/40001/go/src/infra/tricium/se...
File go/src/infra/tricium/service/default/templates (right):

https://codereview.chromium.org/2125603002/diff/40001/go/src/infra/tricium/se...
go/src/infra/tricium/service/default/templates:1: ../templates
Is this a symlink? This repo may be checked out on Windows - I suggest a CQ dry
run and maybe manually watching infra waterfall after landing to make sure it
all works OK.

[emso, 2016-07-06 10:13:34]

The CQ bit was checked by emso@chromium.org

[emso, 2016-07-06 10:13:34]

The patchset sent to the CQ was uploaded after l-g-t-m from sergiyb@chromium.org
Link to the patchset: https://codereview.chromium.org/2125603002/#ps40001
(title: "Adds optional display of the request form")

[commit-bot: I haz the power, 2016-07-06 10:13:47]

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, 2016-07-06 10:21:13]

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

[commit-bot: I haz the power, 2016-07-06 10:21:14]

Try jobs failed on following builders:
  Infra Mac Tester on master.tryserver.infra (JOB_FAILED,
https://build.chromium.org/p/tryserver.infra/builders/Infra%20Mac%20Tester/bu...)

[emso, 2016-07-07 09:18:45]

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
File go/src/infra/tricium/service/dispatch.yaml (right):

https://codereview.chromium.org/2125603002/diff/1/go/src/infra/tricium/servic...
go/src/infra/tricium/service/dispatch.yaml:5: module: workflow-launcher
On 2016/07/06 06:21:28, emso wrote:
> On 2016/07/05 12:16:00, Paweł Hajdan Jr. wrote:
> > FWIW, I'm not sure how useful is it to have each of these parts as a
different
> > module technically.
> > 
> > I remember your earlier question about how to run the modules locally. Is
> there
> > a good way to do that?
> 
> AFAIK, the benefit of having these as separate modules is the option of
updating
> and scaling them separately. 

For got to answer your second question. I didn't find a way of testing all
modules locally so I deployed and tested that way.

[emso, 2016-07-07 09:50:09]

The CQ bit was checked by emso@chromium.org

[emso, 2016-07-07 09:50:10]

The patchset sent to the CQ was uploaded after l-g-t-m from
phajdan.jr@chromium.org, sergiyb@chromium.org
Link to the patchset: https://codereview.chromium.org/2125603002/#ps60001
(title: "Updates go packages for appengine")

[commit-bot: I haz the power, 2016-07-07 09:50:21]

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, 2016-07-07 09:58:18]

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

[commit-bot: I haz the power, 2016-07-07 09:58:19]

Try jobs failed on following builders:
  Infra Mac Tester on master.tryserver.infra (JOB_FAILED,
https://build.chromium.org/p/tryserver.infra/builders/Infra%20Mac%20Tester/bu...)

[emso, 2016-07-07 10:25:41]

The CQ bit was checked by emso@chromium.org

[emso, 2016-07-07 10:25:41]

The patchset sent to the CQ was uploaded after l-g-t-m from
phajdan.jr@chromium.org, sergiyb@chromium.org
Link to the patchset: https://codereview.chromium.org/2125603002/#ps80001
(title: "Adds infra test coverage expectation")

[commit-bot: I haz the power, 2016-07-07 10:25:48]

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, 2016-07-07 10:33:51]

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

[commit-bot: I haz the power, 2016-07-07 10:33:52]

Try jobs failed on following builders:
  Infra Mac Tester on master.tryserver.infra (JOB_FAILED,
https://build.chromium.org/p/tryserver.infra/builders/Infra%20Mac%20Tester/bu...)

[emso, 2016-07-07 10:50:39]

The CQ bit was checked by emso@chromium.org

[emso, 2016-07-07 10:50:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
phajdan.jr@chromium.org, sergiyb@chromium.org
Link to the patchset: https://codereview.chromium.org/2125603002/#ps100001
(title: "Temporarily lowers testing expectation")

[commit-bot: I haz the power, 2016-07-07 10:50: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, 2016-07-07 10:58:11]

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

[commit-bot: I haz the power, 2016-07-07 10:58:12]

Try jobs failed on following builders:
  Infra Mac Tester on master.tryserver.infra (JOB_FAILED,
https://build.chromium.org/p/tryserver.infra/builders/Infra%20Mac%20Tester/bu...)

[emso, 2016-07-08 08:42:09]

The CQ bit was checked by emso@chromium.org

[emso, 2016-07-08 08:42:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
phajdan.jr@chromium.org, sergiyb@chromium.org
Link to the patchset: https://codereview.chromium.org/2125603002/#ps120001
(title: "Adds copyright comments and infra testing files")

[commit-bot: I haz the power, 2016-07-08 08:42:21]

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, 2016-07-08 09:57:24]

Description was changed from

==========
Adds initial Tricium service GAE configs

This change has been deployed to chrome-infra-tricim-dev and tested manually. To
re-run the test, go to the landing page and click the dummy analysis request
button. The expected result is a task that goes through the three queues of the
app (verified on the task queue overview page).

BUG=625781
==========

to

==========
Adds initial Tricium service GAE configs

This change has been deployed to chrome-infra-tricim-dev and tested manually. To
re-run the test, go to the landing page and click the dummy analysis request
button. The expected result is a task that goes through the three queues of the
app (verified on the task queue overview page).

BUG=625781

Committed:
https://chromium.googlesource.com/infra/infra/+/269063afbcba7c7f870e9522668d7...
==========

[commit-bot: I haz the power, 2016-07-08 09:57:26]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/infra/infra/+/269063afbcba7c7f870e9522668d7...

[nodir, 2016-07-12 17:10:36]

nodir@chromium.org changed reviewers:
+ nodir@chromium.org

[nodir, 2016-07-12 17:10:38]

https://codereview.chromium.org/2125603002/diff/120001/go/src/infra/tricium/s...
File go/src/infra/tricium/service/default/handlers.go (right):

https://codereview.chromium.org/2125603002/diff/120001/go/src/infra/tricium/s...
go/src/infra/tricium/service/default/handlers.go:28: basePage.Execute(w, data)
drive by: the returned error is silently ignored. it won't show up in logs
applies to all Execute calls

[emso, 2016-07-14 06:48:42]

https://codereview.chromium.org/2125603002/diff/120001/go/src/infra/tricium/s...
File go/src/infra/tricium/service/default/handlers.go (right):

https://codereview.chromium.org/2125603002/diff/120001/go/src/infra/tricium/s...
go/src/infra/tricium/service/default/handlers.go:28: basePage.Execute(w, data)
On 2016/07/12 17:10:38, nodir wrote:
> drive by: the returned error is silently ignored. it won't show up in logs
> applies to all Execute calls

Yes, added a fix for this in https://codereview.chromium.org/2149853003/.
Thanks.