rexyai / RestRserve

R web API framework for building high-performance microservices and app backends
https://restrserve.org
275 stars 32 forks source link

Please share your feedback #109

Closed dselivanov closed 4 years ago

dselivanov commented 4 years ago

This topic is to share feedback about current dev version - documentation, API, etc. This topic is NOT for questions - use stackoverflow instead and mark you question with restrserve tag

luckys59 commented 4 years ago

Hi Dmitriy, The RestRserve is absolutely fantastic and I loved it personally.

Just for sake of feedback following are my points:

  1. Needed more documentation - It took some time to explore how to parse data from body.
  2. More error handling - We need to list all parameters which was handled in Plumber, initially I didn't get but later understood.
  3. I was unable to install it on Windows but it worked on Linux so did not try again.

Again current RestRserve really serving more than expected 🥇

dselivanov commented 4 years ago

@luckys59 could you elaborate on this a little bit more?

More error handling - We need to list all parameters which was handled in Plumber, initially I didn't get but later understood.

luckys59 commented 4 years ago

@dselivanov, I don't remember exact error messages now but initially it took some time to figure out. Plumber handles parameter parsing quite intuitively even if data is coming as query parameter or in body. All I have to do is list the parameter names where RestRserve needs explicit and detailed description. Please see the example below where I have listed plumber convention and RestRserve convention for one service.

# ***********************
# Plumber
# ***********************
#* @param param1
#* @param param2
#* @param param3
#* @param var_name
#* @post /api

# ***********************
# Main function
# ***********************
fapi <- function(param1,param2,param3, var_name){
    # getting data paths
    # R function goes here
}

# ***********************
# RestRserve
# ***********************
api = function(request, response) {
  request_body = fromJSON(rawToChar(request$body))

  param1 = request_body$param1
  param2 = request_body$param2
  param3 = request_body$param3
  var_name = request_body$var_name

  response$body = toJSON(rownames = FALSE, fapi(param1,param2,param3,var_name))
  forward()
}
app$add_post(path = "/api", FUN = api)

Please don't take it as a critic, it's a fantastic utility. If I would have understood web services better it would have not take longer to understand.

dselivanov commented 4 years ago

@luckys59 seems you don't use all the recent features we've added (check this article - http://restrserve.org/articles/ContentHandlers.html - json encoding and decoding now can be done automagically). Note that forward() calls are deprecated.

More idiomatic 0.2 code will be something like:

library(RestRserve)

fapi = function(param1,param2,param3, var_name){
  list(param1,param2,param3, var_name)
}

app = Application$new(content_type = 'application/json')
api = function(request, response) {
  request_body = request$body_decoded

  param1 = request_body$param1
  param2 = request_body$param2
  param3 = request_body$param3
  var_name = request_body$var_name

  response$set_body(fapi(param1,param2,param3,var_name))
}

app$add_post(path = "/api", FUN = api)
luckys59 commented 4 years ago

Hi @dselivanov , Thanks for sharing this. I would wait for pushing changes into cran and then make the changes. Since applications is already running well and I don't want to make changes. Otherwise please let me know if you don't anticipate changes so I ask my team to get the latest version and do necessary updates now.

Thanks!

dselivanov commented 4 years ago

see #120

Great lib, deployed it a brief while ago first time instead of opencpu (works much better with reticulate and pointer objects as fasttext models). Found it a bit difficult to navigate different examples to get to where I wanted to be. Also restrserve requires a bit more upfront knowledge about restapis than opencpu, where you kind of just build the package and done. I like the freedom and (percieved) architectural simplicity of restrserve over opencpu (I feel more in control), but for a fresh new user, the learning curve feels steeper then for say plumber (but restrserve is also much more powerful). More explicit guidance about the type of applications and when and where you have to specifify things to get those running would be awesome. I mean you can get it if you just read the example code and think about it, but it can be more simple for new users for say a typical use case as a ML prediction api. The examples are good on this one,but require a bit more knowledge than may be handy for a starter or say plumber convert. Opencpu is also a bit more complex but has more documentation. Restrserve is a great, new and still evolving way for making usable rest apis in R.

mrchypark commented 4 years ago

Hi, I love this package. Thank you for build.

I think this package is not considor unicode.

for example, hello world to path /hello/path/{text} and text in unicode, it's broken text return.

dselivanov commented 4 years ago

@mrchypark thanks for feedback, appreciate it. Could you please fill an issue with a reproducible example please? so we can fix it.

mrchypark commented 4 years ago

here's hello world built in example RestRserve.

And i request to endpoint this /hello/path/{name} using /hello/path/안녕.

image

but request using httr, It's fine.

GET("http://localhost:8080/hello/path/안녕") %>% 
    content()

here's hello world code example.


#!/usr/bin/env Rscript

## ---- load packages ----

library(RestRserve)

## ---- create handler for the HTTP requests ----

# simple response
hello_handler = function(request, response) {
  response$body = "Hello, World!"
}

# handle query parameter
heelo_query_handler = function(request, response) {
  # user name
  nm = request$parameters_query[["name"]]
  # default value
  if (is.null(nm)) {
    nm = "anonym"
  }
  response$body = sprintf("Hello, %s!", nm)
}

# handle path variable
hello_path_handler = function(request, response) {
  # user name
  nm = request$parameters_path[["name"]]
  response$body = sprintf("Hello, %s!", nm)
}

## ---- create application -----

app = Application$new(
  content_type = "text/plain"
)

## ---- register endpoints and corresponding R handlers ----

app$add_get(
  path = "/hello",
  FUN = hello_handler
)

app$add_get(
  path = "/hello/query",
  FUN = heelo_query_handler
)

app$add_get(
  path = "/hello/path/{name}",
  FUN = hello_path_handler,
  match = "regex"
)

## ---- start application ----
backend = BackendRserve$new()
backend$start(app, http_port = 8080)
s-u commented 4 years ago

Please note that the URL you use is invalid - only ASCII character are allowed in URLs. httr does not process the URL nor does it encode it, so your invalid URL is passed as-is. If you want to pass non-ASCII characters in URLs, you have to encode them and handle the encoding on the receiving end as well - see RFC2396

mrchypark commented 4 years ago

@s-u As I said, httr request is fine to get returns include non-ASCII characters.

but you means url encoding is needed before request, I understand?

s-u commented 4 years ago

HTTP doesn't support anything other than ASCII in URLs - there is no provision to declare encoding in the URL. Only HTTP request body can specify encoding, so that's the only way to reliably transport non-ASCII content. Some browsers will re-code URLs typed by the user in UTF-8 byte sequences (using the %xx byte representation) in that case you have to make sure you interpret the payload in UTF-8 and not any other non-UTF-8 locale. In addition, if you want the output to be treated correctly, you have to also declare the response encoding accordingly - e.g. above you have to add

response$content_type="text/html; charset=UTF-8"

and depending on the locale you run R in you may have to make sure your objects are in UTF-8 encoding. Please refer to R documentation on character encoding and associated functions.

mrchypark commented 4 years ago

Thank you for your guide. I can return correctly with content type.

response$content_type="text/html; charset=UTF-8"
dselivanov commented 4 years ago

@mrchypark I'm glad the issue is resolved, thanks to Simon. Next time please create a separate ticket for a bug report - it will be easier to track.

lorenzwalthert commented 4 years ago

In a previous dev version (can't recall the exact version), it appears to me that the (json) body was not serialized. With the current CRAN release, it is serialized into a list. In my special use case, that's not what I want, because I will serialize the JSON into an sf object instead of a list with geojsonsf::geojson_sf(). With the current CRAN release, I will first need to deserialize and put the list into a string and then use geojsonsf::geojson_sf(). For this reason, I'd love to see a way to supply the serializer function for the body, or, alternatively, not have the body serialized as it seems the case with the parameters (they are just passed as strings).

dselivanov commented 4 years ago

@lorenzwalthert it seems you are confusing terms "serializing" and "deserializing". Serializing to JSON means translating R object into a JSON string. Deserializing means restoring R object from a JSON string. If you want to serialize your R object into something different you can set response$encode = identity as described in the vignette

Check the vignette for a reference specify how to handle encoding and decoding for different content types in EncodeDecodeMiddleware

Keep in mind that default Application constructor receives default EncodeDecodeMiddleware value. And default EncodeDecodeMiddleware decodes application/json into R list. You can amend this decoding using code like:

library(RestRserve)

edmw = EncodeDecodeMiddleware$new()
edmw$ContentHandlers$set_decode("application/json", identity)

app = Application$new(middleware = c(edmw))
app$add_post("/", FUN = function(req, res) {
  print(req$body)
})

r = Request$new(path = "/", method = "POST", body = '{"hello":"world"}', content_type = "application/json")
app$process_request(r)

However I recommend to add another content handler for application/geojson content type in order to not have mysterious bugs related to application/json handling in future.

It may look like a lot of boilerplate coding, but actually it prevents errors associated with "automagic" style.

lorenzwalthert commented 4 years ago

Thanks @dselivanov for your time and code! It works as expected now. Would you consider a PR adding a serializer to RestRserve for geojson? It seems as the application content type would be application/json+geo according to the geojson specification and we could serialize it with the geojsonsf package, either in suggests of imports. Happy to contribute this if you want.

dselivanov commented 4 years ago

@lorenzwalthert thanks for the proposal. As of now I'm not sure about adding geojson serializer to the main RestRserve repo. It will take some time to figure out on what is the best way to add such plugins...

However I would really appreciate if you can create PR with example with geojson encoding/decoding middleware. Same way as we provide other examples in inst/examples

junghwan-yun commented 4 years ago

Hi. I am currently testing our company's production application for this package. Hi. I am currently testing the application of this package to the production. Thank you for designing the package.

Is there a way to adjust the response timeout in RestRserve? If not, I would like to add a document about this content.

dselivanov commented 4 years ago

@junghwan-yun if I understand correctly and you wan to limit response time, you need to put some proxy behind RestRserve and configure timeouts there. For example timeout server in HAproxy.

junghwan-yun commented 4 years ago

@dselivanov Thanks for quick response :)

kfquach commented 4 years ago

Hello, thanks for writing this package! Is there support for processing a .xlsx file? I am familiar with .csv via:

  cnt <- request$get_file("csv")
  dt <- read_csv(cnt)

I've tried something like this without success:

  cnt <- request$get_file("xlsx")
  dt <- xlsx::read.xlsx(cnt, sheetIndex = 1, header = TRUE)

Thank you!

dselivanov commented 4 years ago

@kfquach please post questions on stackoverflow. This thread is not for questions.

kfquach commented 4 years ago

Thank you. I have posted the question here if you can assist.

dselivanov commented 4 years ago

I'm going to close this issue as it becomes off-topic. However we are open for feedback in terms of bug reports and reports related to poor/incomplete documentation.

For other questions please use stackoverflow as this way it will more useful/easily searchable for other users.