Open jerome-labidurie opened 2 years ago
First off, thanks! 😃
I don't use any container software so I'm pretty much indifferent concerning this. The way I see it, the dockerfile along with the shell scripts is pretty much self-contained and doesn't need any special maintenance. So that may speak for integrating it into this repository.
A few things I'm simply not sure about, sorry if these questions are just uneducated:
/usr/src/app
, is that something Docker specific?cron.sh
)? Isn't there supposed to be anything like stop.sh
, or is that rather done by simply killing the entire dock?HI,
* _MinecraftStats_ may be updated, so shouldn't the start script try to pull if the repository is already cloned - or are these docks stateless and this is how they operate?
Yes, this can be done in the starting script, as an example itzg/minecraft-server download the last server version on startup. I will do the modification. (we can also have a variable to use a specific tag or HEAD)
* Is it a good idea to hardcode the working directory to `/usr/src/app`, is that something Docker specific?
The container is based on the official python one. I just took the same path as in the examples . Another common practice is to use
/app
.* Is it common practice to run docks in an endless while loop like that (`cron.sh`)? Isn't there supposed to be anything like `stop.sh`, or is that rather done by simply killing the entire dock?
afaik, yes. The
docker stop
command will send SIGTERM (and then SIGKILL) to stop the processes. Docker expect a never ending process. But I'm far from a docker specialist !
Best regards
https://github.com/jerome-labidurie/docker-MinecraftStats/blob/main/start.sh#L47-L48
http.server
module shouldn't be used for production. use nginx webserver as part of the multi-stage buildshttps://github.com/jerome-labidurie/docker-MinecraftStats/blob/main/cron.sh#L19
A ready made docker container would be useful for easy test/deployment of MinecraftStats.
I made (a simple) one available on :
I'm not creating a pull request as I don't think this should be included in this sources.
However, if you want to take it over, here or in another repo, I would be happy to create the PR.
Best regards