Convert sky_server over to a go-based http server.

Convert sky_server over to a go-based http server.

We keep seeing timeouts on the bots that seem to be due
to cherrypy dropping requests on the floor, which in turn
causes imports to stall, which causes the test to time out.
In local testing, I was able to reproduce the timeouts
and wasn't able to do so with the go-based server.

R=abarth@chromium.org, eseidel@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/9bc957919c2969883e10c0faeda78d53d96a08ed

[ojan, 2014-11-22 03:59:20]

ojan@chromium.org changed reviewers:
+ abarth@chromium.org, eseidel@chromium.org, esprehn@chromium.org

[abarth-chromium, 2014-11-22 04:08:46]

lgtm

[abarth-chromium, 2014-11-22 04:09:56]

Actually, can we not check in the binary?

[jamesr, 2014-11-24 19:21:18]

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

[jamesr, 2014-11-24 19:21:18]

Yeah, please don't check a binary in.  Instead add a file describing how to
build the binary (including what toolchain version etc you're using) and use
this script:

https://code.google.com/p/chromium/codesearch#chromium/tools/depot_tools/uplo...

and check in a .sha1 file.  Then add a hook using this script:

https://code.google.com/p/chromium/codesearch#chromium/tools/depot_tools/down...

like one of these:

https://code.google.com/p/chromium/codesearch#chromium/src/DEPS&q=DEPS&sq=pac...

to plop the binary on disk somewhere.

[ojan, 2014-12-03 02:08:12]

OK. Done. abarth really wanted the binary to only be pulled if you actually
tried to use it instead of everyone who checks out mojo needing to download it.
So I've made test_sky, test_perf and skydb do the downloading instead of
gclient.

Also, this patch depends on https://codereview.chromium.org/752803003 since it
needs it in order to put the binary in the out directory instead of next to the
.sha1 file.

hinoka, agable: ^^^^^ FYI.

[ojan, 2014-12-03 02:09:29]

To be clear, this needs another review since it now touches a ton of python code
that the previously lgtm'ed patch didn't.

[esprehn, 2014-12-03 02:26:43]

https://codereview.chromium.org/746373002/diff/20001/sky/tools/skygo/sky_serv...
File sky/tools/skygo/sky_server.go (right):

https://codereview.chromium.org/746373002/diff/20001/sky/tools/skygo/sky_serv...
sky/tools/skygo/sky_server.go:27: http.ServeFile(w, r, path)
Does this serve images with the right mime?

[ojan, 2014-12-03 02:54:19]

https://codereview.chromium.org/746373002/diff/20001/sky/tools/skygo/sky_serv...
File sky/tools/skygo/sky_server.go (right):

https://codereview.chromium.org/746373002/diff/20001/sky/tools/skygo/sky_serv...
sky/tools/skygo/sky_server.go:27: http.ServeFile(w, r, path)
On 2014/12/03 02:26:43, esprehn wrote:
> Does this serve images with the right mime?

Yes

[eseidel, 2014-12-04 18:36:48]

lgtm

i'm not sure why this is better than httpd.  I guess having some go on our
brains could be helpful.

[esprehn, 2014-12-04 20:05:49]

On 2014/12/04 at 18:36:48, eseidel wrote:
> lgtm
> 
> i'm not sure why this is better than httpd.  I guess having some go on our
brains could be helpful.

Yeah, I'm not sure why we don't just use apache, but we can give it a shot. :)

[jamesr, 2014-12-04 22:15:08]

https://codereview.chromium.org/746373002/diff/20001/sky/tools/skygo/sky_serv...
File sky/tools/skygo/sky_server.go (right):

