Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add a context manager to telnetlib.Telnet #69671

Closed
desbma mannequin opened this issue Oct 26, 2015 · 16 comments
Closed

Add a context manager to telnetlib.Telnet #69671

desbma mannequin opened this issue Oct 26, 2015 · 16 comments
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@desbma
Copy link
Mannequin

desbma mannequin commented Oct 26, 2015

BPO 25485
Nosy @bitdancer, @berkerpeksag, @desbma, @matrixise
Files
  • issue25485.patch
  • issue25485-2.patch
  • issue25485-3.patch
  • issue25485-4.patch
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2015-11-28.17:27:24.660>
created_at = <Date 2015-10-26.18:20:19.145>
labels = ['type-feature', 'library']
title = 'Add a context manager to telnetlib.Telnet'
updated_at = <Date 2015-11-28.17:27:24.658>
user = 'https://github.com/desbma'

    bugs.python.org fields:

    activity = <Date 2015-11-28.17:27:24.658>
actor = 'r.david.murray'
assignee = 'none'
closed = True
closed_date = <Date 2015-11-28.17:27:24.660>
closer = 'r.david.murray'
components = ['Library (Lib)']
creation = <Date 2015-10-26.18:20:19.145>
creator = 'desbma'
dependencies = []
files = ['40879', '40902', '41106', '41119']
hgrepos = []
issue_num = 25485
keywords = ['patch']
message_count = 16.0
messages = ['253489', '253597', '253629', '253631', '253632', '253633', '253634', '253737', '253739', '253753', '254813', '254814', '255031', '255083', '255547', '255548']
nosy_count = 6.0
nosy_names = ['r.david.murray', 'SilentGhost', 'python-dev', 'berker.peksag', 'desbma', 'matrixise']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue25485'
versions = ['Python 3.6']
    @desbma
    Copy link
    Mannequin Author

    desbma mannequin commented Oct 26, 2015

    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.

    @desbma desbma mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Oct 26, 2015
    @berkerpeksag
    Copy link
    Member

    This seems like a reasonable request to me.

    See https://docs.python.org/devguide/ to learn how to contribute to Python.

    @matrixise
    Copy link
    Member

    Here is a small patch

    @matrixise
    Copy link
    Member

    Maybe, modify the documentation or improve the current examples.

    @SilentGhost
    Copy link
    Mannequin

    SilentGhost mannequin commented Oct 28, 2015

    This probably needs test

    @matrixise
    Copy link
    Member

    totally agree, I will work on this issue tomorrow

    @desbma
    Copy link
    Mannequin Author

    desbma mannequin commented Oct 28, 2015

    I was actually writing a patch with a test, but since Stéphane beat me to it, I'll let him do the job :)

    @matrixise
    Copy link
    Member

    New version of the patch, I have modified the 'test' function. In this function, I use the with statement with the __enter__ and __exit__.

    @SilentGhost
    Copy link
    Mannequin

    SilentGhost mannequin commented Oct 30, 2015

    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/.

    @matrixise
    Copy link
    Member

    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\>

    @matrixise
    Copy link
    Member

    Should I develop a mock telnet server for the unittest ?

    @bitdancer
    Copy link
    Member

    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.

    @matrixise
    Copy link
    Member

    Need review for this new patch.

    • Add test
    • Improve the documentation
    • Update the Misc/NEWS

    @matrixise
    Copy link
    Member

    I applied the recommendation of SilentGhost for this issue.

    Need review

    Thank you

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 28, 2015

    New changeset 277824f2d133 by R David Murray in branch 'default':
    bpo-25485: Add context manager support to Telnet class.
    https://hg.python.org/cpython/rev/277824f2d133

    @bitdancer
    Copy link
    Member

    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).

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @berkerpeksag @matrixise @bitdancer