Hi Shammon, Thanks for the reply.
Do we really need to have `@Internal` methods in an `@Public` interface or > class? In general, if a class or interface is marked as `@Public `, it is > better that their public methods should also be `@Public`, because even if > marked as `@Internal`, users are not aware of it when using it, which could > be strange. It is more like a legacy issue that the public and internal usage share the same concrete class. e.g. DataStream.getId() is for internal usage, but happens to be in DataStream which is a public class. This should be avoided in the future. It is a good practice to create separate interfaces should be created for the users in this case. Regarding the API stability promotion, you may want to check the FLIP-197[1]. Thanks, Jiangjie (Becket) Qin [1] https://cwiki.apache.org/confluence/display/FLINK/FLIP-197%3A+API+stability+graduation+process On Mon, Sep 11, 2023 at 11:43 AM Shammon FY <zjur...@gmail.com> wrote: > Thanks Jing for starting this discussion. > > For @Becket > > 1. All the public methods / classes / interfaces MUST be annotated with > one of the @Experimental / @PublicEvolving / @Public. In practice, all the > methods by default inherit the annotation from the containing class, unless > annotated otherwise. e.g. an @Internal method in a @Public class. > > Do we really need to have `@Internal` methods in an `@Public` interface or > class? In general, if a class or interface is marked as `@Public `, it is > better that their public methods should also be `@Public`, because even if > marked as `@Internal`, users are not aware of it when using it, which could > be strange. > > @Jing Besides `@Internal`, I have some cents about `@PublicEvolving` and > `@Public`. Currently when we add an interface which will be used by > external systems, we often annotate it as `@PublicEvolving`. But when > should this interface be marked as `@Public`? I find it is difficult to > determine this. Is `@PublicEvolving` really necessary? Should we directly > remove `@PublicEvolving` and use `@Public` instead? I think it would be > simpler. > > Best, > Shammon FY > > > On Mon, Sep 11, 2023 at 11:05 AM Becket Qin <becket....@gmail.com> wrote: > > > Hi Jing, > > > > Thanks for bringing up the discussion. My two cents: > > > > 1. All the public methods / classes / interfaces MUST be annotated with > one > > of the @Experimental / @PublicEvolving / @Public. In practice, all the > > methods by default inherit the annotation from the containing class, > unless > > annotated otherwise. e.g. an @Internal method in a @Public class. > > > > 2. I agree it would be too verbose to annotate every internal method / > > class / interface. Currently we already treat the methods / interfaces / > > classes without annotations as effectively @Internal. > > > > 3. Per our discussion in the other thread, @Deprecated SHOULD coexist > with > > one of the @Experimental / @PublicEvolving / @Public. In that > > case, @Deprecated overrides the other annotation, which means that public > > API will not evolve and will be removed according to the deprecation > > process. > > > > 4. The internal methods / classes / interfaces SHOULD NOT be marked as > > deprecated. Instead, an immediate refactor should be done to remove the > > "deprecated" internal methods / classes / interfaces, and migrate the > code > > to its successor. Otherwise, technical debts will build up. > > > > Thanks, > > > > Jiangjie (Becket) Qin > > > > > > > > On Sat, Sep 9, 2023 at 5:29 AM Jing Ge <j...@ververica.com.invalid> > wrote: > > > > > Hi devs, > > > > > > While I was joining the flink-avro enhancement and cleanup discussion > > > driven by Becket[1], I realized that there are some issues with the > > current > > > Flink API annotation usage in the source code. > > > > > > As far as I am concerned, Flink wants to control the access/visibility > of > > > APIs across modules and for downstreams. Since no OSGI is used(it > should > > > not be used because of its complexity, IMHO), Flink decided to use a > very > > > lightweight but manual solution: customized annotation like @Internal, > > > @Experimental, @PublicEvolving, > > > etc. This is a Flink only concept on top of JDK annotation and is > > therefore > > > orthogonal to @Deprecated or any other annotations offered by JDK. > After > > > this concept has been used, APIs without one of these annotations are > in > > > the kind of gray area which means they have no contract in the context > of > > > this new concept. Without any given metadata they could be considered > > > as @Internal or @Experimental, because changes are allowed to be > applied > > at > > > any time. But there is no clear definition and therefore different > people > > > will understand it differently. > > > > > > There are two options to improve it, as far as I could figure out: > > > > > > option 1: All APIs must have one of those annotations. We should put > some > > > effort into going through all source code and add missing annotations. > > > There were discussions[2] and activities going in this direction. > > > option 2: the community comes to a new consensus that APIs without > > > annotation equals one of @Internal, @Experimental, or @PublicEvolving. > I > > > personally will choose @Internal, because it is the safest one. And if > > > @Internal is chosen as the default one, it could also be deprecated, > > > because no annotation equals @Internal. If it makes sense, I can > create a > > > FLIP and help the community reach this consensus. > > > > > > Both options have their own pros and cons. I would choose option 2, > since > > > we will not end up with a lot of APIs marked as @Internal. > > > > > > Looking forward to hearing your thoughts. > > > > > > Best regards > > > Jing > > > > > > > > > [1] https://lists.apache.org/thread/7zsv528swbjxo5zk0bxq33hrkvd77d6f > > > [2] https://lists.apache.org/thread/zl2rmodsjsdb49tt4hn6wv3gdwo0m31o > > > > > >
