Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(91018)

#20627: Add context manager support to xmlrpc.client.ServerProxy

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 6 months ago by brett
Modified:
5 years, 5 months ago
Reviewers:
merwok
CC:
brett.cannon, eric.araujo, jesstess, Claudiu.Popa, devnull_psf.upfronthosting.co.za, storchaka
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Total comments: 3

Patch Set 3 #

Total comments: 3

Patch Set 4 #

Patch Set 5 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/xmlrpc.client.rst View 1 2 3 4 3 chunks +14 lines, -9 lines 0 comments Download
Lib/test/test_xmlrpc.py View 1 2 3 4 2 chunks +18 lines, -0 lines 0 comments Download
Lib/xmlrpc/client.py View 1 2 3 4 1 chunk +6 lines, -0 lines 0 comments Download

Messages

Total messages: 2
eric.araujo
http://bugs.python.org/review/20627/diff/11230/Doc/library/xmlrpc.client.rst File Doc/library/xmlrpc.client.rst (right): http://bugs.python.org/review/20627/diff/11230/Doc/library/xmlrpc.client.rst#newcode196 Doc/library/xmlrpc.client.rst:196: The :class:`ServerProxy` can be used as a context manager ...
5 years, 5 months ago #1
eric.araujo
5 years, 5 months ago #2
http://bugs.python.org/review/20627/diff/11276/Doc/library/xmlrpc.client.rst
File Doc/library/xmlrpc.client.rst (right):

http://bugs.python.org/review/20627/diff/11276/Doc/library/xmlrpc.client.rst#...
Doc/library/xmlrpc.client.rst:194: .. versionadded:: 3.5
Should be versionchanged.

http://bugs.python.org/review/20627/diff/11276/Doc/library/xmlrpc.client.rst#...
Doc/library/xmlrpc.client.rst:197: for closing the underlying transport.
Instances can be used as context manager, not the class.

http://bugs.python.org/review/20627/diff/11276/Lib/test/test_xmlrpc.py
File Lib/test/test_xmlrpc.py (right):

http://bugs.python.org/review/20627/diff/11276/Lib/test/test_xmlrpc.py#newcod...
Lib/test/test_xmlrpc.py:728: except xmlrpclib.Fault:
I would put the except clause outside of the with block.  The reason is that
this test checks that the transport is closed even if an unhandled exception is
raised in the with block, so I think it’s better to have the exception raised
but not handled in the block.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+