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

Adding optional stream argument to DownloadBase. #20

Merged
merged 2 commits into from
Jul 31, 2017

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Jul 31, 2017

This is towards a fix for #5 but I wanted to discuss before doing the more "serious" work.

/cc @tseaver who I can't assign

@@ -162,10 +185,15 @@ def _process_response(self, response):
self._finished = True
_helpers.require_status_code(
response, _ACCEPTABLE_STATUS_CODES, self._get_status_code)
if self._stream is not None:
self._write_to_stream(response)

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@dhermes
Copy link
Contributor Author

dhermes commented Jul 31, 2017

Will merge when this goes green. I'm assuming @jonparrott's LGTM extends to this PR with even less code.

@dhermes dhermes merged commit 30405d4 into googleapis:master Jul 31, 2017
@dhermes dhermes deleted the prework-fix-5 branch July 31, 2017 21:52
@dhermes dhermes mentioned this pull request Aug 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants