You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flink.apache.org by Galen Warren <ga...@cvillewarrens.com> on 2022/02/24 21:58:14 UTC
possible bug in Statefun Golang SDK related to Context.Caller
On the Context interface, Caller is defined like this:
// Caller is the caller function instance's Address, if applicable. This is
nil
// if the message was sent to this function via an ingress.
Caller() *Address
... so, in the event a function is invoked from an ingress, as opposed to
from another function, the returned value should be nil.
However, it seems that the statefunContext.caller property -- the value of
which is returned by Caller() -- is always set to a non-nil value when the
context is constructed. From handler.go:
sContext := statefunContext{
Mutex: new(sync.Mutex),
self: self,
storage: storage,
response: response,
}
var cancel context.CancelFunc
sContext.Context, cancel = context.WithCancel(ctx)
* var caller Address*
* if invocation.Caller != nil {*
* caller = addressFromInternal(invocation.Caller)*
* }*
* sContext.caller = &caller*
So, even when there is no caller associated with the invocation, the caller
field is set to a non-nil pointer, and the referenced Address has
FunctionType=nil and Id="", which is then returned by Context.Caller().
Should the bolded code instead be the following, to only assign
sContext.caller when the invocation has an associated caller?
if invocation.Caller != nil {
caller := addressFromInternal(invocation.Caller)
sContext.caller = &caller
}
I'd be happy to submit a fix if needed ... thanks.
Re: possible bug in Statefun Golang SDK related to Context.Caller
Posted by Galen Warren <ga...@cvillewarrens.com>.
PR created: [FLINK-26375][statefun-golang-sdk] Fix Statefun Golang SDK to
return nil from Context.Caller when there is no caller by galenwarren ·
Pull Request #304 · apache/flink-statefun (github.com)
<https://github.com/apache/flink-statefun/pull/304>
On Fri, Feb 25, 2022 at 9:53 AM Galen Warren <ga...@cvillewarrens.com>
wrote:
> Thanks! I created a JIRA: [FLINK-26375] Fix Statefun Golang SDK to return
> nil from Context.Caller when there is no caller - ASF JIRA (apache.org)
> <https://issues.apache.org/jira/browse/FLINK-26375>
>
> ... and will update soon with the PR.
>
> On Fri, Feb 25, 2022 at 3:01 AM Igal Shilman <ig...@apache.org> wrote:
>
>> Hi Galen,
>>
>> I think that you are right this does seem like a bug, looking at the Java
>> SDK, we replace an empty address with a null value.[1]
>> We should do the same here.
>> I've also checked the Python SDK[2], and the JS SDK[3] and they have the
>> same problem.
>>
>> Looking forward to your contribution!
>>
>> Thanks,
>> Igal.
>>
>>
>> [1]
>>
>> https://github.com/apache/flink-statefun/blob/725202cd69b78442d1287deec000ae2d52da4bda/statefun-sdk-java/src/main/java/org/apache/flink/statefun/sdk/java/handler/ProtoUtils.java#L49,L51
>> [2]
>>
>> https://github.com/apache/flink-statefun/blob/725202cd69b78442d1287deec000ae2d52da4bda/statefun-sdk-python/statefun/request_reply_v3.py#L115,L122
>> [3]
>>
>> https://github.com/apache/flink-statefun/blob/725202cd69b78442d1287deec000ae2d52da4bda/statefun-sdk-js/src/handler.ts#L240,L244
>>
>> On Thu, Feb 24, 2022 at 10:58 PM Galen Warren <ga...@cvillewarrens.com>
>> wrote:
>>
>> > On the Context interface, Caller is defined like this:
>> >
>> > // Caller is the caller function instance's Address, if applicable.
>> This is
>> > nil
>> > // if the message was sent to this function via an ingress.
>> > Caller() *Address
>> >
>> > ... so, in the event a function is invoked from an ingress, as opposed
>> to
>> > from another function, the returned value should be nil.
>> >
>> > However, it seems that the statefunContext.caller property -- the value
>> of
>> > which is returned by Caller() -- is always set to a non-nil value when
>> the
>> > context is constructed. From handler.go:
>> >
>> > sContext := statefunContext{
>> > Mutex: new(sync.Mutex),
>> > self: self,
>> > storage: storage,
>> > response: response,
>> > }
>> >
>> > var cancel context.CancelFunc
>> > sContext.Context, cancel = context.WithCancel(ctx)
>> >
>> > * var caller Address*
>> > * if invocation.Caller != nil {*
>> > * caller = addressFromInternal(invocation.Caller)*
>> > * }*
>> > * sContext.caller = &caller*
>> >
>> > So, even when there is no caller associated with the invocation, the
>> caller
>> > field is set to a non-nil pointer, and the referenced Address has
>> > FunctionType=nil and Id="", which is then returned by Context.Caller().
>> >
>> > Should the bolded code instead be the following, to only assign
>> > sContext.caller when the invocation has an associated caller?
>> >
>> > if invocation.Caller != nil {
>> > caller := addressFromInternal(invocation.Caller)
>> > sContext.caller = &caller
>> > }
>> >
>> > I'd be happy to submit a fix if needed ... thanks.
>> >
>>
>
Re: possible bug in Statefun Golang SDK related to Context.Caller
Posted by Galen Warren <ga...@cvillewarrens.com>.
Thanks! I created a JIRA: [FLINK-26375] Fix Statefun Golang SDK to return
nil from Context.Caller when there is no caller - ASF JIRA (apache.org)
<https://issues.apache.org/jira/browse/FLINK-26375>
... and will update soon with the PR.
On Fri, Feb 25, 2022 at 3:01 AM Igal Shilman <ig...@apache.org> wrote:
> Hi Galen,
>
> I think that you are right this does seem like a bug, looking at the Java
> SDK, we replace an empty address with a null value.[1]
> We should do the same here.
> I've also checked the Python SDK[2], and the JS SDK[3] and they have the
> same problem.
>
> Looking forward to your contribution!
>
> Thanks,
> Igal.
>
>
> [1]
>
> https://github.com/apache/flink-statefun/blob/725202cd69b78442d1287deec000ae2d52da4bda/statefun-sdk-java/src/main/java/org/apache/flink/statefun/sdk/java/handler/ProtoUtils.java#L49,L51
> [2]
>
> https://github.com/apache/flink-statefun/blob/725202cd69b78442d1287deec000ae2d52da4bda/statefun-sdk-python/statefun/request_reply_v3.py#L115,L122
> [3]
>
> https://github.com/apache/flink-statefun/blob/725202cd69b78442d1287deec000ae2d52da4bda/statefun-sdk-js/src/handler.ts#L240,L244
>
> On Thu, Feb 24, 2022 at 10:58 PM Galen Warren <ga...@cvillewarrens.com>
> wrote:
>
> > On the Context interface, Caller is defined like this:
> >
> > // Caller is the caller function instance's Address, if applicable. This
> is
> > nil
> > // if the message was sent to this function via an ingress.
> > Caller() *Address
> >
> > ... so, in the event a function is invoked from an ingress, as opposed to
> > from another function, the returned value should be nil.
> >
> > However, it seems that the statefunContext.caller property -- the value
> of
> > which is returned by Caller() -- is always set to a non-nil value when
> the
> > context is constructed. From handler.go:
> >
> > sContext := statefunContext{
> > Mutex: new(sync.Mutex),
> > self: self,
> > storage: storage,
> > response: response,
> > }
> >
> > var cancel context.CancelFunc
> > sContext.Context, cancel = context.WithCancel(ctx)
> >
> > * var caller Address*
> > * if invocation.Caller != nil {*
> > * caller = addressFromInternal(invocation.Caller)*
> > * }*
> > * sContext.caller = &caller*
> >
> > So, even when there is no caller associated with the invocation, the
> caller
> > field is set to a non-nil pointer, and the referenced Address has
> > FunctionType=nil and Id="", which is then returned by Context.Caller().
> >
> > Should the bolded code instead be the following, to only assign
> > sContext.caller when the invocation has an associated caller?
> >
> > if invocation.Caller != nil {
> > caller := addressFromInternal(invocation.Caller)
> > sContext.caller = &caller
> > }
> >
> > I'd be happy to submit a fix if needed ... thanks.
> >
>
Re: possible bug in Statefun Golang SDK related to Context.Caller
Posted by Igal Shilman <ig...@apache.org>.
Hi Galen,
I think that you are right this does seem like a bug, looking at the Java
SDK, we replace an empty address with a null value.[1]
We should do the same here.
I've also checked the Python SDK[2], and the JS SDK[3] and they have the
same problem.
Looking forward to your contribution!
Thanks,
Igal.
[1]
https://github.com/apache/flink-statefun/blob/725202cd69b78442d1287deec000ae2d52da4bda/statefun-sdk-java/src/main/java/org/apache/flink/statefun/sdk/java/handler/ProtoUtils.java#L49,L51
[2]
https://github.com/apache/flink-statefun/blob/725202cd69b78442d1287deec000ae2d52da4bda/statefun-sdk-python/statefun/request_reply_v3.py#L115,L122
[3]
https://github.com/apache/flink-statefun/blob/725202cd69b78442d1287deec000ae2d52da4bda/statefun-sdk-js/src/handler.ts#L240,L244
On Thu, Feb 24, 2022 at 10:58 PM Galen Warren <ga...@cvillewarrens.com>
wrote:
> On the Context interface, Caller is defined like this:
>
> // Caller is the caller function instance's Address, if applicable. This is
> nil
> // if the message was sent to this function via an ingress.
> Caller() *Address
>
> ... so, in the event a function is invoked from an ingress, as opposed to
> from another function, the returned value should be nil.
>
> However, it seems that the statefunContext.caller property -- the value of
> which is returned by Caller() -- is always set to a non-nil value when the
> context is constructed. From handler.go:
>
> sContext := statefunContext{
> Mutex: new(sync.Mutex),
> self: self,
> storage: storage,
> response: response,
> }
>
> var cancel context.CancelFunc
> sContext.Context, cancel = context.WithCancel(ctx)
>
> * var caller Address*
> * if invocation.Caller != nil {*
> * caller = addressFromInternal(invocation.Caller)*
> * }*
> * sContext.caller = &caller*
>
> So, even when there is no caller associated with the invocation, the caller
> field is set to a non-nil pointer, and the referenced Address has
> FunctionType=nil and Id="", which is then returned by Context.Caller().
>
> Should the bolded code instead be the following, to only assign
> sContext.caller when the invocation has an associated caller?
>
> if invocation.Caller != nil {
> caller := addressFromInternal(invocation.Caller)
> sContext.caller = &caller
> }
>
> I'd be happy to submit a fix if needed ... thanks.
>