You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flink.apache.org by Niels Basjes <Ni...@basjes.nl> on 2020/07/13 15:03:00 UTC
Use of SinkFunction.Context without generic parameter.
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
Re: Use of SinkFunction.Context without generic parameter.
Posted by Niels Basjes <Ni...@basjes.nl>.
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 <al...@apache.org>
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
Re: Use of SinkFunction.Context without generic parameter.
Posted by Aljoscha Krettek <al...@apache.org>.
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
>