Hi Steven, Sink is already deprecated. It was deprecated at the moment where we introduced SinkV2.
Hi Jark, Hi Dong, My understanding is the SinkV2 is a workable interface. The most important connectors have been migrated (Kafka, Filesystem) and more connectors (OpenSearch, ElasticSearch, Kinesis) use it successfully. To make SinkV2 public, it does not need to have all possible functionality. Public APIs can be extended. That's what we do all the time. There will also always be bugs. So, these points can not be categorical blockers to promote the API. What are the specific issues/tickets that are blocking us? Can we in your opinion only deprecate it when every single connector in Apache Flink is migrated already? In my opinion it is the time to ask users to the migrate their connectors. More importantly, @Deprecated would signal users not to build new connectors on SinkFunction. I would arque its also very misleading to users to not @Deprecated SinkFunction given that is clearly will be deprecated. Cheers, Konstantin Am Mo., 6. Feb. 2023 um 13:26 Uhr schrieb Jark Wu <imj...@gmail.com>: > I agree with Dong Lin. > > Oracle explains how to use Deprecate API [1]: > > You are strongly recommended to use the Javadoc @deprecated tag with > > appropriate comments explaining how to use the new API. This ensures > > developers will *have a workable migration path from the old API to the > > new API*. > > > From a user's perspective, the workable migration path is very important. > Otherwise, it blurs the semantics of API deprecation. The Flink API's > compatibility and stability issues in the past left a bad impression on the > downstream projects. We should be careful when changing and deprecating > APIs, especially when there are known migration gaps. I think it's a good > idea to migrate Flink-owned connectors before marking old API deprecated. > This ensures downstream projects can migrate to new APIs smoothly. > > Best, > Jark > > [1]: > > https://docs.oracle.com/javase/8/docs/technotes/guides/javadoc/deprecation/deprecation.html > > On Mon, 6 Feb 2023 at 10:01, Steven Wu <stevenz...@gmail.com> wrote: > > > Regarding the discussion on global committer [1] for sinks with global > > transactions, there is no consensus on solving that problem in SinkV2. > Will > > it require any breaking change in SinkV2? > > > > Also will SinkV1 be deprecated too? or it should happen sometime after > > SinkFunction deprecation? > > > > [1] https://lists.apache.org/thread/82bgvlton9olb591bfg2djv0cshj1bxj > > > > On Sun, Feb 5, 2023 at 2:14 AM Dong Lin <lindon...@gmail.com> wrote: > > > > > Hi Konstantin, > > > > > > Thanks for the comment! Please see my comment inline. > > > > > > Cheers, > > > Dong > > > > > > On Sat, Feb 4, 2023 at 2:06 AM Konstantin Knauf <kna...@apache.org> > > wrote: > > > > > > > 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 > > > > > > > > > > Yes, I also believe we all have this strong interest. I just hope that > > this > > > can be done in the best possible way that does not confuse users. > > > > > > I probably still have the same concern regarding its impact on users: > if > > we > > > mark an API as deprecated, it effectively means the users of this API > > > should start to migrate to another API (e.g. SinkV2) and we might > remove > > > this API in the future. However, given that we know there are known > > > problems preventing users from doing so, it seems that we are not ready > > to > > > send this message to users right. > > > > > > If I understand correctly, I guess you are suggesting that by marking > > > SinkFunction as deprecated, we can put higher pressure on Flink > > > contributors to update the existing Flink codebase to improve and use > > > SinkV2. > > > > > > I am not sure this is the right way to use @deprecated, which has a > > > particular meaning for its users rather than contributors. And I am > also > > > not sure we can even pressure contributors of an open-source project > into > > > developing a feature (e.g. migrate all existing SinkFunction subclasses > > to > > > SinkV2). IMO, the typical way is for the contributor with interest/time > > to > > > work on the feature, or talk to other contributors whether they are > > willing > > > to collaborate/work on this, rather than pressuring other contributors > > into > > > working on this. > > > > > > > > > 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 > > > > > > > > > > Similar to the reason described above, I am not sure we have a > > chicken/egg > > > problem here. The issue here is that SinkV2 is not ready and we have a > > lot > > > of existing SinkFunction that is not migrated by ourselves. We (Flink > > > contributors) probably do not need to mark SinkFunction as deprecated > in > > > order to address these issues in our own codebase. > > > > > > > > > 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. > > > > > > > > > > Yeah I also agree with you. > > > > > > > > > > > > > > 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 > > > > > > > > > > -- https://twitter.com/snntrable https://github.com/knaufk
