Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the caching behavior of web/Dockerfile. #329

Closed
wants to merge 1 commit into from

Conversation

briansmith
Copy link
Contributor

Previously if anything changed under web/ then the images would get
completely rebuilt. In particular fetching new packages in
yarn install can be very slow.

Attempt to minimize file dependencies in earlier stages of the Dockerfile
to maximize the number of layers that can be cached, both in the web
asset stage and the go server stage.

Signed-off-by: Brian Smith [email protected]

@briansmith briansmith requested a review from rmars February 10, 2018 02:27
@briansmith briansmith self-assigned this Feb 10, 2018
@adleong adleong added the review/ready Issue has a reviewable PR label Feb 10, 2018
@briansmith
Copy link
Contributor Author

@rmars @klingerf There's no rush for this. TBH, this is more of a "does this idea make sense at all" question rather than a serious PR because I haven't even tested the resultant image, beyond verifying that bin/docker-build-web succeeds. In particular, I am assuming yarn install only package.json and yarn.lock to execute correctly based on seeing others do it, e.g. yarnpkg/yarn#749 (comment). Also I'm assuming that the Go building step doesn't require any non-Go source files.

@briansmith briansmith force-pushed the b/web-Dockerfile-caching branch 2 times, most recently from f6fb7ba to dcfd1ef Compare February 10, 2018 03:17
@briansmith briansmith force-pushed the b/web-Dockerfile-caching branch from dcfd1ef to 31b1fe6 Compare February 12, 2018 23:49
Previously if anything changed under web/ then the images would get
completely rebuilt. In particular fetching new packages in
`yarn install` can be very slow.

Attempt to minimize file dependencies in earlier stages of the Dockerfile
to maximize the number of layers that can be cached, both in the web
asset stage and the go server stage.

Signed-off-by: Brian Smith <[email protected]>
@briansmith briansmith force-pushed the b/web-Dockerfile-caching branch from 31b1fe6 to ba6ebc1 Compare February 13, 2018 22:34
Copy link

@rmars rmars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when I run docker-compose build && docker-compose up -d on this branch, then visit the web UI, I get errors:

web_1          | 2018/02/16 21:02:25 http: panic serving 172.21.0.1:58688: runtime error: invalid memory address or nil pointer dereference
web_1          | goroutine 54 [running]:
web_1          | net/http.(*conn).serve.func1(0xc420082780)
web_1          | 	/usr/local/go/src/net/http/server.go:1697 +0xd0
web_1          | panic(0xd43760, 0x139d080)
web_1          | 	/usr/local/go/src/runtime/panic.go:491 +0x283
web_1          | html/template.(*Template).lookupAndEscapeTemplate(0x0, 0xe4bfd1, 0x4, 0x0, 0x0, 0x0)
web_1          | 	/usr/local/go/src/html/template/template.go:144 +0x50
web_1          | html/template.(*Template).ExecuteTemplate(0x0, 0x7faaa811d470, 0xc4204702d0, 0xe4bfd1, 0x4, 0xd80460, 0xc4202fa0e0, 0x7faaa8179ef0, 0xcf6660)
web_1          | 	/usr/local/go/src/html/template/template.go:133 +0x43
web_1          | github.com/runconduit/conduit/web/srv.(*Server).RenderTemplate(0xc42006e3c0, 0x7faaa81803f0, 0xc4204702d0, 0xe548fa, 0xd, 0xe4bfd1, 0x4, 0xdbe680, 0xc420039590, 0xc420039590, ...)
web_1          | 	/go/src/github.com/runconduit/conduit/web/srv/server.go:115 +0x314
web_1          | github.com/runconduit/conduit/web/srv.(*Server).RenderTemplate-fm(0x7faaa81803f0, 0xc4204702d0, 0xe548fa, 0xd, 0xe4bfd1, 0x4, 0xdbe680, 0xc420039590, 0x0, 0x0)
web_1          | 	/go/src/github.com/runconduit/conduit/web/srv/server.go:69 +0x8d
web_1          | github.com/runconduit/conduit/web/srv.(*handler).handleIndex(0xc4200398f0, 0x7faaa81803f0, 0xc4204702d0, 0xc420412900, 0x0, 0x0, 0x0)
web_1          | 	/go/src/github.com/runconduit/conduit/web/srv/handlers.go:35 +0x1d1
web_1          | github.com/runconduit/conduit/web/srv.(*handler).(github.com/runconduit/conduit/web/srv.handleIndex)-fm(0x7faaa81803f0, 0xc4204702d0, 0xc420412900, 0x0, 0x0, 0x0)
web_1          | 	/go/src/github.com/runconduit/conduit/web/srv/server.go:82 +0x66
web_1          | github.com/julienschmidt/httprouter.(*Router).ServeHTTP(0xc4200392c0, 0x7faaa81803f0, 0xc4204702d0, 0xc420412900)
web_1          | 	/go/src/github.com/julienschmidt/httprouter/router.go:299 +0x6f1
web_1          | github.com/runconduit/conduit/web/srv.(*Server).ServeHTTP(0xc42006e3c0, 0x7faaa81803f0, 0xc4204702d0, 0xc420412900)
web_1          | 	/go/src/github.com/runconduit/conduit/web/srv/server.go:49 +0x4d

probably due to the template files not being where the web server expects them

@briansmith briansmith assigned briansmith and unassigned briansmith Feb 22, 2018
@briansmith
Copy link
Contributor Author

Thanks @rmars I didn't realize that templates/ has to be copied too. I didn't (don't) quite know how the web parts work.

Anyway, I'm going to drop this. I'm not in a good position to validate changes to the web/ component like this and I've no context on what would actually be optimal. I'll let the branch continue to exist for a while in case somebody cares about this and wants to take it over the line.

@briansmith briansmith closed this Feb 22, 2018
@adleong adleong removed the review/ready Issue has a reviewable PR label Feb 22, 2018
@briansmith briansmith deleted the b/web-Dockerfile-caching branch April 10, 2018 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants