Skip to content
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

Make supervisord log level configurable via env #308

Closed
wants to merge 2 commits into from

Conversation

w4tsn
Copy link
Contributor

@w4tsn w4tsn commented Jun 10, 2021

The supervisord log level controls the containers log output regardless
of the DEBUG=0|1 flag or other nginx log level settings, thus should be
configurable.

IMO I'd set log level to warn by default to supress access logs which
is desired for most production deployments I suppose. That really depends
on the main use-case for this image though. If it's for development setups,
then of course debug is totally fine. Also debug won't brake existing
behavior.

DOCS: the docs need an update to indicate that this can be controlled
for docker containers. While we are at it I'd also add the other options
of hipervisord that can be controlled this way.

The supervisord log level controls the containers log output regardless
of the DEBUG=0|1 flag or other nginx log level settings, thus should be
configurable.

IMO I'd set log level to warn by default to supress access logs which
is desired for most production deployments I suppose. That really depends
on the main use-case for this image though. If it's for development setups,
then of course debug is totally fine. Also debug won't brake existing
behavior.

DOCS: the docs need an update to indicate that this can be controlled
for docker containers. While we are at it I'd also add the other options
of hipervisord that can be controlled this way.
@w4tsn w4tsn force-pushed the feature/supervisord-log-level-env branch from 568dea3 to c85a3c2 Compare June 10, 2021 14:57
@satterly
Copy link
Member

LGTM. Thanks! 👍

@satterly
Copy link
Member

satterly commented Dec 3, 2021

Thanks for the PR but will prefer #349 as the config setup has changed since this PR was submitted.

@satterly satterly closed this Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants