search

sbg / sevenbridges-r

Seven Bridges API Client, CWL Schema, Meta Schema, and SDK Helper in R
https://sbg.github.io/sevenbridges-r/
Apache License 2.0
35 stars 14 forks source link

Projects older than 100 not retrieved with a$project(<pattern>) #75

Open mfoos opened 5 years ago

mfoos commented 5 years ago

Hi folks,

I was having a problem where authentication would work fine, and listing all projects with p <- a$project(complete = TRUE) worked fine, but then I was not able to retrieve a project with p <- a$project("project_name") even though it was listed and populated and I had permission to see it.

However I spot-checked "old" projects and realized that p <- a$project("project_name", complete = TRUE) did work to retrieve my project, which was project #126 for us (that is, larger than the 100 automatically retrieved without complete = TRUE)

This extra parameter is not mentioned in the references with "Get a project by pattern-matching its name" so I assume it is unintended behavior. Thanks!

nanxstats commented 5 years ago

Hi @mfoos -thanks a lot for the feedback. I think it might be related to the fact that the default value for complete was set to FALSE in the package. This behavior is described in this section:

https://sbg.github.io/sevenbridges-r/articles/api.html#api-general-information

I also feel that the readme only offered some brief showcases (not even mentioning complete at all), while the examples in the API vignette are more proper. For example, this "project details" section in the API vignette has better examples of using complete = TRUE when doing name pattern matching:

https://sbg.github.io/sevenbridges-r/articles/api.html#get-details-about-existing-project

I believe setting complete = TRUE by default in the package would avoid a lot of boilerplates and surprises. Unfortunately, for backward compatibility reasons, it's not practical anymore to flip this setting on the package level after it is released for more than three years.

The silver lining is, if we have a chance to build a new, rewritten API client package in the future, we probably will do it differently. Until then, let's just try to be mindful and set complete = TRUE when it might be needed.

nanxstats commented 5 years ago

Hmm, by looking at the code again, I feel we could potentially have a backward-compatible patch that improves usability.

It goes like this: we can expose complete as a new global option just like offset and limit, set its default value to be FALSE, and change the API function/method arguments to load the value from the global option. This way, the previous API scripts should still run, and in new scripts users can enable this globally by setting options(complete = TRUE) once.

I'll try to make this happen as soon as I got time. So stay tuned.