This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Make subclassing SocketServer simpler for non-blocking frameworks
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.4
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: giampaolo.rodola, kristjan.jonsson, martin.panter, pitrou, vstinner
Priority: normal Keywords: patch

Created on 2012-03-14 23:22 by kristjan.jonsson, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
75646.patch kristjan.jonsson, 2012-03-14 23:22 delegate timeout to _get_request_timeout() review
75647.patch kristjan.jonsson, 2012-03-14 23:50 illustrating timeout without socket.socket.
Messages (9)
msg155816 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2012-03-14 23:22
SocketServer employs select.select to achieve timeout behaviour on accepting new requests.  Unfortunately, the way this is implemented makes it tricky to bypass this behaviour for users that want to achieve timeout in a different manner.
This defect presents two patches.  One shows how we delegate the timeout to a new function, _get_request_timeout(), that can be overridden by subclasses.
The second shows a concrete version of this function, one that uses the socket's own timeout property rather than select, to get the same behaviour.
msg155876 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-03-15 11:37
As discussed on python-dev, I'm against this change. Furthermore, removing the explicit select() call also removes the opportunity to use a wake-up fd instead of the current polling mechanism.
msg156584 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2012-03-22 16:09
What's wrong with making functionality overridable, Antoine?  By all means, let's keep the select() but allow subclasses to elect not to use it.

As for the wakeup fd, there isn't such a thing.  Why object on the basis of a hypothetical feature which might be added later?  And should it be added later, it would require changes to SocketServer anyway.  Why not arrange it so that such functionality can be added in an extensible manner?
msg156604 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-03-22 18:51
> What's wrong with making functionality overridable, Antoine?  By all
> means, let's keep the select() but allow subclasses to elect not to use 
> it.

It's making the implementation more complex for no obvious point.
Using select() is not a bug in itself.
Furthermore, you can simply reimplement serve_forever() in your subclass, if you really want it.
msg160601 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2012-05-14 10:03
Btw, I have found that socket.closesocket() is quite able, on Windows, to abort an ongoing accept() call.
msg259975 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-02-10 04:48
FTR the python-dev discussion looks like it is <https://marc.info/?l=python-dev&m=133169928819705&w=2>. The argument for this change seems to be to support users monkey-patching the socket module, but not monkey-patching the select module. But this seems unreasonable to me.

I also agree the would make it harder to fix the wakeup polling problem. I suggest to reject this change.

Also, if you want a method to be overridable in subclasses, I don’t think it should have an underscore prefix.
msg354335 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-10-10 08:18
Antoine Pitrou: "...  I'm against this change ..."

Martin Panter: "... But this seems unreasonable to me. ..."

I close the issue. You can use asyncio for non-blocking networking code. Or have a look at Twisted, Tornado, eventlet, etc.

If you really want to hack socketserver, I suggest you to copy the whole socketserver.py file. But asynchronous programming is a hard problem which requires more than modiying a single function. Slowly, you will built something similar the the project that I cited.
msg354368 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2019-10-10 12:06
Nice necro :)
Socketserver is already subclassable and overridable for so many things.  Hard to understand the reluctancy to _allow_ for a different way to handle accept timeouts.  But this is also why I stopped contributing to core, because it turned out to be more about lobbying than anything else.  Anyway this is already implemented in PythonClassic, so no need for me to push it upstream :)
msg354373 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-10-10 13:49
> Hard to understand the reluctancy to _allow_ for a different way to handle accept timeouts.  But this is also why I stopped contributing to core, because it turned out to be more about lobbying than anything else.

To get a change merged into Python, you need to find someone who wants to be responsible for it: not only merge the change, but also maintain the code later. I touched socketserver recently for my work on making the Python CI more reliable. It was not easy to get a review. I ended to merge the changes with no review (if I recall properly).

I dislike socketserver design: it tries to be too generic. Like supporting TCP and UDP, support threading and forking models, etc.

To be honest, I didn't look at your issue. I only close it for bug triage, since there was no activity since 2016 and 2 core devs were against the change.

And you want to revisit the change, please reopen the issue.
History
Date User Action Args
2022-04-11 14:57:28adminsetgithub: 58515
2019-10-10 13:49:55vstinnersetmessages: + msg354373
2019-10-10 12:06:26kristjan.jonssonsetmessages: + msg354368
2019-10-10 08:18:30vstinnersetstatus: open -> closed

nosy: + vstinner
messages: + msg354335

resolution: rejected
stage: patch review -> resolved
2016-02-10 04:48:13martin.pantersetmessages: + msg259975
components: + Library (Lib)
stage: patch review
2015-01-17 03:40:19martin.pantersetnosy: + martin.panter
2012-05-14 10:03:57kristjan.jonssonsetmessages: + msg160601
2012-03-22 18:51:15pitrousetmessages: + msg156604
2012-03-22 16:09:42kristjan.jonssonsetmessages: + msg156584
2012-03-15 11:37:53pitrousetnosy: + pitrou
messages: + msg155876
2012-03-15 09:34:31giampaolo.rodolasetnosy: + giampaolo.rodola
2012-03-14 23:50:22kristjan.jonssonsetfiles: + 75647.patch
2012-03-14 23:49:52kristjan.jonssonsetfiles: - 75647.patch
2012-03-14 23:24:14kristjan.jonssonsetfiles: + 75647.patch
2012-03-14 23:22:58kristjan.jonssoncreate