Hi Dawid, Thanks for the reply. I am currently looking at the Beam Flink runner, and there are quite some hacks the Beam runner has to do in order to deal with the backwards incompatible changes in AbstractStreamOperator and some of the classes exposed by it. Regardless of what we think, the fact is that AbstractStreamOperator is marked as PublicEvolving today, and our users use it. It is a basic rule of public API that anything exposed by a public interface should also be public. This is the direct motivation of this FLIP.
Regarding the StreamTask / StreamConfig exposure, if you look at the StreamOperatorFactory which is also a PublicEvolving class, it actually exposes the StreamTask, StreamConfig as well as some other classes in the StreamOperatorParameters. So these classes are already exposed in multiple public APIs. Keeping our public API stability guarantee is really fundamental and critical to the users. With the current status of inconsistent API stability annotations, I don't see how can we assure of that. From what I can see, accidental backwards incompatible changes is likely going to keep happening. So I'd say let's see how to fix forward instead of doing nothing. BTW, Initially I thought this is just an accidental mismatch, but after further exam, it looks that it is a bigger issue. I guess one of the reasons we end up in this situation is that we haven't really thought it through regarding the boundary between framework and user space, i.e. what framework primitives we want to expose to the users. So instead we just expose a bunch of internal things and hope users only use the "stable" part of them. Thanks, Jiangjie (Becket) Qin On Fri, Jan 13, 2023 at 7:28 PM Dawid Wysakowicz <dwysakow...@apache.org> wrote: > Hi Becket, > > May I ask what is the motivation for the change? > > I'm really skeptical about making any of those classes `Public` or > `PublicEvolving`. As far as I am concerned there is no agreement in the > community if StreamOperator is part of the `Public(Evolving)` API. At > least I think it should not. I understand `AbstractStreamOperator` was > marked with `PublicEvolving`, but I am really not convinced it was the > right decision. > > The listed classes are not the only classes exposed to > `AbstractStreamOperator` that are `Internal` that break the consistency > and I am sure there is no question those should remain `Internal` such > as e.g. StreamTask, StreamConfig, StreamingRuntimeContext and many more. > > As it stands I am strongly against giving any additional guarantees on > API stability to the classes there unless there is a good motivation for > a given feature and we're sure this is the best way to go forward. > > Thus I'm inclined to go with -1 on any proposal on changing annotations > for the sake of matching the one on `AbstractStreamOperator`. I might be > convinced to support requests to give better guarantees for well > motivated features that are now internal though. > > Best, > > Dawid > > On 12/01/2023 10:20, Becket Qin wrote: > > Hi flink devs, > > > > I'd like to start a discussion thread for FLIP-286[1]. > > > > As a recap, currently while AbstractStreamOperator is a class marked as > > @PublicEvolving, some classes exposed via its methods / fields are > > marked as @Internal. This FLIP wants to fix this inconsistency of > > stability / scope annotation. > > > > Comments are welcome! > > > > Thanks, > > > > Jiangjie (Becket) Qin > > > > [1] > > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=240880841 > > >
