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

Swagger Support #417

Closed
NavinCJ opened this issue Apr 27, 2019 · 10 comments
Closed

Swagger Support #417

NavinCJ opened this issue Apr 27, 2019 · 10 comments
Labels
docs Related to documentation only theme: OpenAPI pertains to OpenAPI will not fix No intended plans to ever address issue
Milestone

Comments

@NavinCJ
Copy link

NavinCJ commented Apr 27, 2019

Does the current annotation support swagger specs fully? For example I wanted to have a POST API with name value in form-data whose value is of type 'File'. If annotations do not support, is it possible for me to prepare the swagger json/yml separately and pass it as argument when I launch the router?

@schloerke
Copy link
Collaborator

Current annotation support full swagger spec?

The GitHub version currently allows for users to add their own spec information. plumber does not currently validate this spec, so please return a valid swagger spec object.

To get the latest: remotes::install_github("trestletech/plumber")

"Add you own spec" example: (changes title to the current time)
https://github.com/trestletech/plumber/blob/796e8fe9cdad22b76b63f8f7bd48a5605a243817/scripts/manual_testing.R#L14-L20

If elements are required to be an array, I suggest using list(), rather than a vector.

@schloerke
Copy link
Collaborator

  • I do not see plumber ever supporting a completely full spec through plumber tags alone.
  • plumber is open to adding new tags that are simple / concise.

Please see #390 as an example of an error caused a vector of size 1 when it should be treated as a list.

@lorenzwalthert
Copy link

Pretty interesting. Two comments on that:

When I tried to run the above code with the devel version of swagger, I get an error:

library(plumber)
pr <- plumber$new()
pr$handle("GET", "/:path/here", function(){})

pr$run(
  port = 1234,
  swagger = function(pr_, spec, ...) {
    spec$info$title <- Sys.time()
    spec
  }
)
Running plumber API at http://127.0.0.1:1234
Running Swagger UI  at http://127.0.0.1:1234/__swagger__/
<simpleError: 'swagger_spec' is not an exported object from 'namespace:swagger'>
<simpleError: 'swagger_spec' is not an exported object from 'namespace:swagger'>

When I use the devel version of rstudio/swagger, then I don't get an error.

So maybe specify a minimal version requirement on swagger?

But when I want to click on the json link right below the API title (http://127.0.0.1:1234/openapi.json in my case), I get this error:

<simpleError in if (any(toRemove)) {    x[toRemove] <- NULL}: missing value where TRUE/FALSE needed>

This also happens when I don't specify the swagger argument, so maybe it is unrelated to this thread.

I do not see plumber ever supporting a completely full spec through plumber tags alone.

Makes sense to me. What about a mixed approach? With purrr::list_modify(), we one could:

  • read in a swagger config file and convert it to a list.
  • take the swagger argument of pr$run().
  • Merge the two, i.e. if an element is not set by plumber already, take the one from the config file.

That would allow to maintain a clean swagger config file because specifying many elements of the configuration similar to

     spec$info$title <- Sys.time() 

may lead to code that is not very readable.

Optionally, I think it would be nice if there would be a function to export the config file. I did not find that? Is it there already? Thanks for your work on the project.

@schloerke
Copy link
Collaborator

@lorenzwalthert

So maybe specify a minimal version requirement on swagger?

Great idea.

if (any(toRemove)) { x[toRemove] <- NULL}: missing value where TRUE/FALSE needed

Yay for finding a bug!

Specifying elements of the config may lead to code that is not very readable.

Correct utils::modifyList might be a better, stock approach for merging the two lists.

function to export the config file?

Check out pr$swaggerFile() for now. I'd like move forward with pr$openAPIFile(), but I'm not sold on the function name. (Too many caps in a row)

@schloerke
Copy link
Collaborator

(Leaving this issue open for now as there is currently dev version support but not support on the CRAN version of plumber.)

@schloerke schloerke added this to the v0.5.0 - Next CRAN release milestone May 1, 2019
@NavinCJ
Copy link
Author

NavinCJ commented May 7, 2019

Current annotation support full swagger spec?

The GitHub version currently allows for users to add their own spec information. plumber does not currently validate this spec, so please return a valid swagger spec object.

To get the latest: remotes::install_github("trestletech/plumber")

"Add you own spec" example: (changes title to the current time)
https://github.com/trestletech/plumber/blob/796e8fe9cdad22b76b63f8f7bd48a5605a243817/scripts/manual_testing.R#L14-L20

If elements are required to be an array, I suggest using list(), rather than a vector.

Thank you so much, very useful.
I was able to inject my custom swagger spec using yaml::read_yaml( ). Can't wait for v0.5.0 to be released.

@eduardszoecs
Copy link

@schloerke Thanks for clarification on midterm scope. Injecting a custom swagger works like a charm with the yaml fix mentioned in #390 .

@schloerke schloerke added docs Related to documentation only theme: OpenAPI pertains to OpenAPI labels May 28, 2019
@jeffkeller87
Copy link

Just wanted to include an idea for automatically populating the query parameter required field in a Swagger spec by inspecting a function's formal arguments:

#' @get /myFun
myFun <- function(x, y = 1, z = NULL) {
  
}

args <- formals(myFun)
sapply(args, is.symbol)
# x     y     z 
# TRUE FALSE FALSE 

Which could be used to generate the equivalent of

r <- plumb("myFun.R")
r$run(port = 8000, host = "0.0.0.0", swagger = function(pr_, spec, ...) {
  spec$paths$`/myFun`$get$parameters[[1]]$required <- TRUE
  spec
})

@meztez
Copy link
Collaborator

meztez commented Apr 29, 2020

formals won't work in this case:

myFun <- function(x, y = 1, z = NULL, b = .GlobalEnv) {
  
}

args <- formals(myFun)
sapply(args, is.symbol)
#    x     y     z     a     b 
# TRUE FALSE FALSE FALSE  TRUE 

But this will?

sapply(args, identical, formals(function(x){})$x)
#    x     y     z     a     b 
# TRUE FALSE FALSE FALSE FALSE 

@schloerke
Copy link
Collaborator

Plumber has no plans on fully supporting OpenAPI Spec via plumber tags (#*/#').

Going to close this issue as the v1.0.0 release of plumber will allow for users to set the open api information manually.

Example:

pr$set_api_spec(object)
pr$set_api_spec(func)
pr_set_api_spec(pr, object)
pr_set_api_spec(pr, function)
## Via the `@plumber` tag
#* @plumber
function(pr) {
  pr_set_api_spec(pr, object)  
}

Also, we are able to validate what plumber is returning using validate_api_spec(pr).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Related to documentation only theme: OpenAPI pertains to OpenAPI will not fix No intended plans to ever address issue
Projects
None yet
Development

No branches or pull requests

6 participants