[issue26404] socketserver context manager

[Martin Panter]

Martin Panter added the comment:

Thanks for the patch Aviv. I made a few minor English grammar etc tweaks in the 
version I committed, as pointed out in the review comments.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26404] socketserver context manager

[Roundup Robot]

Roundup Robot added the comment:

New changeset 5c4303c46a18 by Martin Panter in branch 'default':
Issue #26404: Add context manager to socketserver, by Aviv Palivoda
https://hg.python.org/cpython/rev/5c4303c46a18

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26404] socketserver context manager

[Terry J. Reedy]

Terry J. Reedy added the comment:

I agree and consider this is an argument for adding the context manager methods 
and using them with 'with' in the examples.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26404] socketserver context manager

[Martin Panter]

Martin Panter added the comment:

Server_close() was only documentated last year; see Issue 23254. For the 
examples that run until you interrupt them, the servers currently emit a 
resource warning (in addition to the KeyboardInterrupt traceback and the Python 
2 bytes warnings):

$ python -bWall TCPServer.py
127.0.0.1 wrote:
TCPServer.py:16: BytesWarning: str() on a bytes instance
  print(self.data)
b'hello world with TCP'
127.0.0.1 wrote:
TCPServer.py:16: BytesWarning: str() on a bytes instance
  print(self.data)
b'python is nice'
^CTraceback (most recent call last):
  File "TCPServer.py", line 28, in 
server.serve_forever()
  File "/usr/lib/python3.5/socketserver.py", line 237, in serve_forever
ready = selector.select(poll_interval)
  File "/usr/lib/python3.5/selectors.py", line 367, in select
fd_event_list = self._poll.poll(timeout)
KeyboardInterrupt
sys:1: ResourceWarning: unclosed 
[Exit 1]

If you ignore the warning, there isn’t much effective difference, because the 
socket gets closed when the process exits, or if you are lucky, when Python 
garbage collects the global “server” object. But IMO it is bad practice not to 
clean up resources properly, especially in an API example.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26404] socketserver context manager

[Aviv Palivoda]

Aviv Palivoda added the comment:

The examples did not close the server but I am not sure if that was a mistake. 
The server is not being closed only on examples where it is expected to run 
until there is a keyboard interrupt. However you can see that when you send 
keyboard interrupt to a server you start using python -m http.server you will 
do close the server.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26404] socketserver context manager

[Terry J. Reedy]

Terry J. Reedy added the comment:

This seems like an appropriate enhancement.  I notice that the serve_forever 
examples did not previously have server_close().  Was this an oversight or will 
the server_close in __exit__ just be a no-op?

--
nosy: +terry.reedy

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26404] socketserver context manager

[Aviv Palivoda]

Aviv Palivoda added the comment:

updated the patch according to the CR comments.

--
Added file: http://bugs.python.org/file42030/socketserver_context_manager4.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26404] socketserver context manager

[Aviv Palivoda]

Aviv Palivoda added the comment:

Updated the patch:
1. Did the changes requested in the CR.
2. Changed the example's in wsgiref.simple_server to use context manager.
3. Added What's New entry.

--
Added file: http://bugs.python.org/file42016/socketserver_context_manager3.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26404] socketserver context manager

[Martin Panter]

Martin Panter added the comment:

Thanks, the patch looks pretty good to me. I left some comments about minor 
things. Also, someone will need to write a What’s New entry.

--
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26404] socketserver context manager

[Aviv Palivoda]

Aviv Palivoda added the comment:

Only closing the server :).
1. Did the changes requested in the CR.
2. Changed the example's in xmlrpc.server, http.server to use context manager.
3. Changed the xmlrpc.server, http.server server implementation when running 
python -m {xmlrpc.server, http.server} to use context manager.

--
Added file: http://bugs.python.org/file42008/socketserver_context_manager2.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26404] socketserver context manager

[Martin Panter]

Martin Panter added the comment:

I would support this change, as long as we aren’t supporting using 
server_close() to stop the server loop at the same time.

It might be worth updating other example code that uses server_close(), in 
particular in the xmlrpc.server documentation.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26404] socketserver context manager

[Aviv Palivoda]

New submission from Aviv Palivoda:

As Martin commented on my patch at issue 26392 the socketserver.server_close is 
like the file close. That made me think that we should add a context manager to 
the socketserver.

--
components: Library (Lib)
files: socketserver_context_manager.patch
keywords: patch
messages: 260639
nosy: martin.panter, palaviv
priority: normal
severity: normal
status: open
title: socketserver context manager
type: enhancement
versions: Python 3.6
Added file: http://bugs.python.org/file41993/socketserver_context_manager.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com