-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docker.up: Fix --no-django-debug behavior and introduce --detach #202
base: main
Are you sure you want to change the base?
Conversation
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`. Signed-off-by: Diego Sueiro <diego.sueiro@arm.com> Signed-off-by: Peter Hoyes <Peter.Hoyes@arm.com>
This makes it simple to start the application in detached/daemon mode without requiring a terminal to output to. Signed-off-by: Ryan Roberts <ryan.roberts@arm.com> Signed-off-by: Diego Sueiro <diego.sueiro@arm.com> Signed-off-by: Peter Hoyes <Peter.Hoyes@arm.com>
Thanks for your contribution. I have a few questions before moving forward with this PR.
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.
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? |
Thanks for your response.
Currently
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. |
Interesting. I think we should change the
Note that we explicitly say to not use this setup for production or custom installations in https://dev.readthedocs.io/en/latest/install.html
and 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/ |
Makes sense, I'll raise a PR over on that repo.
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 |
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.
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 👍🏼 |
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.