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

HTTPResponse is missing an implementation of readinto #57673

Closed
bitdancer opened this issue Nov 23, 2011 · 7 comments
Closed

HTTPResponse is missing an implementation of readinto #57673

bitdancer opened this issue Nov 23, 2011 · 7 comments
Labels
easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@bitdancer
Copy link
Member

BPO 13464
Nosy @orsenthil, @pitrou, @bitdancer, @fbidu
PRs
  • bpo-42060: Remove assert from http/client.py #22737
    		•
    Files
  • issue13464.patch: Patch based on 06087f6890af
  • issue13464_r1.patch: Updated 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 2011-12-06.21:41:39.277>
created_at = <Date 2011-11-23.18:13:58.798>
labels = ['easy', 'type-feature', 'library']
title = 'HTTPResponse is missing an implementation of readinto'
updated_at = <Date 2020-10-23.19:09:49.240>
user = 'https://github.com/bitdancer'

    bugs.python.org fields:

    activity = <Date 2020-10-23.19:09:49.240>
actor = 'fbidu'
assignee = 'none'
closed = True
closed_date = <Date 2011-12-06.21:41:39.277>
closer = 'pitrou'
components = ['Library (Lib)']
creation = <Date 2011-11-23.18:13:58.798>
creator = 'r.david.murray'
dependencies = []
files = ['23847', '23850']
hgrepos = []
issue_num = 13464
keywords = ['patch', 'easy']
message_count = 7.0
messages = ['148199', '148837', '148846', '148852', '148939', '148940', '150627']
nosy_count = 6.0
nosy_names = ['orsenthil', 'pitrou', 'r.david.murray', 'python-dev', 'Jon.Kuhn', 'fbidu']
pr_nums = ['22737']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue13464'
versions = ['Python 3.3']
    @bitdancer
    Copy link
    Member Author

    HTTPResponse subclasses RawIOBase, but does not provide an implementation of readinto, only read. This means that it is not conforming to the IO spec, and so it cannot be wrapped in a BufferedIOBase when using the C version of io.

    @bitdancer bitdancer added type-bug An unexpected behavior, bug, or error stdlib Python modules in the Lib dir easy labels Nov 23, 2011
    @JonKuhn
    Copy link
    Mannequin

    JonKuhn mannequin commented Dec 4, 2011

    This is my first contribution to a real open source project.

    Attached is my patch. Suggestions for improvements are welcome.

    @pitrou
    Copy link
    Member

    pitrou commented Dec 4, 2011

    Hello Jon, and thanks for the patch. I have a couple of comments:

    • readinto() shouldn't return None but 0 when there is nothing to read (this corresponds to read() returning b"")

    • I see _read_chunked() is only ever called with amt=None, so perhaps it can be simplified?

    Also, a nitpick: the doc entry needs a "versionadded" tag.

    @JonKuhn
    Copy link
    Mannequin

    JonKuhn mannequin commented Dec 4, 2011

    Thanks for the comments. Attached is an updated patch.

    In the RawIOBase docs it says "If the object is in non-blocking mode and no bytes are available, None is returned." So I wasn't sure if that meant any time no bytes were available or just when no bytes are available and EOF has not been reached. -- I updated it to return 0 instead of None.

    I simplified _read_chunked() and renamed it to _readall_chunked() since that is all it does.

    I added the versionadded tag specifying that it was added in 3.3 since the patch is for the default branch.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Dec 6, 2011

    New changeset 806cfe39f729 by Antoine Pitrou in branch 'default':
    Issue bpo-13464: Add a readinto() method to http.client.HTTPResponse.
    http://hg.python.org/cpython/rev/806cfe39f729

    @pitrou
    Copy link
    Member

    pitrou commented Dec 6, 2011

    Ok, thank you. I've now committed the patch in the default branch. Congratulations!

    (since the documentation doesn't claim that HTTPResponse implements RawIOBase, I tend to consider this a feature request rather than a bugfix, hence no 3.2 commit)

    @pitrou pitrou closed this as completed Dec 6, 2011
    @pitrou pitrou added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Dec 6, 2011
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jan 4, 2012

    New changeset 4b21f651eeee by Antoine Pitrou in branch 'default':
    Issue bpo-13713: fix a regression in HTTP chunked reading after 806cfe39f729
    http://hg.python.org/cpython/rev/4b21f651eeee

    @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
    easy 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
    2 participants
    @bitdancer @pitrou