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.

Sign up for GitHub

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

[SVCS-421] Removing unsizable flag from ResponseStreamReader #273

Closed
wants to merge 2 commits into from
Closed

[SVCS-421] Removing unsizable flag from ResponseStreamReader #273

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
746b460
Removing unstable flag from ResponseStreamReader
AddisonSchiller Sep 25, 2017
1dc7d25
Merge branch 'develop' into feature/remove-unsizable-response-reader-…
cslzchen Dec 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions waterbutler/core/streams/http.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,10 @@ def _make_boundary_stream(self):

class ResponseStreamReader(BaseStream):

def __init__(self, response, size=None, name=None, unsizable=False):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

size=None and unsizable=False are used together, should we remove both? And please make sure that no one calls ResponseStreamReader with the removed kwargs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe keep those two but improve conditional logic. However, we still need to make sure that int(size) does not throw exceptions (or we need to catch them when that happens). Whether size is of type int or string makes a difference.

elif not unsizable and size is not None:
    try:
        elf._size = int(size)
    except:
        elf._size = None

Copy link
Contributor

@Johnetordoff Johnetordoff Dec 18, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So after a little research I've found the unsizable argument is never used, and the size argument is used, but seems to usually be None, so I've decided to change the code to match the suspected intention of original writer. When a size is given the stream will read to that size, if it is None, the whole stream will be read.

 def __init__(self, response, size=None, name=None):
        super().__init__()
        if 'Content-Length' in response.headers:
            self._size = int(response.headers['Content-Length'])
        else:
            self._size = size

        self._name = name
        self.response = response

def __init__(self, response, size=None, name=None):
super().__init__()
if 'Content-Length' in response.headers:
self._size = int(response.headers['Content-Length'])
elif not unsizable:
self._size = int(size)
else:
self._size = None

Expand Down