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

[SVCS-421] Remove unsizable response reader flag #309

Conversation

Johnetordoff
Copy link
Contributor

@Johnetordoff Johnetordoff commented Dec 18, 2017

Ticket

https://openscience.atlassian.net/browse/SVCS-421
A continuation of #273

Purpose

The purpose of this PR is the remove the superfluous unsizable argument from the response reader safely and change the stream size behavior to make more sense.

Changes

  • Removes unsizable parameter for StreamResponseReader
  • Changes conditional so size parameter directly controls stream size.
  • Adds unit tests.

Side effects

None that I know of.

QA Notes

Download a file from each of the following providers:

  • GoogleDrive
  • Github
  • Bitbucket
  • Box
  • Dropbox
  • GitLab

These all use size as a positional argument.

Deployment Notes

None.

@coveralls
Copy link

coveralls commented Dec 18, 2017

Coverage Status

Coverage increased (+0.1%) to 89.89% when pulling ab86787 on Johnetordoff:feature/remove-unsizable-response-reader-flag into 3ec764a on CenterForOpenScience:develop.

@cslzchen cslzchen self-requested a review January 3, 2018 20:18
Copy link
Contributor

@cslzchen cslzchen left a comment

Choose a reason for hiding this comment

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

LGTM with minor style updates in secondary PR and move to PCR. 🎆 🎆

[SVCS-421] Update style, use HTTPStatus and fix typo
@coveralls
Copy link

coveralls commented Jan 3, 2018

Coverage Status

Coverage increased (+0.1%) to 89.983% when pulling 44c2514 on Johnetordoff:feature/remove-unsizable-response-reader-flag into cae41fc on CenterForOpenScience:develop.

@coveralls
Copy link

coveralls commented Jan 3, 2018

Coverage Status

Coverage increased (+0.1%) to 89.983% when pulling 44c2514 on Johnetordoff:feature/remove-unsizable-response-reader-flag into cae41fc on CenterForOpenScience:develop.

@felliott
Copy link
Member

felliott commented Jan 8, 2018

👍, merging

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

Successfully merging this pull request may close these issues.

None yet

5 participants