Use of SinkFunction.Context<T> without generic parameter.

[Niels Basjes]

Hi,

I prefer to have my code as "warning free" as possible so I was looking
through some of the warnings my IDE gives about some of the Flink code I'm
working on when I ran into this:

Raw use of parameterized class 'SinkFunction.Context'


Apparently the interface for a SinkFunction has the method

void invoke(IN value, Context context)


https://github.com/apache/flink/blob/master/flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/SinkFunction.java#L51

So *Context* is used here as a normal class.
Yet the actual definition (in the same file, about 15 lines below) shows

interface Context<T> {
which is a generic.
https://github.com/apache/flink/blob/master/flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/SinkFunction.java#L66

So in all SinkFunction implementations everywhere this issues a warning in
the code (also in what I'm working on).

I cannot simply fix my code by adding the type parameters because that
makes my code no longer implement the interface.


'invoke(IN, Context<IN>)' in
'org.apache.flink.streaming.connectors.gcp.pubsub.PubSubSink' clashes with
'invoke(IN, Context)' in
'org.apache.flink.streaming.api.functions.sink.SinkFunction'; both methods
have same erasure, yet neither overrides the other


I tried to simply add the extra function to the SinkFunction
void invoke(IN value, Context<IN> context)

which fails with

'invoke(IN, Context)' clashes with 'invoke(IN, Context<IN>)'; both methods
have same erasure


So they are "same enough" to clash and "different enough" to not be seen as
an implementation of the interface.

So as far as I can see the 'only' way to fix this is either a breaking API
change and cleanly add this generic parameter everywhere OR remove it from
the interface.

Note that the Context<T> interface does not use the generic parameter and
has the comment
// Interface might be extended in the future with additional methods.

I read this as future extensibility which has not been implemented (yet)
and which has not been placed in all code correctly.

What do you guys think of this ?

I'm inclined to simply remove the generic type parameter from this
interface.

--
Best regards / Met vriendelijke groeten,

Niels Basjes

[Aljoscha Krettek-2]

Hi,

this was actually my mistake back then. :-O

I'm open to removing the generic parameter from Context if we are sure that it won't break user code. I think it doesn't, because you cannot actually use it with the generic parameter, as you found. Also, I think custom sink implementations in user code are somewhat rare.

Best,
Aljoscha

On Mon, Jul 13, 2020, at 17:03, Niels Basjes wrote:

> Hi,
>
> I prefer to have my code as "warning free" as possible so I was looking
> through some of the warnings my IDE gives about some of the Flink code I'm
> working on when I ran into this:
>
> Raw use of parameterized class 'SinkFunction.Context'
>
>
> Apparently the interface for a SinkFunction has the method
>
> void invoke(IN value, Context context)
>
>
> https://github.com/apache/flink/blob/master/flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/SinkFunction.java#L51
>
> So *Context* is used here as a normal class.
> Yet the actual definition (in the same file, about 15 lines below) shows
>
> interface Context<T> {
> which is a generic.
> https://github.com/apache/flink/blob/master/flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/SinkFunction.java#L66
>
> So in all SinkFunction implementations everywhere this issues a warning in
> the code (also in what I'm working on).
>
> I cannot simply fix my code by adding the type parameters because that
> makes my code no longer implement the interface.
>
>
> 'invoke(IN, Context<IN>)' in
> 'org.apache.flink.streaming.connectors.gcp.pubsub.PubSubSink' clashes with
> 'invoke(IN, Context)' in
> 'org.apache.flink.streaming.api.functions.sink.SinkFunction'; both methods
> have same erasure, yet neither overrides the other
>
>
> I tried to simply add the extra function to the SinkFunction
> void invoke(IN value, Context<IN> context)
>
> which fails with
>
> 'invoke(IN, Context)' clashes with 'invoke(IN, Context<IN>)'; both methods
> have same erasure
>
>
> So they are "same enough" to clash and "different enough" to not be seen as
> an implementation of the interface.
>
> So as far as I can see the 'only' way to fix this is either a breaking API
> change and cleanly add this generic parameter everywhere OR remove it from
> the interface.
>
> Note that the Context<T> interface does not use the generic parameter and
> has the comment
> // Interface might be extended in the future with additional methods.
>
> I read this as future extensibility which has not been implemented (yet)
> and which has not been placed in all code correctly.
>
> What do you guys think of this ?
>
> I'm inclined to simply remove the generic type parameter from this
> interface.
>
> --
> Best regards / Met vriendelijke groeten,
>
> Niels Basjes
>

[Niels Basjes]

Hi,

Cool. I'm going to put up a fix (
https://issues.apache.org/jira/browse/FLINK-18606 )
I think that even people who ARE making their own Sink are not affected
because the method signature of the invoke does not use the parameter.

Niels.

On Wed, Jul 15, 2020 at 11:32 AM Aljoscha Krettek <[hidden email]>
wrote:

> Hi,
>
> this was actually my mistake back then. :-O
>
> I'm open to removing the generic parameter from Context if we are sure
> that it won't break user code. I think it doesn't, because you cannot
> actually use it with the generic parameter, as you found. Also, I think
> custom sink implementations in user code are somewhat rare.
>
> Best,
> Aljoscha
>
> On Mon, Jul 13, 2020, at 17:03, Niels Basjes wrote:
> > Hi,
> >
> > I prefer to have my code as "warning free" as possible so I was looking
> > through some of the warnings my IDE gives about some of the Flink code
> I'm
> > working on when I ran into this:
> >
> > Raw use of parameterized class 'SinkFunction.Context'
> >
> >
> > Apparently the interface for a SinkFunction has the method
> >
> > void invoke(IN value, Context context)
> >
> >
> >
> https://github.com/apache/flink/blob/master/flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/SinkFunction.java#L51
> >
> > So *Context* is used here as a normal class.
> > Yet the actual definition (in the same file, about 15 lines below) shows
> >
> > interface Context<T> {
> > which is a generic.
> >
> https://github.com/apache/flink/blob/master/flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/SinkFunction.java#L66
> >
> > So in all SinkFunction implementations everywhere this issues a warning
> in
> > the code (also in what I'm working on).
> >
> > I cannot simply fix my code by adding the type parameters because that
> > makes my code no longer implement the interface.
> >
> >
> > 'invoke(IN, Context<IN>)' in
> > 'org.apache.flink.streaming.connectors.gcp.pubsub.PubSubSink' clashes
> with
> > 'invoke(IN, Context)' in
> > 'org.apache.flink.streaming.api.functions.sink.SinkFunction'; both
> methods
> > have same erasure, yet neither overrides the other
> >
> >
> > I tried to simply add the extra function to the SinkFunction
> > void invoke(IN value, Context<IN> context)
> >
> > which fails with
> >
> > 'invoke(IN, Context)' clashes with 'invoke(IN, Context<IN>)'; both
> methods
> > have same erasure
> >
> >
> > So they are "same enough" to clash and "different enough" to not be seen
> as
> > an implementation of the interface.
> >
> > So as far as I can see the 'only' way to fix this is either a breaking
> API
> > change and cleanly add this generic parameter everywhere OR remove it
> from
> > the interface.
> >
> > Note that the Context<T> interface does not use the generic parameter and
> > has the comment
> > // Interface might be extended in the future with additional methods.
> >
> > I read this as future extensibility which has not been implemented (yet)
> > and which has not been placed in all code correctly.
> >
> > What do you guys think of this ?
> >
> > I'm inclined to simply remove the generic type parameter from this
> > interface.
> >
> > --
> > Best regards / Met vriendelijke groeten,
> >
> > Niels Basjes
> >
>

--
Best regards / Met vriendelijke groeten,

Niels Basjes