Re: question about StatefunContext in golang Statefun SDK

2022-02-23 Thread Galen Warren
Great -- thanks for your help! PR created: [FLINK-26340][statefun-golang-sdk] Add ability in Golang SDK to create new statefun.Context from existing one, but with a new underlying context.Context by galenwarren · Pull Request #303 · apache/flink-statefun (github.com)

Re: question about StatefunContext in golang Statefun SDK

2022-02-23 Thread Austin Cawley-Edwards
I agree with you Galen, adding a default implementation would not be so natural here. We could eventually change this from being an interface to just a struct (like http.Request) if there is no use case for custom implementations. Thanks for the nice discussion here + contribution :) Austin On

Re: question about StatefunContext in golang Statefun SDK

2022-02-23 Thread Galen Warren
Thanks, Till. > Is it possible in Go to provide a default implementation for a method? > Maybe that way the introduction of the new method would not be breaking. > I can't think of a way to do this that would be seamless to someone who had chosen to create a custom struct implementing

Re: question about StatefunContext in golang Statefun SDK

2022-02-23 Thread Till Rohrmann
Thanks a lot for the discussion Austin and Galen. I think it would be fine to break the API at this point in time. Moreover, I don't assume that many people have their own statefun.Context implementations. Is it possible in Go to provide a default implementation for a method? Maybe that way the

Re: question about StatefunContext in golang Statefun SDK

2022-02-22 Thread Galen Warren
Yeah, good point. I wasn't considering that someone else might be implementing that interface. Practically, I think that's pretty unlikely, but good question. On Tue, Feb 22, 2022 at 6:36 PM Austin Cawley-Edwards < austin.caw...@gmail.com> wrote: > I think the only complication with adding

Re: question about StatefunContext in golang Statefun SDK

2022-02-22 Thread Austin Cawley-Edwards
I think the only complication with adding `WithContext` to the interface is that it makes it breaking :/ If it were the concrete implementation, we'd have more flexibility. I'm not sure what guarantees are on those interfaces though – @sjwies...@gmail.com, wdyt? On Tue, Feb 22, 2022 at 4:19 PM

Re: question about StatefunContext in golang Statefun SDK

2022-02-22 Thread Galen Warren
I think I would choose the WithContext method at this point, assuming that the implementation keeps any instance of the stateful context immutable (which should be doable). It's the simplest option IMO, simpler than an adapter approach. Thanks for the suggestion. My slight preference would be to

Re: question about StatefunContext in golang Statefun SDK

2022-02-22 Thread Austin Cawley-Edwards
> > What does "SomeOtherFunc" need with the statefun context > > I think it's hard to answer this question, in a general sense. Depending > on what is being done, it might need to read a value from statefun Storage, > write one back, etc. To me, this indicates that the context is responsible for

Re: question about StatefunContext in golang Statefun SDK

2022-02-22 Thread Galen Warren
> > One place we could look is the `net/http` Request, which has a > `WithContext` method[1] that seems to expose the behavior we're looking > for. > I considered something similar, too, and upon another look, maybe I dismissed it too quickly. The concern I had was that an implementation that

Re: question about StatefunContext in golang Statefun SDK

2022-02-22 Thread Austin Cawley-Edwards
Hey, Sorry for the late response – been off the ML for a few days. I am not too familiar with other Go libs that use a custom context. One place we could look is the `net/http` Request, which has a `WithContext` method[1] that seems to expose the behavior we're looking for. That could be added

Re: question about StatefunContext in golang Statefun SDK

2022-02-22 Thread Galen Warren
Thanks, Seth. I'm curious what you would think about an approach that kept everything as-is, by default, but allowed for a separated context and runtime in the Invoke method, on an opt-in basis, via an adapter? On Tue, Feb 22, 2022 at 10:28 AM Seth Wiesman wrote: > Hi all, > > I believe the

Re: question about StatefunContext in golang Statefun SDK

