standardebooks / web

The source code for the Standard Ebooks website.
https://standardebooks.org
Creative Commons Zero v1.0 Universal
236 stars 66 forks source link

Update Docker setup in readme #301

Open acabal opened 10 months ago

acabal commented 10 months ago

Hi @robinwhittleton, we recently had a PR to update the Docker config, which resulted in some confusion because the readme asks to call docker run to start the container. People who don't know the difference between docker run and docker start can end up using docker run repeatedly, which will try to re-create the container repeatedly when that's not necessary.

Can you update the readme to use docker create + docker start instead of docker run? Also can you try running the new Docker container to confirm I got all the updates right? Thanks!

robinwhittleton commented 10 months ago

This is where my lack of Docker knowledge shows: it’s not a technology I use particularly beyond starting a container sometimes, and this was my first attempt to actually write a Dockerfile myself.

Either way, I spun up your latest changes and am getting 500 errors. Not sure if it’s a config problem at my end or not, will spend a little time debugging.

acabal commented 10 months ago

OK. The sticking point in that PR was that the person wanted to add guards to start-server.sh to not initialize the container if it was already initialized; but in my thinking, there must be a place where we can put initialization code so that it initializes the container exactly once, so adding guards is not the right solution.

For example if we have a container with a database, when we create the image we want to pre-populate the database with data for initial configuration. There must be a way to do that exactly once - otherwise every time the container starts the database would get overwritten with pre-populated data!

I don't know where that place is, so we have to figure this out. Or maybe the person in the PR was also wrong?