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

Allow disabling sendfile via an environment variable #1252

Closed
edmorley opened this Issue May 3, 2016 · 3 comments

Comments

Projects
None yet
3 participants
@edmorley
Contributor

edmorley commented May 3, 2016

Virtualbox has a bug where when using shared folders, the contents of a file transferred using sendfile can sometimes end up corrupted. A ticket for this issue has been open against Virtualbox for several years, but it still remains unfixed:
https://www.virtualbox.org/ticket/9069
https://www.virtualbox.org/ticket/12597

As a result, a pref was added to gunicorn to allow disabling:
http://docs.gunicorn.org/en/latest/settings.html#sendfile

However since --no-sendfile should only be used in local development environments, this means our in-repository wrapper scripts that invoke gunicorn need to be different for dev vs prod.

To save having to add further complexity to these scripts (and to also help the case where someone invokes gunicorn directly rather than using the scripts), it would be great if a GUNICORN_DISABLE_SENDFILE environment variable (or similar) could be used instead. We'd then just define this only in the local Vagrant development environment's puppet scripts, leaving prod unaffected.

If this is something that would be accepted, I'd be happy to open a PR. (I realise we can't just add environment variables for every single supported setting, but wasn't sure where the line lay, given things like #1205 were accepted.)

@benoitc

This comment has been minimized.

Show comment
Hide comment
@benoitc

benoitc May 10, 2016

Owner

Are there already servesr that does something like it? In that case we may look at the environment variable they are using instead of using our own. Imo having smth "SENDFILE=no" would be better. thoughts?

Owner

benoitc commented May 10, 2016

Are there already servesr that does something like it? In that case we may look at the environment variable they are using instead of using our own. Imo having smth "SENDFILE=no" would be better. thoughts?

@tilgovi

This comment has been minimized.

Show comment
Hide comment
@tilgovi

tilgovi May 10, 2016

Collaborator

A cursory search does not reveal a typical environment variable for this purpose in other server software. I would be fine with SENDFILE, though. We should be liberal about the value: 0, 1, yes, no, true, false, case insensitive?

Collaborator

tilgovi commented May 10, 2016

A cursory search does not reveal a typical environment variable for this purpose in other server software. I would be fine with SENDFILE, though. We should be liberal about the value: 0, 1, yes, no, true, false, case insensitive?

@benoitc benoitc closed this in d5a07ce May 21, 2016

edmorley added a commit to edmorley/gunicorn that referenced this issue May 23, 2016

Docs: Spelling corrections & markup tweaks
Fixes the changelog bullets added for #1252, as well as a number of
spelling corrections and markup improvements.

edmorley added a commit to edmorley/gunicorn that referenced this issue May 23, 2016

Docs: Spelling corrections & markup tweaks
Fixes the changelog bullets added for #1252, as well as a number of
spelling corrections and markup improvements.
@edmorley

This comment has been minimized.

Show comment
Hide comment
@edmorley

edmorley May 23, 2016

Contributor

Thank you for adding this! :-)

Contributor

edmorley commented May 23, 2016

Thank you for adding this! :-)

fofanov pushed a commit to fofanov/gunicorn that referenced this issue Mar 16, 2018

Allow disabling sendfile via an environment variable
add support for the `SENDFILE` environment variable.

fix #1252

fofanov pushed a commit to fofanov/gunicorn that referenced this issue Mar 16, 2018

Docs: Spelling corrections & markup tweaks
Fixes the changelog bullets added for #1252, as well as a number of
spelling corrections and markup improvements.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment