Hi! These suggestions sound good in general.
I am wondering if "Experimental" is not the wrong word here, because most of the things are not experimental, but just possibly subject to slight changes (though API breaking). Experimental has the connotation that something is unstable (execution wise) and should best be avoided altogether. If half of the Flink code base is marked "Experimental", it seems to contradict that this is actually in 1.0 shape. I actually like the term "Internal" that was also suggested. It pretty much says what we actually want to say: A certain method or class is not declared part of the public API, but as of now only internal API. Greetings, Stephan On Fri, Feb 5, 2016 at 8:38 PM, Fabian Hueske <fhue...@gmail.com> wrote: > Hi, > > @Experimental and @Internal have the class scope, because we need to be > able to mark internal classes of @Public classes as experimental or > internal. > > In some cases we annotated classes as @Public and all (or most) methods as > @Experimental, to indicate that a class can be used, but its internals > might change. > For example TypeInformation is a public class (and many classes that extend > this class) but most methods are @Experimental to allow users to used a > TypeInformation as is, but keep the freedom to change the internals. If you > find something that does not look correct, please start a discussion about. > The annotations can still be changed before the 1.0 release. > > I agree with Max that annotating more classes with @Experimental (not only > inner classes) would be a good idea. > IMO, we should annotate all classes that belong to the user-facing API with > either @Public or @Experimental. This would mean that all non-annotated > classes are not part of the API and must be considered as internal. I > believe this would avoid a lot of uncertainty and also easy the annotation > management as a whole. For instance, we can more easily check how the API > (stable and experimental) changed between releases. > > Cheers, Fabian > > 2016-02-05 17:24 GMT+01:00 Maximilian Michels <m...@apache.org>: > > > Hi Robert, > > > > Thanks a lot for all the work of going through the classes. At first > > sight, the classes look quite well chosen. > > > > One question concerning the @Public, @Experimental, and @Internal > > annotations: > > > > @Public may only be used for classes or interfaces. @Experimental or > > @Internal are used for marking methods in @Public classes only? If > > that is the case, we should restrict the annotations to this scope via > > @Target. The current master also permits to tag classes with > > @Experimental or @Internal. > > > > Marking classes with @Experimental might actually make sense. What was > > the rational behind always declaring a class public first to restrict > > its methods to internal or experimental? > > > > Cheers, > > Max > > > > On Fri, Feb 5, 2016 at 3:07 PM, Robert Metzger <rmetz...@apache.org> > > wrote: > > > Hi, > > > > > > tl;dr: we now have @Public, @Internal, @Experimental annotations. Check > > > your stuff before the release! > > > > > > Fabian and I spend some time the last weeks to annotate all classes we > > > consider to be userfacing and stable with the "*@Public*" annotation. I > > > just pushed those changes to master. > > > > > > There is also an annotation "*@Internal*" for marking interfaces users > > > should not use because they are internal to the system (for example > > > "DataStream.getId()"). > > > > > > The annotation "*@Experimental*" marks unstable methods within stable > > > classes (for example SingleOutputStreamOperator.uid()). > > > Also note that we should also annotate @Deprecated methods with > > > @Experimental so that they are not part of the stable interface (this > > works > > > only before the 1.0 release). I checked that all current @Deprecated > > > annotations are excluded. > > > > > > > > > *I would like to ask everyone to spend some time before the 1.0 release > > to > > > go through some core classes and see if there is anything we forgot.* > > > *We will not be able to touch methods and classes which are public > after > > > the 1.0 release.* > > > > > > Some areas where I feel we should check closely are the following: > > > - InputFormats > > > - State-related classes > > > - Windowing related classes > > > - DataStream API (global() / forward(); output to files; ... ). > > > > > > Fabian and I also realized that there are some downsides to this > > annotation > > > approach. > > > a) By not annotating all classes, its easy to forget some classes. And > > its > > > not obvious to users that "no annotation" means "not public". > > > > > > b) For example the "SourceFunction.SourceContext" interface is @Public > > > because users use the methods of the interface. > > > However, the underlying implementations are internal to Flink (users > will > > > most likely not implement their own SourceContexts). Adding a new > method > > to > > > the SourceContext interface will break its compatibilty (because users > > > would need to implement the new method), however, for API users it does > > not > > > matter when we are adding new methods. > > > We decided to make the interface @Public, but we added a comment > > explaining > > > the issue. > > > If we want to add a method after the 1.0 release, we can define an > > exclude > > > in the maven plugin which enforces the interface stability. > > > > > > > > > For making the whole annotation analysis business a bit easier, I'm > > > currently working on a little tool to output a list of public classes > and > > > methods. I'll post that on the mailing list at a later point. > > > > > > Regards, > > > Robert > > >
