-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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 SO_REUSEPORT Optional #1669
Conversation
I am slightly in favor of a No strong opinion on how to test this. |
I agree with you about the default being |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great to me. Thank you for opening a PR. I'll give others a chance to weigh in on whether we want to test this at all, otherwise I'll merge in the next day or so.
Thanks. I have updated the documentation. |
reuse_port | ||
~~~~~~~~~~ | ||
|
||
* ``--reuse-port`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation for the new option will be generated automatically from gunicorn/config.py
when you run make html
in docs/
directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand your comment, @berkerpeksag. I generated this change by running make -C docs html
. Am I not supposed to commit settings.rst?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought you manually edited it because I didn't see the "Set the SO_REUSEPORT
flag on the listening socket." part in the generated file. Now I don't know why make html
didn't get the documentation in the desc
attribute.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I generated it, but then edited it because running it also brought in logconfig_dict
(which presumably was omitted previously and should be in its own PR), and also changed chdir
, user
and group
. I must have got the edit wrong. Sorry about that. It's all there now.
gunicorn/config.py
Outdated
default = False | ||
|
||
desc = """\ | ||
Set the ``SO_REUSEPORT`` flag on the listening socket. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add .. versionadded:: 19.8
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I always forget this and you always catch it. Thanks! :-D
The cause of build failure is prospector-dev/prospector#200. All tests passed and I don't think we need a test case for this since testing the actual change reliably would be hard. |
Thanks! |
I'm open to opinions as to whether
SO_REUSEPORT
should be the default or not.Also, would appreciate some advice about unit testing this change.