https://codereview.chromium.org/746373002/diff/20001/sky/tools/skygo/sky_serv...
sky/tools/skygo/sky_server.go:14: type skyHandler struct {
if the only state you need is a string, you can just do

type skyHandler string

and implement the function on skyHandler

func (s skyHandler) ServeHTTP(...) {
   path := path.Join(s, r.URL.Path)
   ...
}

types can be all sorts of things in go

https://codereview.chromium.org/746373002/diff/20001/sky/tools/skygo/sky_serv...
sky/tools/skygo/sky_server.go:18: func SkyHandler(root string) http.Handler {
don't need to export this, so lowercase (although i don't think you need it at
all)

[ojan, 2014-12-04 23:57:20]

https://codereview.chromium.org/746373002/diff/20001/sky/tools/skygo/sky_serv...
File sky/tools/skygo/sky_server.go (right):

https://codereview.chromium.org/746373002/diff/20001/sky/tools/skygo/sky_serv...
sky/tools/skygo/sky_server.go:14: type skyHandler struct {
On 2014/12/04 22:15:08, jamesr wrote:
> if the only state you need is a string, you can just do
> 
> type skyHandler string
> 
> and implement the function on skyHandler
> 
> func (s skyHandler) ServeHTTP(...) {
>    path := path.Join(s, r.URL.Path)
>    ...
> }
> 
> types can be all sorts of things in go

I think my go-fu is not quite up to this. I tried this with s/s/root/ and got
the following error:
$ go build sky_server.go 
# command-line-arguments
./sky_server.go:17: cannot use root (type *skyHandler) as type string in
argument to path.Join
./sky_server.go:34: cannot use skyHandler(root) (type skyHandler) as type
http.Handler in argument to http.Handle:
	skyHandler does not implement http.Handler (ServeHTTP method has pointer
receiver)

I'm gonna commit this for now. Happy to fix this in a followup (would actually
be happy to better learn the language).

FWIW, I arrived at this code by cargo-culting the FileServer in the golang http
code.

https://codereview.chromium.org/746373002/diff/20001/sky/tools/skygo/sky_serv...
sky/tools/skygo/sky_server.go:18: func SkyHandler(root string) http.Handler {
On 2014/12/04 22:15:08, jamesr wrote:
> don't need to export this, so lowercase (although i don't think you need it at
> all)

done

[jamesr, 2014-12-05 00:02:43]

http://play.golang.org/p/Dkxr1yPD6I

Sounds like you used a pointer to a skyHandler instead of a skyHandler as the
receiver of ServeHTTP or passed a pointer instead of an instance to Handle()

[ojan, 2014-12-05 00:04:15]

On 2014/12/05 at 00:02:43, jamesr wrote:
> http://play.golang.org/p/Dkxr1yPD6I
> 
> Sounds like you used a pointer to a skyHandler instead of a skyHandler as the
receiver of ServeHTTP or passed a pointer instead of an instance to Handle()

Lol. Yeah. I just realized that a minute before you said this. Will fix and
land. Thanks!

[jamesr, 2014-12-05 00:07:51]

Cool.  If you end up needing more state using a struct and a pointer-to-struct
is really useful since you don't want to copy everything for each request, but
as long as you just have a string this is a little nicer imho

[ojan, 2014-12-05 00:09:07]

On 2014/12/05 at 00:07:51, jamesr wrote:
> Cool.  If you end up needing more state using a struct and a pointer-to-struct
is really useful since you don't want to copy everything for each request, but
as long as you just have a string this is a little nicer imho

Actually...still doesn't seem to work. path.Join doesn't like taking a
skyHandler. http://play.golang.org/p/l9pWIsQsRo

[ojan, 2014-12-05 00:10:14]

Committed patchset #3 (id:40001) manually as
9bc957919c2969883e10c0faeda78d53d96a08ed (presubmit successful).

[jamesr, 2014-12-05 00:13:10]

On 2014/12/05 00:09:07, ojan wrote:
> On 2014/12/05 at 00:07:51, jamesr wrote:
> > Cool.  If you end up needing more state using a struct and a
pointer-to-struct
> is really useful since you don't want to copy everything for each request, but
> as long as you just have a string this is a little nicer imho
> 
> Actually...still doesn't seem to work. path.Join doesn't like taking a
> skyHandler. http://play.golang.org/p/l9pWIsQsRo

http://play.golang.org/p/m_G2NmiqAV