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

fix sendfile option validation #1168

Merged
merged 1 commit into from
Dec 28, 2015
Merged

fix sendfile option validation #1168

merged 1 commit into from
Dec 28, 2015

Conversation

benoitc
Copy link
Owner

@benoitc benoitc commented Dec 28, 2015

sendfile option don't set any default for now which makes the validation fail
(and then tests) since no boolean is given. For now just return when the value
is not set.

benoitc referenced this pull request Dec 28, 2015
The --no-sendfile option had a confusing entry in the usage message.
Even though sendfile is enabled by default, the --no-sendfile flag
showed a true value as the default, which could be interpreted to
mean that by default sendfile support is disabled.

This change makes the default "None", meaning sendfile is not
disabled, which is hopefully slightly more clear.

Close #1156
@benoitc
Copy link
Owner Author

benoitc commented Dec 28, 2015

#1169 makes travis-ci return an OK. gaiohttp test is still broken on this branch

@benoitc
Copy link
Owner Author

benoitc commented Dec 28, 2015

@tilgovi oups pushed a new version, i missed to commit a part in the config.py on which I moved part of your previous diff. Let me know if it's ok for you.

@benoitc
Copy link
Owner Author

benoitc commented Dec 28, 2015

though maybe we should just keep your change instead of introducing a new property... Anyway let me know :)

sendfile option don't set any default for now which makes the validation fail
(and then tests) since no boolean is given. For now just return when the value
is not set.
@benoitc
Copy link
Owner Author

benoitc commented Dec 28, 2015

nvm I'm tired :) pushed the third version just fixing the test.

@tilgovi
Copy link
Collaborator

tilgovi commented Dec 28, 2015

Okay :-). We can do more another time.

On Mon, Dec 28, 2015, 11:41 Benoit Chesneau notifications@github.com
wrote:

nvm I'm tired :) pushed the third version just fixing the test.


Reply to this email directly or view it on GitHub
#1168 (comment).

@benoitc
Copy link
Owner Author

benoitc commented Dec 28, 2015

ok :) merging it. will make the release asap. thanks for the review!

benoitc added a commit that referenced this pull request Dec 28, 2015
@benoitc benoitc merged commit 960d5ef into master Dec 28, 2015
mjjbell pushed a commit to mjjbell/gunicorn that referenced this pull request Mar 16, 2018
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