search

cloudfoundry / python-buildpack

Cloud Foundry buildpack for the Python Language
http://docs.cloudfoundry.org/buildpacks/
Apache License 2.0
121 stars 279 forks source link

Variable GUNICORN_CMD_ARGS set to '--access-logfile -' has precedence to command line args #114

Closed keymon closed 5 years ago

keymon commented 6 years ago

Summary

I am reporting this on behalf of one of our development teams.

In https://github.com/cloudfoundry/python-buildpack/blob/8813afed0be323fc7670ac43aaaaf94528660bc1/src/python/supply/supply.go#L557 there is a default value for GUNICORN_CMD_ARGS set, with export GUNICORN_CMD_ARGS=${GUNICORN_CMD_ARGS:-'--access-logfile -'}.

But this variable GUNICORN_CMD_ARGS which has the highest level of precedence when setting the config: (env > cmd line args > config)

Because that the setting could not be changed until you either:

This breaks the expectations of the developers and violates the least astonishment principle, as passing some arguments to the command just does not work.

We think this could be implemented in a slightly different manner or documented in the documentation of the buildpack documentation.

What version of the buildpack you are using?

python-buildpack-v1.6.17.zip

If you were attempting to accomplish a task, what was it you were attempting to do?

Disable access logs by passing --access-logfile as argument

What did you expect to happen?

My --access-logfile to work

What was the actual behavior?

It does not work, it keeps sending the logs to stdout until you unset the variable GUNICORN_CMD_ARGS

cf-gitbot commented 6 years ago

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/158720707

The labels on this github issue will be updated when the story is started.

jfeeny commented 6 years ago

@keymon Having access logs appear in your app logs (by writing to stdout) seems like a good default to us. I agree, we should document that we set --access-logfile to - by default and instruct how to override it. @sclevine Can we create and prioritize a story for this?

As far as the precedence issue, I don't see you attempting to set the --access-logfile in your command. It seems that gunicorn simply unions the args from your command with those in GUNICORN_CMD_ARGS, but according to gunicorn docs, the command-line args will take precedence if an argument appears in both.

Can you try adding --access-logfile None and let us know if it works? None looks to be the default for this argument, according to gunicorn docs.

dfreilich commented 5 years ago

Closing this due to a lack of activity. If it's still an issue, let us know and we'll prioritize it.