bpo-33540: Add block_on_close attr to socketserver#6911
Conversation
Add a new block_on_close class attribute to ForkingMixIn and ThreadingMixIn classes of socketserver.
|
I rebased my PR. |
ned-deily
left a comment
ned-deily
left a comment
There was a problem hiding this comment.
A few wording nits. Otherwise, LGTM
| :meth:`socketserver.ThreadingMixIn.server_close` waits until all non-daemon | ||
| threads complete. Use daemonic threads by setting | ||
| threads complete, except if | ||
| :attr:`socketserver.ThreadingMixIn.block_on_close` attribute if false. Use |
There was a problem hiding this comment.
should be "attribute is false"
|
|
||
| Add a new :attr:`socketserver.ForkingMixIn.block_on_close` class | ||
| attribute to opt-in for the old behaviour. | ||
|
|
There was a problem hiding this comment.
It might read better it the addition were not a separate paragraph. How about removing the blank line and rewording a bit, perhaps:
If necessary, set the new :attr:socketserver.ForkingMixIn.block_on_close class
attribute to False to obtain the pre-3.7 behavior.
There was a problem hiding this comment.
Your proposed sentence doesn't explicitly say that the attribute is new in 3.7. I removed the empty line.
|
|
||
| Add a new :attr:`socketserver.ForkingMixIn.block_on_close` class attribute to | ||
| :class:`socketserver.ForkingMixIn` and :class:`socketserver.ThreadingMixIn` | ||
| classes. Set the class attribute to ``False`` to get the old behaviour. |
| * :meth:`socketserver.ThreadingMixIn.server_close` now waits until all | ||
| non-daemon threads complete. Set the new | ||
| :attr:`socketserver.ThreadingMixIn.block_on_close` class attribute to | ||
| ``False`` to get the old behaviour. |
| * :meth:`socketserver.ForkingMixIn.server_close` now waits until all | ||
| child processes complete. Set the new | ||
| :attr:`socketserver.ForkingMixIn.block_on_close` class attribute to ``False`` | ||
| to get the old behaviour. |
|
When you're done making the requested changes, leave the comment: |
* old => pre-3.7 * remove an empty line
|
I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @ned-deily: please review the changes made to this pull request. |
|
Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 2.7. |
|
Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7. |
|
Sorry, @vstinner, I could not cleanly backport this to |
Add a new block_on_close class attribute to ForkingMixIn and ThreadingMixIn classes of socketserver to opt-in for pre-3.7 behaviour. (cherry picked from commit 453bd0b) Co-authored-by: Victor Stinner <vstinner@redhat.com>
|
GH-7088 is a backport of this pull request to the 3.7 branch. |
5 participants
Add a new block_on_close class attribute to ForkingMixIn and
ThreadingMixIn classes of socketserver.
https://bugs.python.org/issue33540