classification
Title: Add a context manager to telnetlib.Telnet
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: SilentGhost, berker.peksag, desbma, matrixise, python-dev, r.david.murray
Priority: normal Keywords: patch

Created on 2015-10-26 18:20 by desbma, last changed 2015-11-28 17:27 by r.david.murray. This issue is now closed.

Files
File name Uploaded Description Edit
issue25485.patch matrixise, 2015-10-28 20:58 review
issue25485-2.patch matrixise, 2015-10-30 15:24 review
issue25485-3.patch matrixise, 2015-11-21 00:06 review
issue25485-4.patch matrixise, 2015-11-22 06:41 review
Messages (16)
msg253489 - (view) Author: desbma (desbma) * Date: 2015-10-26 18:20
telnetlib.Telnet could have a context manager to call close() automatically.

I can provide a patch if the idea is approved, although I have never contributed to Python.
msg253597 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2015-10-28 12:10
This seems like a reasonable request to me.

See https://docs.python.org/devguide/ to learn how to contribute to Python.
msg253629 - (view) Author: Stéphane Wirtel (matrixise) * Date: 2015-10-28 20:58
Here is a small patch
msg253631 - (view) Author: Stéphane Wirtel (matrixise) * Date: 2015-10-28 21:04
Maybe, modify the documentation or improve the current examples.
msg253632 - (view) Author: SilentGhost (SilentGhost) * Date: 2015-10-28 21:09
This probably needs test
msg253633 - (view) Author: Stéphane Wirtel (matrixise) * Date: 2015-10-28 21:14
totally agree, I will work on this issue tomorrow
msg253634 - (view) Author: desbma (desbma) * Date: 2015-10-28 21:20
I was actually writing a patch with a test, but since Stéphane beat me to it, I'll let him do the job :)
msg253737 - (view) Author: Stéphane Wirtel (matrixise) * Date: 2015-10-30 15:24
New version of the patch, I have modified the 'test' function. In this function, I use the with statement with the __enter__ and __exit__.
msg253739 - (view) Author: SilentGhost (SilentGhost) * Date: 2015-10-30 15:37
That's fine as a code change, but that's not the kind of test I meant. There is a Lib/test/test_ftplib.py and it basically needs test_with_statement added to one or few test cases. Have a look at how the similar functions implemented in Lib/test/.
msg253753 - (view) Author: Stéphane Wirtel (matrixise) * Date: 2015-10-30 19:42
Ok i will submit an other patch for the tests and the documentation

> On 30 oct. 2015, at 4:37 PM, SilentGhost <report@bugs.python.org> wrote:
> 
> 
> SilentGhost added the comment:
> 
> That's fine as a code change, but that's not the kind of test I meant. There is a Lib/test/test_ftplib.py and it basically needs test_with_statement added to one or few test cases. Have a look at how the similar functions implemented in Lib/test/.
> 
> ----------
> 
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue25485>
> _______________________________________
msg254813 - (view) Author: Stéphane Wirtel (matrixise) * Date: 2015-11-17 18:58
Should I develop a mock telnet server for the unittest ?
msg254814 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-11-17 19:52
The existing tests seem to have at least some infrastructure that would apply here, so figuring out how to use/extend that is better than writing new code, I'd say.
msg255031 - (view) Author: Stéphane Wirtel (matrixise) * Date: 2015-11-21 00:06
Need review for this new patch.

* Add test
* Improve the documentation
* Update the Misc/NEWS
msg255083 - (view) Author: Stéphane Wirtel (matrixise) * Date: 2015-11-22 06:41
I applied the recommendation of SilentGhost for this issue.

Need review

Thank you
msg255547 - (view) Author: Roundup Robot (python-dev) Date: 2015-11-28 17:25
New changeset 277824f2d133 by R David Murray in branch 'default':
#25485: Add context manager support to Telnet class.
https://hg.python.org/cpython/rev/277824f2d133
msg255548 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-11-28 17:27
Thanks, Stéphane.  I adjusted the docs...we don't seem to be very consistent about how we document addition of content manager support, so I picked what I liked best.  Also, FYI it is generally best (at this point in time) to not include the NEWS item, but enhancements should inlcude a what's new entry (I added one).
History
Date User Action Args
2015-11-28 17:27:24r.david.murraysetstatus: open -> closed
resolution: fixed
messages: + msg255548

stage: needs patch -> resolved
2015-11-28 17:25:14python-devsetnosy: + python-dev
messages: + msg255547
2015-11-22 06:41:37matrixisesetfiles: + issue25485-4.patch

messages: + msg255083
2015-11-21 00:06:51matrixisesetfiles: + issue25485-3.patch

messages: + msg255031
2015-11-17 19:52:56r.david.murraysetnosy: + r.david.murray
messages: + msg254814
2015-11-17 18:58:28matrixisesetmessages: + msg254813
2015-10-30 19:42:25matrixisesetmessages: + msg253753
2015-10-30 15:37:31SilentGhostsetmessages: + msg253739
2015-10-30 15:24:14matrixisesetfiles: + issue25485-2.patch

messages: + msg253737
2015-10-28 21:20:25desbmasetmessages: + msg253634
2015-10-28 21:14:44matrixisesetmessages: + msg253633
2015-10-28 21:09:39SilentGhostsetnosy: + SilentGhost
messages: + msg253632
2015-10-28 21:04:23matrixisesetmessages: + msg253631
2015-10-28 20:58:29matrixisesetfiles: + issue25485.patch

nosy: + matrixise
messages: + msg253629

keywords: + patch
2015-10-28 12:10:13berker.peksagsetnosy: + berker.peksag

messages: + msg253597
stage: needs patch
2015-10-26 18:20:19desbmacreate