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

Allow for async filters #907

Open
king-of-poppk opened this issue Mar 7, 2023 · 15 comments
Open

Allow for async filters #907

king-of-poppk opened this issue Mar 7, 2023 · 15 comments
Labels
answered? Waiting for confirmation that the issue is solved docs Related to documentation only knowledge 🤯 Learn about common {plumber} problems

Comments

@king-of-poppk
Copy link

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?

@meztez
Copy link
Collaborator

meztez commented Nov 24, 2023

@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 meztez added the need info Further information has been requested label Nov 24, 2023
@king-of-poppk
Copy link
Author

@meztez I may be wrong. I will double-check.

@meztez meztez added difficulty: advanced Best for maintainers to address and removed need info Further information has been requested labels Nov 24, 2023
@meztez
Copy link
Collaborator

meztez commented Nov 25, 2023

@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?

@schloerke
Copy link
Collaborator

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.

@meztez
Copy link
Collaborator

meztez commented Nov 25, 2023

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.

@schloerke
Copy link
Collaborator

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
  })

@schloerke
Copy link
Collaborator

Important part is that forward() is called within the main R worker.

@meztez
Copy link
Collaborator

meztez commented Nov 25, 2023

@schloerke Oh, I get it. I thought it needed to be called last but it just sets a flag. Ok, this will work now.

@schloerke
Copy link
Collaborator

schloerke commented Nov 25, 2023

I wish forward() was required to be returned from filter().

Migrating towards that goal is really hard without spooky deprecation message or a separate solution entirely (ex: endpoint based filters).

@meztez
Copy link
Collaborator

meztez commented Nov 25, 2023

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 }

@meztez
Copy link
Collaborator

meztez commented Nov 25, 2023

Important part is that forward() is called within the main R worker.

Yeah, because it sets the flags in the .global env?

@schloerke
Copy link
Collaborator

Important part is that forward() is called within the main R worker.

Yeah, because it sets the flags in the .global env?

Yup!

@meztez meztez added docs Related to documentation only answered? Waiting for confirmation that the issue is solved knowledge 🤯 Learn about common {plumber} problems and removed difficulty: advanced Best for maintainers to address labels Nov 25, 2023
@king-of-poppk
Copy link
Author

Argh this might be one of those XY problem again. I might have tried only with async/await from the coro package, which implies having the forward call inside the future, which won't work.

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 plumber::forward() ahead of the processing is not an option, so I guess this is what it should look like:

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() { ... }
  )

@schloerke
Copy link
Collaborator

I might have tried only with async/await from the coro package, which implies having the forward call inside the future, which won't work.

@king-of-poppk Can you show me a small copy of this code? The async/await logic is built on a state machine which execute in the main R process. I'd be curious how you're using {future} within {coro}.

@king-of-poppk
Copy link
Author

Can you show me a small copy of this code? The async/await logic is built on a state machine which execute in the main R process.

Ah yes of course my mistake, it should have worked then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
answered? Waiting for confirmation that the issue is solved docs Related to documentation only knowledge 🤯 Learn about common {plumber} problems
Projects
None yet
Development

No branches or pull requests

3 participants