Skip to content

Fixed chunk_size values in iter_content() and iter_lines(). False default instead of None. #589

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

Closed
wants to merge 2 commits into from

Conversation

mponton
Copy link

@mponton mponton commented May 6, 2012

Following discussion about issue #539 Kenneth agreed that a default
chunk_size of 1 for iter_lines() would make more sense, but
apparently changed chunk_size for iter_content() by mistake
instead, which did solve the iter_lines() issue but also changed the
default iter_content() chunk to 1 bytes which is not desirable.

Also changed default for decode_unicode to False instead of None in iter_lines().

Marco Ponton added 2 commits May 6, 2012 12:56
Following discussion about issue #539 Kenneth agreed that a default
`chunk_size` of 1 for `iter_lines()` would make more sense, but
apparently changed `chunk_size` for `iter_content()` by mistake
instead, which did solve the `iter_lines()` issue but also changed the
default `iter_content()` chunk to 1 bytes which is not desirable.
…ines()

Doesn't change logic, but makes more sense.
@travisbot
Copy link

This pull request fails (merged 85e83c4 into c84012e).

@mponton
Copy link
Author

mponton commented May 6, 2012

Hmmm, I think the logic in

if pending is not None:
    chunk = pending + chunk
lines = chunk.splitlines()

if lines and lines[-1] and chunk and lines[-1][-1] == chunk[-1]:
    pending = lines.pop()
else:
    pending = None

Does not handle "\r\n" as splitlines() does when using a chunk_size of 1. The quote:

quote = (
            '''Agamemnon  \n'''
            '''\tWhy will he not upon our fair request\r\n'''
            '''\tUntent his person and share the air with us?'''
        )

used in test_requests.py contains \r\n which ends up becoming a line in iter_lines() when the iterator hits the \r character, next the \n character is read and an empty line is spit by chunk.splitlines(). Thus the test fails as 4 lines are returned instead of 3. If the chunk_size is larger and \r\n are read in a single chunk, then it works as expected.

@kennethreitz
Copy link
Contributor

decode_unicode is None for a reason :)

@kennethreitz
Copy link
Contributor

iter_content also has a proper size now, so closing. Thanks though!

@mponton
Copy link
Author

mponton commented May 8, 2012

OK, not sure why decode_unicode should be None but you may then want to change it to None in iter_content() too...? :-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants