Pass ctx from func to func

[skonto]

Right now a custom ctx is used in order to call the API funcs and keep state internally. In Golang ctx is expected to be passed from func to func. I think Golang ctx could be initiated at the http.serve side and then pass it down to process etc.
This would be a more idiomatic use and more aligned on how context is used in the other sdks.

[sjwiesman]

Hmm, I wasn't aware of that interface. I don't think what I'm doing is well served by that interface and maybe it makes sense to rename statefun.Context to something else?

[skonto]

@sjwiesman If you want to follow Go's practices you can easily re-use the already provided context (not sure why it does not work for you), otherwise you need to rename the custom one because it it a bit confusing.

[sjwiesman]

Maybe I'm missing something, but looking at this interface:

type Context interface {
    //It retures a channel when a context is cancelled, timesout (either when deadline is reached or timeout time has finished)
    Done() <-chan struct{}

    //Err will tell why this context was cancelled. A context is cancelled in three scenarios.
    // 1. With explicit cancellation signal
    // 2. Timeout is reached
    // 3. Deadline is reached
    Err() error

    //Used for handling deallines and timeouts
    Deadline() (deadline time.Time, ok bool)

    //Used for passing request scope values
    Value(key interface{}) interface{}
}

I don't see a use for these methods in an implementation of the StatefulFunction interface. In particular, based on the documenation I've seen the context is meant to be immutable which is different than what we need. Whether or not it’s reused I see as orthogonal. I am open to renaming the argument to something else.
Maybe @igalshilman has an opinion.

[skonto]

@sjwiesman you could use a method from the API like WithValue to store key,value pairs. I saw that you keep some values around as state so this is one option of how to use this. Immutability is a concern if you copy a lot of data, but from what I see you keep pointers in the custom context struct, so copying pointers does not seem too bad on the average case anyway.
In addition afaik you create a context when a request is served here and since Gloang's context is request scoped I thought it could be used here. Anyway a custom context is fine if suits your design just wanted to point out some similarities here and why it can be confusing. Btw to be clear I was not stating to use this Gloang context as a drop in replacement for what you have. I am saying if you want pass state per request this is an option. That means:

func (ctx *context) Get(name string) (*any.Any, error) {  

can be re-written as:

func Get(ctx Context, name string) (*any.Any, error) {

Note that if you want to keep state beyond the scope of a request this is not suitable.

Maybe the latter was not clear from my side ;)

[igalshilman]

Hi @skonto,
First of all welcome to the project 🥳 , and thank you for sharing your Go experience/idiomatic ways of doing things!

Let me take a step back and describe what is going on:

1. Currently the only support protocol of invoking remote functions is a RequestReply.
2. This protocol sends all the information that the remote function needs together with its argument, and expects to receive a list of mutations to the state, together with any additional messages to be sent.
3. The StatefulFunctionContext is indeed scoped to a single HTTP request (in the RequestReply protocol) and it captures the state sent, and it keeps track of any state mutation (in the scope of a single HTTP request) and any outgoing messages.
4. When the request ends, the list of mutations and messages is extract from the context, and is serialized as part of the response.

So, I do think that there is a need for a class like that to exist, to capture all of these concerns.

Now for the Go context, to my understanding the Go context is used for:

1. coordinated cancelation / timeout of a group of coroutines.
2. passing in additional values with a narrow scope (per request)

It seems like (1) is pretty idiomatic in Go, especially If the user function would spawn different go-routines, so it seems logical to provide a child context to the stateful functions.
But we still need to account for StatefulFunctionContext responsibilities, so one suggestion would be:

1. to split the StatefulFunctionContext to StateStore and Output where the StateStore captures the responsibilities of managing state and the Output would capture the responsibilities of sending messages to other functions/egresses.
2. Then we attach these two objects to the child request context by using WithValue(..) with constant key names.

The end result might look like this:
states, err = ctx.Get(statefun.StateStoreKey)

and
out, err = ctx.Get(statefun.OutputKey)

This would give us both idiomatic usage of the Go context, and we keep managing the previous responsibilities.

[sjwiesman]

Resolved in aca3da2