Allow passing of sockets for socket activation #215
Conversation
…on if binding the socket should be skipped.
|
@Frank-Krick there's one line that lacks coverage. Please fix that. |
|
For maintainers, I noticed this statement:
Should I make a new issue for that? |
|
@Frank-Krick Please sign CONTRIBUTORS.txt and push that commit to this PR. |
stevepiercy
left a comment
stevepiercy
left a comment
There was a problem hiding this comment.
My change requests are concern docs and error messages.
Make sure you sign CONTRIBUTORS.txt, too.
|
Hi @stevepiercy, I'm done with the requested changes. Btw. in adjustments.py, line 231, there is still the "host and or port" in the error message related to listen and host, port. Let me know if I should change that in this pull request too, for the sake of consistency. Thanks, Frank |
Yes, please! Thank you! |
|
Done, thanks. |
stevepiercy
left a comment
stevepiercy
left a comment
There was a problem hiding this comment.
Thank you! The core maintainers will review when they are available.
|
Thanks very much for this work! I'm currently under the weather with a cold, but will get to this asap to review it and give you feedback and or accept it! |
|
You're welcome and get well soon. |
|
If you could roll #217 into this as well, that would be great! |
|
I'll do the changes to the documentation and roll in #217. Please have a look at my comment above regarding the use of different socket types (UNIX vs INET). |
No. It's not something I care to fix. |
…s well as the use of unsupported sockets
|
Hi @bertjwregeer, I added the check to prevent mixing of UNIX and INET sockets. The same check also throws an error if the socket has a type other than Stream or a family other than INET, INET6 or UNIX. This should address all your review comments. Thanks, Frank |
|
Frank, thank you very much! Last but not least, we need some defensive coding in |
|
I added a check for using hasattr(socket, 'AF_UNIX') to make the test case work on Windows. That means that there will be a line of code not covered using the unit tests on Windows. Will that create problems during the test runs? |
|
No, we don't check coverage on Windows, only on MacOS (where I develop) and Linux (Where Travis runs). Looks like we are good now. I'll give it one last once over tomorrow and then get it merged :-)! |
stevepiercy
left a comment
stevepiercy
left a comment
There was a problem hiding this comment.
Couple of minor docs change requests.
|
@stevepiercy I've done the changes. Thanks |
|
@Frank-Krick thanks very much! |
| from init systems, process and socket managers, or other launchers that can provide | ||
| pre-bound sockets. | ||
|
|
||
| The following shows a code example starting waitress with two Internet sockets pre-bound sockets. |
There was a problem hiding this comment.
This sentence does not make sense. I hoped that merging the two lines into one would have been cleaned up by removing the extra "sockets". I'll submit a quick PR to fix it.
3 participants
As discussed in issue #204, the pull request adds the new parameter
socketstoserveandcreate_serverwhich can be used to create waitress instances using already existing and bound sockets, e.g. for socket activation with systemd or launchd.The changes are tested with several additional test cases.
In addition to the code changes, it adds a new page to the documentation and a new entry to describe the new parameter
socketsin the page for arguments. Adding the entry in arguments is done under the assumption that the next version will be 1.1.1 to supply the version number forversionadded.The changes only add the parameter, they do not handle init system specific tasks like creating sockets from file descriptors because we want to avoid dependencies to init systems.
Closes #204
This change is