Docker setup with nginx #588
Conversation
docker_build.sh
Outdated
|
|
||
| # Get yarn installed correctly | ||
| curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | apt-key add - | ||
| apt-get update |
There was a problem hiding this comment.
this has to be in the dockerfile
There was a problem hiding this comment.
Because those calls can be cached into a docker layer
There was a problem hiding this comment.
My understanding is that it's still cached into a docker layer, and that it's a good idea to collapse your Dockerfile into as few layers as needed. Is my understanding wrong?
There was a problem hiding this comment.
Yes but the whole layer will have the apt-get update and all the install of yarn and the project itself
There was a problem hiding this comment.
Yes, and that was my intent in arranging it this way. It bundles all the app dependencies together and ensures that stale dependencies don't break the app.
There was a problem hiding this comment.
After talking to yan a bit yesterday we're revising the direction and goal of this PR. I'm going to rework it so that API requests are always proxied through nginx. This eliminates the need to yarn build at startup time, so I'll be refactoring this PR in a way that makes this conversation moot.
Yeah, all kinds of bad choices here, nothing good. I tried to curl | bash the install script, but it randomly fails. Same if I include the install script. This seemed like the least bad choice, because at least it works every time. |
why not just ref: https://github.com/nvm-sh/nvm/blob/master/README.md |
|
Could those scripts be in a
Weird. Like network errors? |
I'm not sure what was going on. I tried to debug it but the nvm code is pretty spaghetti-ish. Something that's part of generating the NVM URL ends up blanking it out, and I wasn't able to track down why or what. I can give it a try again after some of the reworking I talked to yan about. |
Looking forward to seeing the new version. My gut tells me that doing dependency resolution at runtime in docker is not the best way forward. Ideally at build time when we bake the container all dependencies are resolved and we just distribute the build artifacts. Hopefully that's also where Skully is aiming for. Other than that, very excited to see this project happen. |
|
This should ready to go. All API requests go through nginx now, and the only stuff in the build script is strictly related to providing a happy environment to nvm. |
|
@skullydazed can you give instructions on how to start the container? I tried Changes look very good, but I wanted to test it locally before approval |
OK, I figured it out from the README. I think showing how to build it would be good to, but other than that I'm ready to approve this. Tested on macOS |
|
@skullydazed are you happy with this? |
|
I'm happy with the code itself. You wanted me to add build instructions to the readme and I haven't had a chance to sit down to do that yet. |
|
@skullydazed let me know when you're done with docs and we can merge this |
skullydazed
force-pushed
the
docker_setup
branch
from
84c0819 to
9b6cb4a
Compare
|
I've just added (and tested) instructions for building the container, and rebased from master. This PR should be ready to go. |
|
Build failure is due to force push causing a broken git command. |
So that we can share build artifacts between different projects (and because it's becoming more popular to develop locally using docker) I setup a docker container for configurator. We will be able to leverage this for both https://github.com/qmk/qmk_web_stack and https://github.com/qmk/qmk_configurator_appliance.