2022-02-22 Thread Seth Wiesman
Hi all, I believe the discussion revolved around: 1. fewer parameters 2. better aligned with other language sdks 3. we found precedent in other libraries (apologies this was long enough ago I cannot remember which ones, I'm looking through old discussions now) I would in general champion a

Re: question about StatefunContext in golang Statefun SDK

2022-02-22 Thread Till Rohrmann
Hi Galen, Thanks for explaining the problems with the current design. I think I've already learned quite a bit wrt Go thanks to you :-) >From what you describe it seems indeed a bit restrictive to let the statefun.Context contain the context.Context w/o giving access to it. Maybe @Seth Wiesman

Re: question about StatefunContext in golang Statefun SDK

2022-02-21 Thread Galen Warren
So, upon further fiddling, I think it would be possible to keep full backward compatibility and add the option for someone to use an Invoke method with a separate context.Context and statefun.Runtime, via an adapter, if direct manipulation of the context.Context is needed. So, basically, the idea

Re: question about StatefunContext in golang Statefun SDK

2022-02-21 Thread Galen Warren
Was the reason to combine them the desire to have two parameters vs. three, or was there another motivation? On Mon, Feb 21, 2022 at 12:02 PM Seth Wiesman wrote: > FWIW I received a lot of early feedback explicitly asking me to couple the > statefun specific operations with the Context (why the

Re: question about StatefunContext in golang Statefun SDK

2022-02-21 Thread Seth Wiesman
FWIW I received a lot of early feedback explicitly asking me to couple the statefun specific operations with the Context (why the runtime type went away). Seth On Mon, Feb 21, 2022 at 10:32 AM Galen Warren wrote: > Thanks for looking into this! > > The issue I think we'd run into with your

Re: question about StatefunContext in golang Statefun SDK

2022-02-21 Thread Galen Warren
Thanks for looking into this! The issue I think we'd run into with your proposal is that, often, libraries use non-exported types for context keys. Here is an example ; in this case, the non-exported loggerKey{} is used as

Re: question about StatefunContext in golang Statefun SDK

2022-02-21 Thread Till Rohrmann
Thanks a lot for clarifying the problem. I think I now understand the problem. As you've probably figured out, I have no clue about Go and its usage of the Context type. After looking into it a bit I was wondering whether we can't follow a similar route as it is done for the Context type. By

Re: question about StatefunContext in golang Statefun SDK

2022-02-18 Thread Galen Warren
Hmm ... a downside to my proposal is that Go contexts are supposed to be immutable, i.e. when adding a custom value to a context, a new context is created with the new value and the old context isn't changed. Changing the context.Context associated with the statefun.Context sort of goes against

Re: question about StatefunContext in golang Statefun SDK

2022-02-18 Thread Galen Warren
An example of passing it around would be: func (f *MyFunc) Invoke(ctx statefun.Context, message statefun.Message) error { logger := NewLogger() ctx.SetContext(ctxzap.ToContext(ctx, logger)) return SomeOtherFunc(ctx) } func SomeOtherFunc(ctx context.Context) error { logger :=

Re: question about StatefunContext in golang Statefun SDK

2022-02-18 Thread Galen Warren
Ha, our emails keep passing. I've been playing around with options locally, and the SetContext option seems to be the most flexible (and non-breaking), imo. The implementation would be trivial, just add: SetContext(ctx context.Context) ... to the statefun.Context interface, which is

Re: question about StatefunContext in golang Statefun SDK

2022-02-18 Thread Galen Warren
Austin -- what would you think about an option to add SetContext(ctx context.Context) as a method to the statefun.Context interface? Then one could do something like this: func (f *MyFunc) Invoke(ctx statefun.Context, message statefun.Message) error { logger := NewLogger()

Re: question about StatefunContext in golang Statefun SDK

2022-02-18 Thread Austin Cawley-Edwards
It would be helpful to have a small example though, if you have on Galen, to see how you're passing it around. On Fri, Feb 18, 2022 at 11:10 AM Austin Cawley-Edwards < austin.caw...@gmail.com> wrote: > Looking through the statefun Context interface, it indeed doesn't give > access to the

Re: question about StatefunContext in golang Statefun SDK

2022-02-18 Thread Austin Cawley-Edwards
Looking through the statefun Context interface, it indeed doesn't give access to the underlying context.Context and the only implementation is package-private [1]. I don't think there would be a way to update the statfun.Context interface without introducing breaking changes, but if we were to

Re: question about StatefunContext in golang Statefun SDK

2022-02-18 Thread Galen Warren
Sorry Austin, I didn't see your response before I replied. Yes, we're saying the same thing. On Fri, Feb 18, 2022 at 10:56 AM Austin Cawley-Edwards < austin.caw...@gmail.com> wrote: > Hey all, jumping in. This makes sense to me – for instance to attach a > logger with some common metadata, e.g

Re: question about StatefunContext in golang Statefun SDK

2022-02-18 Thread Galen Warren
Sure, you could do that. But it's a common pattern in Go to attach things like loggers, request ids, etc. to the context so that they don't have to be explicitly passed around. It would be convenient to be able to follow this pattern with Statefun. On Fri, Feb 18, 2022 at 10:53 AM Till Rohrmann

Re: question about StatefunContext in golang Statefun SDK

2022-02-18 Thread Austin Cawley-Edwards
Hey all, jumping in. This makes sense to me – for instance to attach a logger with some common metadata, e.g trace ID for the request? This is common in go to add arbitrary items without updating the method signatures, similar to thread local storage in Java. On Fri, Feb 18, 2022 at 10:53 AM Till

Re: question about StatefunContext in golang Statefun SDK

2022-02-18 Thread Till Rohrmann
Thanks for the clarification Galen. If you call the other Go functions, then you could also pass the other values as separate arguments to these functions, can't you? Cheers, Till On Fri, Feb 18, 2022 at 3:31 PM Galen Warren wrote: > The former. > > I think there's potential for confusion here

Re: question about StatefunContext in golang Statefun SDK

2022-02-18 Thread Galen Warren
The former. I think there's potential for confusion here because we're using the word *function *in a couple of senses. One sense is a *stateful function*; another sense is a *Go function*. What I'm looking to do is to put values in the Context so that downstream Go functions that receive the

Re: question about StatefunContext in golang Statefun SDK

2022-02-18 Thread Till Rohrmann
Hi Galen, Am I understanding it correctly, that you would like to set some values in the Context of function A that is then accessible in a downstream call of function B? Or would you like to set a value that is accessible once function A is called again (w/ or w/o the same id)? Cheers, Till On

Re: question about StatefunContext in golang Statefun SDK

2022-02-17 Thread Galen Warren
Also, a potentially simpler way to support this would be to add a SetContext method to the statefun.Context interface, and have it assign the wrapped context. This would not require changes to the function spec, or anything else, and would be more flexible. On Thu, Feb 17, 2022 at 1:05 PM Galen

Re: question about StatefunContext in golang Statefun SDK

2022-02-17 Thread Galen Warren
Thanks for the quick reply! What I'm trying to do is put some things into the context so that they're available in downstream calls, perhaps in methods with pointer receivers to the function struct (MyFunc) but also perhaps in methods that are further downstream that don't have access to MyFunc.

Re: question about StatefunContext in golang Statefun SDK

2022-02-16 Thread Seth Wiesman
Hi Galen, No, that is not currently supported, the current idiomatic way would be to pass those values to the struct implementing the Statefun interface. type MyFunc struct { someRuntimeInfo string } func (m *MyFunc) Invoke(ctx statefun.Context, message statefun.Message) error { } func main() {

Re: question about StatefunContext in golang Statefun SDK

2022-02-16 Thread Galen Warren
Sorry, that should be context.WithValue, not ctx.WithValue ... On Wed, Feb 16, 2022 at 5:35 PM Galen Warren wrote: > When stateful functions are invoked, they are passed an instance of > statefun.Context, which wraps the context.Context received by the HTTP > request. Is there any way to

question about StatefunContext in golang Statefun SDK

2022-02-16 Thread Galen Warren
When stateful functions are invoked, they are passed an instance of statefun.Context, which wraps the context.Context received by the HTTP request. Is there any way to customize that context.Context to, say, hold custom values, using ctx.WithValue()? I don't see a way but I wanted to ask. If not,