Skip to content

Repository navigation

Allow setting chunk_size for Response.iter_bytes() etc... #394

Closed
#1277
Closed
Allow setting chunk_size for Response.iter_bytes() etc...#394
#1277
Labels
requests-compatIssues related to Requests backwards compatibility
Milestone
v1.0

Description

@sethmlarson
sethmlarson
opened on Sep 26, 2019

Requests allowed setting chunk_size within .iter_content() which is currently not an option for our alternatives .stream() and .stream_text().

For .stream_text() we should go the extra step and fix the issue that users sometimes run into when using this feature and use chunk-size for measuring the decoded text, not the raw bytes.

Activity

  1. sethmlarson
    added
    good first issueGood for newcomers
    requests-compatIssues related to Requests backwards compatibility
    on Sep 26, 2019

  2. lovelydinosaur commented on Sep 27, 2019

    @lovelydinosaur
    lovelydinosaur
    on Sep 27, 2019
    Member

    I seem to remeber this playing into some primitives around streaming bytes vs text that we never ended up digging into?

  3. sethmlarson
    removed
    good first issueGood for newcomers
    on Sep 29, 2019

  4. lovelydinosaur commented on Dec 20, 2019

    @lovelydinosaur
    lovelydinosaur
    on Dec 20, 2019
    Member

    A good first pass onto this would be to change the decoder interface slightly, so that instead of eg. yeilding a byte chunk, they yield a list of byte chunks.

    On the first refactoring pass, we don't need to actually change the internal implmentation much - the decoders can just always yield a list with a single item.

    We'd then be able to add a chunk_size argument to the decoders, which would return 0, 1, or many properly-sized chunks on each yield.

  5. florimondmanca
    changed the title [-]Allow setting chunk_size for Response.stream() and Response.stream_text()[/-] [+]Allow setting chunk_size for Response.aiter_*()[/+] on Dec 21, 2019

  6. florimondmanca commented on Dec 21, 2019

    @florimondmanca
    florimondmanca
    on Dec 21, 2019
    Contributor

    Updated the issue title to reflect the current Response.aiter_* API :-) (see #610).

  7. b0g3r commented on Mar 12, 2020

    @b0g3r
    b0g3r
    on Mar 12, 2020
    Contributor

    How could I help with this issue?

  8. florimondmanca commented on Mar 12, 2020

    @florimondmanca
    florimondmanca
    on Mar 12, 2020
    Contributor

    Hi @b0g3r! I think this is still something we’d like to have, and given discussions in python-gitlab/python-gitlab#1036 it seems like some folks would like to see it too. :)

    Ways to move forward would be:

    • Propose an API for this, with context on the existing API on Requests
    • Investigate implementation details (ie how are we going to split chunks: buffering, other? Looking at how Requests/urllib3 do this can help)
    • Draft a PR :)

  9. b0g3r commented on Mar 13, 2020

    @b0g3r
    b0g3r
    on Mar 13, 2020
    Contributor

    Do I understand correctly that we will need to forward chunk_size here?

    httpx/httpx/_dispatch/urllib3.py

    Lines 112 to 115 in a82adcc

    def response_bytes() -> typing.Iterator[bytes]:
    with as_network_error(socket.error):
    for chunk in conn.stream(4096, decode_content=False):
    yield chunk

  10. florimondmanca commented on Mar 13, 2020

    @florimondmanca
    florimondmanca
    on Mar 13, 2020
    Contributor

    @b0g3r Careful that we're in a sort of transition state w.r.t. urllib3 usage due to #804 (we'll soon use our own sync implementation, though keeping urllib3 as an option). Due to this I wouldn't advise relying on any existing urllib3 functionality — also because we'd want to provide chunk sizing on the async layer too, and it'd be odd to have a different implementation on both sides.

    I think we want to look at controlling the chunk size directly from response.iter_bytes()/response.aiter_bytes(), instead…

  11. lovelydinosaur commented on Mar 13, 2020

    @lovelydinosaur
    lovelydinosaur
    on Mar 13, 2020
    Member

    @b0g3r So, as with comment #394 (comment) - the right place to start with this would be a pull request to https://github.com/encode/httpx/blob/master/httpx/_decoders.py that changes the interface of the decoders, so that they return a list of bytes rather than bytes.

    (And correspondingly, changing the places where the response calls the decoder such as

    httpx/httpx/_models.py

    Line 915 in a82adcc

    yield self.decoder.decode(chunk)
    to deal with a list of bytes as a return result.)

    I'd start with that as a foundational pull request, which will then make the remaining work much easier. (Adding chunk sizes to the decoder interface, and through to the response methods.)

  12. b0g3r commented on Mar 13, 2020

    @b0g3r
    b0g3r
    on Mar 13, 2020
    Contributor

    (a)iter_raw(self, chunk_size=1)

    for part in self._raw_stream:
    yield part

    let's use bytestring as buffer

    buffer = b""
for part in self._raw_stream:
    buffer += part
    while len(buffer) >= chunk_size:
        yield buffer[:chunk_size]
        buffer = buffer[chunk_size:]
if buffer:
    yield buffer

    (a)iter_bytes(self, chunk_size=ITER_CHUNK_SIZE)

    • chunk_size=ITER_CHUNK_SIZE (512) because requests has it 🌚
    • calls (a)iter_raw

    (a)iter_text(self, chunk_size=ITER_CHUNK_SIZE)

    • calls (a)iter_bytes

    (a)iter_line(self, chunk_size=ITER_CHUNK_SIZE)

    • calls (a)iter_test
    • current code expects that each chunk containt full line(s), but it's not true (UPD: found code for splitting in LineDecoder)
    • requests has elegant solution

  13. b0g3r commented on Mar 13, 2020

    @b0g3r
    b0g3r
    on Mar 13, 2020
    Contributor

    @tomchristie As I see (a)iter_raw doesn't use any decoder 🤔

  14. piersoh commented on May 21, 2020

    @piersoh
    piersoh
    on May 21, 2020

    Would be good to have chunk_size=None option so that httpx can return chunks at the HTTP chunk boundaries as per the requests library - this is useful for apps that require timely delivery.

  15. 1 remaining item

  16. lovelydinosaur
    modified the milestones: v1.1, v1.0 on Aug 7, 2020
  17. lovelydinosaur
    changed the title [-]Allow setting chunk_size for Response.aiter_*()[/-] [+]Allow setting chunk_size for `Response.iter_bytes()` etc...[/+] on Aug 7, 2020
  18. lovelydinosaur
    changed the title [-]Allow setting chunk_size for `Response.iter_bytes()` etc...[/-] [+]Allow setting `chunk_size` for `Response.iter_bytes()` etc...[/+] on Aug 7, 2020
  19. cdeler
    added 10 commits that reference this issue on Sep 9, 2020

    Added chunk_size to Response.iter_bytes() and Response.aiter_bytes() (e…

    2c74758

    Added chunk_size to Response.iter_bytes() and Response.aiter_bytes() (e…

    e2f38ba

    Added chunk_size to Response.iter_bytes() and Response.aiter_bytes() (e…

    3aab30b

    When chunk_size=None, Response.(a)iter_bytes returns data as it arriv…

    04f11f7

    Added chunk_size to Response.iter_bytes() and Response.aiter_bytes() (e…

    20934d6

    When chunk_size=None, Response.(a)iter_bytes returns data as it arriv…

    b63a642

    Added chunk_size to Response.iter_bytes() and Response.aiter_bytes() (e…

    62f62bc

    When chunk_size=None, Response.(a)iter_bytes returns data as it arriv…

    aceaa56

    Added chunk_size to Response.iter_bytes() and Response.aiter_bytes() (e…

    eb3ea9e

    When chunk_size=None, Response.(a)iter_bytes returns data as it arriv…

    60ea230
  20. lovelydinosaur
    closed this as completedin #1277on Nov 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    requests-compatIssues related to Requests backwards compatibility

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions