-
Notifications
You must be signed in to change notification settings - Fork 255
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
Allow for async filters #907
Comments
@king-of-poppk how would it be used? Why would it not work currently? Aren't filters are just regular steps according to runSteps source? |
@meztez I may be wrong. I will double-check. |
@king-of-poppk I did some further testing and I do not think plumber::forward could be used within a future::future. What would be the use case? |
Correct. You can have a future return and then call forward() within the follow up promise. So if the filter could return a promise and within that promise (which executes in the main R worker) it calls forward(), I think it'll work. |
library(plumber)
library(future)
plan("multisession")
#* @filter future
function(req) {
print(req$PATH_INFO)
if (req$PATH_INFO=="/echo1") {
future::future({
Sys.sleep(10)
plumber::forward()
})
} else {
plumber::forward()
}
}
#* @serializer text
#* @get /echo1
function(msg) {
msg
}
#* @serializer text
#* @get /echo2
function(msg) {
msg
}
I'm working from this, only getting a json serialized true. I will try some things from what I understand from your comment. |
I'm on mobile, so formatting will not be good / tested. Updating your future code to something like this... future::future({
if (req$PATH_INFO=="/echo1") {
Sys.sleep(10)
}
}) %...>% {
value <- .
plumber::forward()
value
} (I hope I'm not making stuff up here 😅) I believe you could even call forward before the promise is returned. plumber::forward()
future::future({
if (req$PATH_INFO=="/echo1") {
Sys.sleep(10)
}
# return data here
}) |
Important part is that |
@schloerke Oh, I get it. I thought it needed to be called last but it just sets a flag. Ok, this will work now. |
I wish Migrating towards that goal is really hard without spooky deprecation message or a separate solution entirely (ex: endpoint based filters). |
Yep it works @king-of-poppk , let me know if this is something you can use. library(plumber)
library(future)
plan("multisession")
#* @filter future
function(req) {
plumber::forward()
future::future({
if (req$PATH_INFO=="/echo1") {
Sys.sleep(10)
}
})
}
#* @serializer text
#* @get /echo1
function(msg) { msg }
#* @serializer text
#* @get /echo2
function(msg) { msg }
|
Yeah, because it sets the flags in the .global env? |
Yup! |
Argh this might be one of those XY problem again. I might have tried only with The goal here is to make the entire filter processing potentially run in parallel, and conditionally forward based on the results of the processing. Calling function myAuthorizationFilter(req, res) {
myAuthorizationFilterLogic(req) %>% then(
onFulfilled = function(value) {
plumber::forward()
},
onRejected = function(error) {
res$status <- 403 # NOTE Or 401 depending on error class.
}
) %>% finally(
function() { ... }
) |
@king-of-poppk Can you show me a small copy of this code? The |
Ah yes of course my mistake, it should have worked then. |
With #204 implemented we now have a pretty convenient way to parallelize IO resource consumption. However, it seems plumber filters still do not have this feature. Would it be possible to make it so that filters can return promises for the
plumber::forward()
call?The text was updated successfully, but these errors were encountered: