Hi everyone,
sorry for joining the discussion late.
1) Is there an option to deprecate SinkFunction in Flink 1.17 while leaving
SinkV2 @PublicEvolving in Flink 1.17. We then aim to make SinkV2 @Public in
and remove SinkFunction in Flink 1.18. @PublicEvolving are intended for
public use. So, I don't see it as a blocker for deprecating SinkFunction
that we have to make SinkV2 @PublicEvovling. For reference this is the
description of @PublicEvovling:
/**
* Annotation to mark classes and methods for public use, but with
evolving interfaces.
*
* <p>Classes and methods with this annotation are intended for public
use and have stable behavior.
* However, their interfaces and signatures are not considered to be
stable and might be changed
* across versions.
*
* <p>This annotation also excludes methods and classes with evolving
interfaces / signatures within
* classes annotated with {@link Public}.
*/
Marking SinkFunction @Deprecated would already single everyone to move to
SinkV2, which we as a community, I believe, have a strong interest in. Its
almost comical how long the transition from SourceFurnction/SinkFunction to
Source/Sink takes us. At the same time, we leave ourselves the option to to
make small changes to SinkV2 if any problems arise during the migration of
these connector.
I think, we have a bit of a chicken/egg problem here. The pressure for
users and contributors is not high enough to move away from SinkFunction as
long as its not deprecated, but at the same time we need people to migrate
their connectors to see if there are any gaps in SinkV2. I believe, the
combination proposed above could bridge this problem.
2) I don't understand the argument of waiting until some of the
implementations are @Public. How can we make the implementations of the
SinkV2 API @Public without making SinkV2 @Public? All public methods of
SinkV2 are part of every implementation. So to me it actually seems to be
opposite: in order to make any of the implementation @Public we first need
to make the API @Public.
Cheers,
Konstantin
Am Mo., 30. Jan. 2023 um 13:18 Uhr schrieb Dong Lin <lindon...@gmail.com>:
> Hi Martijn,
>
> Thanks for driving this effort to clean-up the Flink codebase!
>
> I like the idea to cleanup Flink codebase to avoid having two Sinks. On the
> other hand, I also thing the concern mentioned by Jing makes sense. In
> addition to thinking in terms of the rule proposed in FLIP-197
> <
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-197%3A+API+stability+graduation+process
> >
> (which
> seems to focus mostly on the Flink developers' perspective), it might be
> useful to also think about the story from users' perspective and make sure
> their concerns can be addressed.
>
> Typically, by marking an API as deprecated, we are effectively telling
> users *they should start to migrate to the new API ASAP and we reserve the
> right to remove this API completely in the 1-2 releases*. Then it might be
> reasonable for users to ask questions such as:
>
> 1) Does SinkV2 public API provides all the functionalities needed to
> migrate my existing code from subclassing SinkFunction to subclassing
> SinkV2?
>
> 2) Is the amount of migration work reasonable? If yes, why is a class such
> as HBaseSinkFunction in Flink's own codebase still depending on
> SinkFunction? Maybe Flink developers should eat their own dog food and
> migrate (or deprecate) those classes in the Flink codebase first?
>
> Based on the discussion in this thread, I am not sure we have good answers
> to those questions yet. For the 1st question above, the answer is *no*
> because we already know that the SinkV2 is currently not able to support
> migration for JdbcSink. For the 2nd question above, we know there are many
> non-deprecated class in Flink codebase that are still depending on SinkV2.
> It is probably not nice to users if we tell them to migrate while we know
> there are existing issues that can prevent them from doing so easily.
>
> In order to move forward to deprecate SinkV2, I think it will be super
> useful to first migrate all the connectors managed by Flink community
> (including all externalized connectors) to use SinkV2. This work won't be
> wasted since we need to do this anyway. And it will also give us a chance
> to validate the capabilities of SinkV2 and fix bugs by ourselves as much as
> possible.
>
> What do you think?
>
> Best Regards,
> Dong
>
>
> On Wed, Jan 18, 2023 at 6:52 PM Martijn Visser <martijnvis...@apache.org>
> wrote:
>
> > Hi all,
> >
> > While discussing FLIP-281 [1] the discussion also turned to the
> > SinkFunction and the SinkV2 API. For a broader discussion I'm opening up
> a
> > separate discussion thread.
> >
> > As Yun Tang has mentioned in that discussion thread, it would be a good
> > time to deprecate the SinkFunction to avoid the need to introduce new
> > functions towards (to be) deprecated APIs. Jing rightfully mentioned that
> > it would be confusing to deprecate the SinkFunction if its successor is
> not
> > yet marked as @Public (it's currently @PublicEvolving).
> >
> > My proposal would be to promote the SinkV2 API to @public in Flink 1.17
> and
> > mark the SinkFunction as @deprecated in Flink 1.17
> >
> > The original Sink interface was introduced in Flink 1.12 with FLIP-143
> [2]
> > and extended with FLIP-177 in Flink 1.14 [3] and has been improved on
> > further as Sink V2 via FLIP-191 in Flink 1.15 [4].
> >
> > Looking at the API stability graduation process [5], the fact that Sink
> V2
> > was introduced in Flink 1.15 would mean that we could warrant a promotion
> > to @public already (given that there have been two releases with 1.15 and
> > 1.16 where it was introduced). Combined with the fact that SinkV2 has
> been
> > the result of iteration over the introduction of the original Sink API
> > since Flink 1.12, I would argue that the promotion is overdue.
> >
> > If we promote the Sink API to @public, I think we should also immediately
> > mark the SinkFunction as @deprecated.
> >
> > Looking forward to your thoughts.
> >
> > Best regards,
> >
> > Martijn
> >
> >
> > [1] https://lists.apache.org/thread/l05m6cf8fwkkbpnjtzbg9l2lo40oxzd1
> > [2]
> >
> >
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-143%3A+Unified+Sink+API
> > [3]
> >
> >
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-177%3A+Extend+Sink+API
> > [4]
> >
> >
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-191%3A+Extend+unified+Sink+interface+to+support+small+file+compaction
> > [5]
> >
> >
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-197%3A+API+stability+graduation+process
> >
>
--
https://twitter.com/snntrable
https://github.com/knaufk
