flink-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Maximilian Michels <...@apache.org>
Subject Re: [ANNOUNCE] Please annotate public interfaces!
Date Mon, 08 Feb 2016 08:57:56 GMT
Never thought about "Experimental" meaning unstable but I agree it
sounds better to use the term "Evolving".

On Sun, Feb 7, 2016 at 12:45 AM, Fabian Hueske <fhueske@gmail.com> wrote:
> I agree, Experimental rather suggests unstable behavior than potentially
> changing interfaces.
> +1 for renaming to @PublicEvolving.
>
> Other opinions on strictly annotating all public interfaces with @Public /
> @PublicEvolving and
> defaulting to @Internal for all remaining interfaces?
>
> 2016-02-06 15:27 GMT+01:00 Stephan Ewen <sewen@apache.org>:
>
>> What "Experimental" is really saying is "Public, but not API stable". In
>> that sense, "Internal" is not suitable, as it suggests that a method will
>> never be public.
>>
>> What would you think of renaming "Experimental" to "PublicEvolving" ?
>>
>> That would carry pretty much explicitly the meaning that it is intended for
>> public use, the basic concept will stay, but it may change a bit (code
>> using that may be adjusted a bit in the future). In contrast "Experimental"
>> sounds to me like "no idea if that class/feature is going to be there in
>> the future".
>>
>> What do you think?
>>
>> Stephan
>>
>>
>> On Sat, Feb 6, 2016 at 3:17 PM, Stephan Ewen <sewen@apache.org> wrote:
>>
>> > 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 <fhueske@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 <mxm@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 <rmetzger@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
>> >> >
>> >>
>> >
>> >
>>

Mime
View raw message