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

Feature Request: Sugar for running simple workers and actions #75

Open
digitalbuddha opened this issue May 3, 2020 · 4 comments
Open

Comments

@digitalbuddha
Copy link

Hi folks, I wrote an extension function run which tries to reduce some of the boilerplate associated with running workers. I find myself writing a bit more boilerplate that the minimum when creating a worker that changes state. I'd like the ability to pass a lambda to runningWorker that is an Action.Updater while still receiving the result of my worker.

As an example:

context.runningWorker(
  Worker.from {calendarStore.get(Unit)}) { action { State.ShowingCalendars(it) } 
}

Ideally I wanted to collapse the following to something that asks for a suspended function to execute and a state to show with the response

//does not compile
context.run({calendarStore.get(Unit)}, { State.ShowingCalendars(it) })

I got pretty close but cannot get a working solution that allows me to pass an updater to my run function without having to also pass an action:

context.run({calendarStore.get(Unit)},{ action { State.ShowingCalendars(it) } })

From what I gather (apologies this is all new to me) the issue is with action being unable to pass a param to updater the only reason the above example compiles is that it is being captured from the "handler" without ever getting passed between the action and the action.updater.
For context here is my current solution

inline fun <reified T,StateT, reified OutputT : Any> RenderContext<StateT, OutputT>.run(
        noinline block: suspend () -> T,
        noinline handler: (T) -> WorkflowAction<StateT, OutputT>
    ) {
        runningWorker(Worker.from { block.invoke() }, handler =  handler)
    }

I'm coming from mvrx which has a similar api as I describe above where a single execute function both acts as a mutator and an action. It would be helpful to have the same here.

//mvrx psuedocode
observable.execute{ result:Result
  return NewState(this as State++)
}
@rjrjr
Copy link
Contributor

rjrjr commented May 4, 2020

My concern here is that it's going to be an even bigger temptation to read from the state argument to render instead of the nextState of the WorkflowAction. Your handler lambda really should be receiving a StateT as well as T.

Given that, still want this?

@zach-klippenstein
Copy link
Collaborator

I think my concern is more around obscuring how various calls to the proposed run function would be deduped (even after adding a key parameter). Hiding the fact that there is a Worker here makes it even more magical that two run calls with the same type are considered equivalent, but with a different return type are considered separate. One of the most confusing bits of the API is how workers are deduped, even when they're explicit. I'm not sure if this would make that significantly more confusing, or just marginally confusing relative to how much it already is.

@zach-klippenstein
Copy link
Collaborator

Also, to clarify the discussion going forward, this proposes two separate and unrelated things:

  1. Inlines Worker.from (which is what my concern above is about)
  2. Inlines action (which is what Ray's concern is about, I think). We've also had other proposals about doing the latter (cc @steveinflow)

@digitalbuddha
Copy link
Author

@rjrjr Yup! The handler should be receiving both the result of the worker as well as the state. I am mostly looking for a way to make the common case of get data from a service and make a new state based on the result a bit shorter to write.

@zach-klippenstein zach-klippenstein changed the title Question/Feature Request Feature Request: Sugar for running simple workers and actions Jun 11, 2020
@rjrjr rjrjr transferred this issue from square/workflow Jun 28, 2020
zach-klippenstein added a commit that referenced this issue Feb 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants