Feat/docker

[nanaones]

If you are using a Docker container, you can deploy Django and Flask using Gunicorn.

[jamadden]

Thanks for the PR. Is there a reason this wasn't put in the existing examples/ directory?

[nanaones]

Thanks for the PR. Is there a reason this wasn't put in the existing examples/ directory?

Simple instructions are written in the README, but I will add more guides if needed.
What should I specifically add?

[jamadden]

No, I'm saying this is just an example. It should go in the existing examples/ directory. It should not be a new top-level directory.

[tilgovi]

I have just a few thoughts, but I really appreciate this.

Should we commit to making all or most of our configuration possible to define through the environment? I think I would like that. It would remove the need for the script that builds the command line.

Should we provide the requirements files or let users handle that themselves? For Tornado, users should already have it as a requirement because I think they need to import its APIs. It is sometimes the case that users can write code that is worker agnostic, but other times there is a need to perform concurrent, non-blocking work within a single request and users then import and depend on gevent or eventlet directly already. If we didn't have the requirements files, then the dockerfile itself gets very small.

Building on this, we could have the dockerfile install the user-supplied requirements.txt, which should also contain the framework requirement.

Some users will need to apt-get install additional libraries for their applications to work, but I don't see a great way to handle that and maybe we shouldn't.

I think what I'm seeing from the PR, my comments, and the comment from @jamadden is that this is written as if it could be an official gunicorn quick-start image, but I worry that the set of users for whom that is practical is small. With some changes, I feel like an example becomes sufficient because configuring gunicorn through the environment would be easy enough that a fairly boilerplate dockerfile would be a better starting point than a stock image.

[tilgovi]

Is it reasonable for there to be any other choice here? If we're not running any apt commands in the dockerfile, do we even need this at all?

[tilgovi]

Maybe the expectation is that users will use this as a FROM image?

[nanaones]

Maybe the expectation is that users will use this as a FROM image?

Yes, like this nginx image from dockerhub
https://hub.docker.com/_/nginx

[nanaones]

7bf1f8b

ARG DEBIAN_FRONTEND=noninteractive has been deleted.

[tilgovi]

/work/gunicorn.conf.py would be the default location for this.

[nanaones]

5bc62af

If the user manually configures the gunicorn.conf.py file, adds gunicorn.conf.py to the docker container, and then enters the path to gunicorn.conf.py in the environment variable $CONFIG_FILE_PATH, the gunicorn runs based on the specified file.

[tilgovi]

I might prefer /app or /gunicorn. Is the expectation that users will volume mount this path or that they will use FROM and add their code to this path?

[nanaones]

The set path is /work, but there is no problem even if the user uses a different path. Users can enter other environment variables to suit their environment.

So I put everything related to the settings in /config so that it doesn't overlap with the user's configuration.

[nanaones]

No, I'm saying this is just an example. It should go in the existing examples/ directory. It should not be a new top-level directory.

This idea was benchmarked on Celery's Github. Like Celery, we put GunicornDockerimage in a new top-level directory that Gunicorn can officially support. I think you can put it in the /example folder as you said.

Django and Celery officially support Docker images.

Django
https://hub.docker.com/_/django

[benoitc]

@tilgovi what about having it in this week release. I like the idea. Btw any idea where we coudl host an "official" build? apparently docker hub has became too much restrictive in term of pricing. Thoughts?

[tilgovi]

I don't think we should publish an image. The Django image that is referenced has been deprecated for reasons similar to my objection. A user likely needs their own requirements.txt anyway, so it doesn't help to pre-install Gunicorn in the image. That means the image is really just a Python image.

I think it's useful to have as an example, but I will leave a separate review comment asking for some changes.

We should not hold up release for this because I think most users will find the examples in the repository. They can be added after releasing.

[tilgovi]

I worry that the config.sh script complicates the example and makes it less accessible as an educational tool. A gunicorn.conf.py file in the example that sets values from the environment directly might be more helpful. Something like this would also make sense in any kind of Gunicorn application template. This would be analogous to how the Rails templates install a Puma config that reads from the environment, but sets defaults: https://github.com/rails/rails/blob/6-0-stable/railties/lib/rails/generators/rails/app/templates/config/puma.rb.tt

I think the example can be simplified by removing REQUIREMENTS_FILE_PATH. It made sense when the idea was to ship a base image, because there was a separation between the user's requirements and the base requirements. I would just expect now a single requirements.txt in the work path.

I don't think a separation between /config and /work is helpful, nor is CONFIG_FILE_PATH. Gunicorn accepts an argument on the command line to set the config file path, so if the default execution is gunicorn then the user can docker run app --config <path>. The entrypoint script should not be necessary.

[benoitc]

@nanaones what do you think about the suggested change by @tilgovi ?

[benoitc]

I don't think we should publish an image. The Django image that is referenced has been deprecated for reasons similar to my objection. A user likely needs their own requirements.txt anyway, so it doesn't help to pre-install Gunicorn in the image. That means the image is really just a Python image.

I think it's useful to have as an example, but I will leave a separate review comment asking for some changes.

We should not hold up release for this because I think most users will find the examples in the repository. They can be added after releasing.

Fear enough. I will make the release today. I think it would be good anyway to have a good template for docker images, so anyone can install it. Including on windows for a basic dev environment. That would solve a lot of issues for some developers today that doesn't use UNIX / Linux systems.

[nanaones]

@nanaones what do you think about the suggested change by @tilgovi ?

Sorry to answer now. I thought I was rejected because I didn't receive a reply longer than I thought.
I will post after modifying according to the above modifications.

[nanaones]

I worry that the config.sh script complicates the example and makes it less accessible as an educational tool. A gunicorn.conf.py file in the example that sets values from the environment directly might be more helpful. Something like this would also make sense in any kind of Gunicorn application template. This would be analogous to how the Rails templates install a Puma config that reads from the environment, but sets defaults: https://github.com/rails/rails/blob/6-0-stable/railties/lib/rails/generators/rails/app/templates/config/puma.rb.tt

I think the example can be simplified by removing REQUIREMENTS_FILE_PATH. It made sense when the idea was to ship a base image, because there was a separation between the user's requirements and the base requirements. I would just expect now a single requirements.txt in the work path.

I don't think a separation between /config and /work is helpful, nor is CONFIG_FILE_PATH. Gunicorn accepts an argument on the command line to set the config file path, so if the default execution is gunicorn then the user can docker run app --config <path>. The entrypoint script should not be necessary.

I sympathize enough. I would appreciate it if you think of it as a sample rather than an official image. I also agree with what you said is more useful to configure using the gunicorn.conf.py file.
The config.sh file was originally created to set environment variables that should be in gunicorn.conf.py. The reason is that when using containers through Docker or creating Kubernetes environments through Helm charts, it was easier to apply configuration values ​​as environment variables than injecting them into a file. (This is purely from my experience.).
There were also concerns that users would have a hard time using config.sh. But in this DockerFile, the user does not need to modify the config.sh file.
Also, the DockerFile posted by PR will read the contents of gunicorn.conf.py if it exists.

[nanaones]

I worry that the config.sh script complicates the example and makes it less accessible as an educational tool. A gunicorn.conf.py file in the example that sets values from the environment directly might be more helpful. Something like this would also make sense in any kind of Gunicorn application template. This would be analogous to how the Rails templates install a Puma config that reads from the environment, but sets defaults: https://github.com/rails/rails/blob/6-0-stable/railties/lib/rails/generators/rails/app/templates/config/puma.rb.tt

I think the example can be simplified by removing REQUIREMENTS_FILE_PATH. It made sense when the idea was to ship a base image, because there was a separation between the user's requirements and the base requirements. I would just expect now a single requirements.txt in the work path.

I don't think a separation between /config and /work is helpful, nor is CONFIG_FILE_PATH. Gunicorn accepts an argument on the command line to set the config file path, so if the default execution is gunicorn then the user can docker run app --config <path>. The entrypoint script should not be necessary.

The reason I put the config file in the /config folder is because the user could overwrite files like start.sh that should be in /config, which could cause the container not to work properly.

[nanaones]

@tilgovi
I have reflected the changes you requested. Please check.
Please forgive me for being late.

[tilgovi]

Why specify these as environment variables when they can all be passed to the Gunicorn CLI?

[tilgovi]

I think this is only needed for the Windows Server containers, and it's included upstream for those: docker-library/python#557

[tilgovi]

The Linux images get it from here: https://github.com/docker-library/python/blob/5590cdd4367f088277bb5494d0a0b0f65e9ab491/3.8/buster/Dockerfile#L14