cloudyr / aws.s3

Amazon Simple Storage Service (S3) API Client
https://cloud.r-project.org/package=aws.s3
381 stars 148 forks source link

how do we want to approach returning the unparsed response? #10

Closed almartin82 closed 8 years ago

almartin82 commented 9 years ago

Saw this on bucketlist() and now on getbucket() -- when parse_response = FALSE, the default behavior of the function is to return only the 'contents' of the response -- which doesn't hold for an unparsed response, and you get

> ex
Bucket: 

named list()

Right now I am handling this by identifying unparsed responses by their class (response) and returning that list without any additional processing -- here's one example. basically just drafting off the way we currently handle errors:

  if (inherits(r, "aws_error") | inherits(r, "response")) {
        return(r)

This feels a bit clumsy to me, and presumably whatever we come up with for returning unparsed responses is going to become a common design pattern throughout, so I thought we should kick this around for discussion.

leeper commented 9 years ago

It's probably easiest to just add parse_response as a formal argument to all the functions so that they can all "see" when they're going to get a parsed or unparsed response.

cboettig commented 9 years ago

I hit that issue as well. In general I think returning the httr response object when parse_response is false is preferable to content(response). By default the response print method will show the contents (or rather the top part) anyway, and this makes more detailed debugging info etc available. content(resp) is more of a convenience function anyhow that is not recommended for programmatic use, where explicit parsing is preferred based on the known return types.

On Wed, Jul 22, 2015, 7:23 AM Thomas J. Leeper notifications@github.com wrote:

It's probably easiest to just add parse_response as a formal argument to all the functions so that they can all "see" when they're going to get a parsed or unparsed response.

— Reply to this email directly or view it on GitHub https://github.com/cloudyr/aws.s3/issues/10#issuecomment-123739592.

http://carlboettiger.info

leeper commented 8 years ago

I think we should avoid returning unparsed responses. There's basically one context in which we don't want parsing - that's when we're doing a get_object(). In all other contexts, I don't see any reason why an end user would want that (and if they do, they can call s3HTTP() directly). So, I think I'll close this for now as everything seems to be working fine.