docker.up: Fix --no-django-debug behavior and introduce --detach

[hoyes]

I'd like to propose a couple of small tweaks to inv docker.up

RTD_DJANGO_DEBUG has a default value of True, so ensure that an empty variable is passed to docker compose when --no-django-debug is passed to inv docker.up.

docker.up: Introduce the --detach parameter. This makes it simple to start the application in detached/daemon mode without requiring a terminal to output to.

[humitos]

Thanks for your contribution. I have a few questions before moving forward with this PR.

RTD_DJANGO_DEBUG has a default value of True, so ensure that an empty variable is passed to docker compose when --no-django-debug is passed to inv docker.up.

What's the reasoning behind this behavior? As far as I can tell, all the other environment work in the same way: only define it when it's enabled.

docker.up: Introduce the --detach parameter. This makes it simple to start the application in detached/daemon mode without requiring a terminal to output to.

I don't have a strong preference for this parameter and I never needed while doing development. I usually always need to check the output to understand how the application is working. Can you expand on what is the use case behind this?

[hoyes]

Thanks for your response.

What's the reasoning behind this behavior? As far as I can tell, all the other environment work in the same way: only define it when it's enabled.

Currently inv docker.up --no-django-debug seems to be ineffective because https://github.com/readthedocs/readthedocs.org/blob/main/readthedocs/settings/docker_compose.py#L11 defaults to True if the env var is not set. This change explicitly sets RTD_DJANGO_DEBUG to a false-y value.

I don't have a strong preference for this parameter and I never needed while doing development. I usually always need to check the output to understand how the application is working. Can you expand on what is the use case behind this?

Both these changes are related to a small RTD internal deployment at Arm, where we want to leave the instance unattended. I'm not asking for support with this (it's an internal-facing instance for private projects only)... but wondered if you'd be willing to include the extra parameter upstream to minimize patching.

[humitos]

Currently inv docker.up --no-django-debug seems to be ineffective because readthedocs/readthedocs.org@main/readthedocs/settings/docker_compose.py#L11 defaults to True if the env var is not set. This change explicitly sets RTD_DJANGO_DEBUG to a false-y value.

Interesting. I think we should change the docker_compose.py settings then to default to False when it's not defined.

Both these changes are related to a small RTD internal deployment at Arm, where we want to leave the instance unattended. I'm not asking for support with this (it's an internal-facing instance for private projects only)... but wondered if you'd be willing to include the extra parameter upstream to minimize patching.

Note that we explicitly say to not use this setup for production or custom installations in https://dev.readthedocs.io/en/latest/install.html

Take into account that this setup is intended for development purposes. We do not recommend to follow this guide to deploy an instance of Read the Docs for production.

and

https://github.com/readthedocs/readthedocs.org/blob/92afef7b9bc31849765d1f9488f6d7a55ee90207/dockerfiles/README.rst?plain=1#L1-L7

If you want to have private projects only, we strongly recommend you Read the Docs for Business. Read more about this at https://about.readthedocs.com/

[hoyes]

Interesting. I think we should change the docker_compose.py settings then to default to False when it's not defined.

Makes sense, I'll raise a PR over on that repo.

Note that we explicitly say to not use this setup for production or custom installations

Yea, I've read this but we've proceeded at risk due to some complex compliance requirements combined with a desire to use the same platform as we already use for public docs (where we already use RTD for Business). I'll discuss this some more internally to make sure we're doing the "right thing" still.

Would you accept a PR which allows arbitrary arguments to be forwarded to docker compose inv docker.up --arg="-e RTD_FOO=BAR" ?

[humitos]

Yea, I've read this but we've proceeded at risk due to some complex compliance requirements combined with a desire to use the same platform as we already use for public docs (where we already use RTD for Business). I'll discuss this some more internally to make sure we're doing the "right thing" still.

I would be interested in receiving this feedback if you able to share it 😄 . I strongly recommend you to not use our development environment for production purposes because it will be a nightmare to maintain. I'm sure about that. If you have doubts about Read the Docs for Business being a good fit for your company, you can always give it a try by signing up for our 30-day free trial and/or contact our support team to clarify your doubts.

Would you accept a PR which allows arbitrary arguments to be forwarded to docker compose inv docker.up --arg="-e RTD_FOO=BAR" ?

It depends. What would be the use case for this? I'm happy to add/accept any feature that's useful for the main purpose of this Docker environment, which is development. If we require this flexibility to help with the development of Read the Docs, I'm 👍🏼