search

looker-open-source / lookr

An R library for the Looker API (4.0)
MIT License
77 stars 29 forks source link

sdk$runInlineQuery() results in an error #26

Closed russellpierce closed 6 years ago

russellpierce commented 6 years ago

Hiya, spoke with Peggy B. in Looker support earlier today and she asked me to open this issue related to my inability to get the runInlineQuery method of the SDK to work out of the box.

Here are what my attempts to trace the problem while working with Peggy showed...

sdk$runInlineQuery() calculates a JSON body which it passes in to self$userSession$queryApi$run_inline_query which attempts to use the toJSONString method on the body... which doesn't exist. This results in Error in body$toJSONString : $ operator is invalid for atomic vectors.

Visual inspection of the JSON body produced by sdk$runInlineQuery() also indicates a potential issue with the zero length vectors for sorts and query_timezone being interpreted as [null] rather than [] or being pulled out of the body. This might be an issue of version mismatch with jsonlite, in which case specifying the version required to meet the needs of the package would help.

BTW... thanks for this new Swaggered up version of {lookr}. Relative to the old {rJava} version, for an R programmer, this is so much easier to use and understand.

maxcorbin commented 6 years ago

Heya @russellpierce! Thanks for the bug report, I know what is causing the problem and am working on deploying a fix for it that is backwards compatible. Hoping to get it out today but it might happen over the long weekend.

I'll take a look at the jsonlite issue here too, but my solution is probably going to avoid using jsonlite in favor of the swagger generated code contained in the Query class. I'll implement the same pattern for runLook if it works well.

shawnconnor commented 6 years ago

Thanks @maxcorbin ! I just ran into this issue as well; I was trying to switch a "runLook" to a "runInlineQuery" to get around the limit issue just now, and ran into this one instead. Great to see you already know about it.

maxcorbin commented 6 years ago

This was fixed in https://github.com/looker/lookr/pull/27, it's not the most graceful solution and I still haven't implemented the direct response -> data.frame pipeline that I want in a more complete version but since that'll be a breaking change I left it for the future

russellpierce commented 6 years ago

Thanks for the quick work on this @maxcorbin !