MarkEdmondson1234 / googleAuthR

Google API Client Library for R. Easy authentication and help to build Google API R libraries with OAuth2. Shiny compatible.
https://code.markedmondson.me/googleAuthR
Other
175 stars 51 forks source link

Response from gar_api_generator() doesn't play well with print() #42

Open jarodmeng opened 8 years ago

jarodmeng commented 8 years ago

When googleAuthR.rawResponse option is set to FALSE (which is the default), function built by gar_api_generator() return a response-class object. However, those response objects don't seem to play well with response class S3 methods (defined in httr) such as print.response().

The reason is that print.response() calls content() which in turn accesses the $content element in parsing. When googleAuthR.rawResponse option is set to FALSE, doHttrRequest() replaces req$content with a parsed version of the response content. This replacement might be the key why functions like print.response() doesn't work well.

MarkEdmondson1234 commented 8 years ago

This is kinda intentional, as the Google APIs tend to always give out the same JSON style content, but it has caused some problems when this isn't the case. If you want the httr methods to work then setting googleAuthR.rawResponse to TRUE should give you the fully undiluted response (its a recent addition to work around some of the problems). Or do you have a use case or suggestion on what would make it easier to work with?

jarodmeng commented 8 years ago

I guess my question is why replacing the raw $content with the parsed version? Is the parsed content used later on to warrant the replacement? If not, perhaps store the parsed content in a different element (e.g., $parsed_content) instead of replacing the original $content element. Other functions that use the parsed content can just access the $parsed_content element while response-class methods would work just fine as well.

MarkEdmondson1234 commented 8 years ago

Thats a plan, I will look at it. The $content was returned as my original thought was it was easier for the end user to reach if say they didn't know httr, and then that $content is used by the data_parse_function. But I like keeping it more transparent for httr methods.

MarkEdmondson1234 commented 8 years ago

I made the switch, hopefully hasn't broken anything, doesn't seem to in what I've tested so far, but if you can see anything let me know. Does it now do as you requested?

MarkEdmondson1234 commented 8 years ago

Ok, it did break stuff that assumed the content came out without parsing in $content. It may be easier to remake the print method