flink-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Fabian Hueske <fhue...@gmail.com>
Subject Re: [ANNOUNCE] Please annotate public interfaces!
Date Mon, 08 Feb 2016 12:44:23 GMT
I created FLINK-3366 for renaming @Experimental to @PublicEvolving and
FLINK-3367 to annotate all remaining API classes with @PublicEvolving.

2016-02-08 9:57 GMT+01:00 Maximilian Michels <mxm@apache.org>:

> 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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message