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

Added downloadAsStream method to StorageObject #199

Merged
merged 2 commits into from
Oct 11, 2016

Conversation

gedimin45
Copy link
Contributor

This let's retrieve the body of the StorageObject as a stream. Inspired by PSR-7 response method getBody. This method returns a PSR-7 StreamInterface which enables the user to decide whether they want to write stream to another stream or get the contents as a string.
I also propose to remove downloadAsString and downloadToFile in the next major release since the functionality of these methods is easily achieved using getBody and having the other methods can be confusing.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@googlebot googlebot added the cla: no This human has *not* signed the Contributor License Agreement. label Oct 9, 2016
@gedimin45
Copy link
Contributor Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@googlebot googlebot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Oct 9, 2016
@jdpedrie jdpedrie added the api: storage Issues related to the Cloud Storage API. label Oct 9, 2016
@dwsupplee
Copy link
Contributor

I like the thought of exposing the streams, but am concerned getBody might be too unfamiliar for those who don't have intimate knowledge of the concepts found in PSR-7. Also, I like keeping the helper methods such as downloadToFile as depending on the use case it can save some keystrokes for a user and means they won't need to investigate how to perform that action themselves.

@Ged15 What are your thoughts on getBody => downloadStream / downloadAsStream?

@gedimin45
Copy link
Contributor Author

downloadAsStream also sounds pretty clear 😄 shall I rename it?

@dwsupplee
Copy link
Contributor

Please! :) Thanks for the quick response.

@gedimin45 gedimin45 force-pushed the get-body-for-storage-object branch from 67bd021 to 98e6fd1 Compare October 11, 2016 13:20
@gedimin45 gedimin45 changed the title Added getBody method to StorageObject Added downloadAsStream method to StorageObject Oct 11, 2016
@gedimin45
Copy link
Contributor Author

@dwsupplee I also reused the downloadAsStream method in other methods to avoid duplication

@dwsupplee
Copy link
Contributor

This looks awesome. Thanks again @Ged15!

@dwsupplee dwsupplee merged commit 52c064d into googleapis:master Oct 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the Cloud Storage API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants