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 |
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 <[hidden email]> 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 |
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 <[hidden email]>: > 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 <[hidden email]> > 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 > |
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 <[hidden email]> 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 <[hidden email]>: > > > 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 <[hidden email]> > > 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 > > > |
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>: >> >> > 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 <[hidden email]> >> > 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 >> > >> > > |
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 <[hidden email]>: > 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>: > >> > >> > 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 <[hidden email]> > >> > 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 > >> > > >> > > > > > |
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 <[hidden email]> 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 <[hidden email]>: > >> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]>: >> >> >> >> > 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 <[hidden email]> >> >> > 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 >> >> > >> >> >> > >> > >> |
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 <[hidden email]>: > 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 <[hidden email]> 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 <[hidden email]>: > > > >> 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 <[hidden email]> 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 <[hidden email]> > 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 <[hidden email]>: > >> >> > >> >> > 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 < > [hidden email]> > >> >> > 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 > >> >> > > >> >> > >> > > >> > > >> > |
Thank you for taking care of this Fabian.
I would like to bring your attention to the "ConfigConstants" class. I marked it as "@Public". My intention is to ensure that we don't change configuration parameters after the 1.0 release (this would break existing configuration files of users). The tool I'm intending to use for ensuring interface stability does not ensure that the values of the constants are not changed. It only ensures that the variable names are not changed. What we have right now blocks us from renaming the constants. My reasoning was that if somebody is touching the constant's name, most likely they'll also change the value. But that's just a poor best-effort approach. What's your opinion? Should we remove the @Public annotation there? (With the risk that configuration parameters might change between versions) or leave it? On Mon, Feb 8, 2016 at 1:44 PM, Fabian Hueske <[hidden email]> wrote: > 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 <[hidden email]>: > > > 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 <[hidden email]> > 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 <[hidden email]>: > > > > > >> 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 <[hidden email]> > 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 <[hidden email]> > > 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 <[hidden email]>: > > >> >> > > >> >> > 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 < > > [hidden email]> > > >> >> > 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 > > >> >> > > > >> >> > > >> > > > >> > > > >> > > > |
I think the important part about the ConfigConstants is that the values
don’t change. How they are represented inside of Flink, does not really matter. It would be good if that could be verified automatically. Cheers, Till On Tue, Feb 16, 2016 at 2:59 PM, Robert Metzger <[hidden email]> wrote: > Thank you for taking care of this Fabian. > > I would like to bring your attention to the "ConfigConstants" class. I > marked it as "@Public". My intention is to ensure that we don't change > configuration parameters after the 1.0 release (this would break existing > configuration files of users). > > The tool I'm intending to use for ensuring interface stability does not > ensure that the values of the constants are not changed. It only ensures > that the variable names are not changed. What we have right now blocks us > from renaming the constants. > My reasoning was that if somebody is touching the constant's name, most > likely they'll also change the value. But that's just a poor best-effort > approach. > > What's your opinion? Should we remove the @Public annotation there? (With > the risk that configuration parameters might change between versions) or > leave it? > > > > On Mon, Feb 8, 2016 at 1:44 PM, Fabian Hueske <[hidden email]> wrote: > > > 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 <[hidden email]>: > > > > > 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 <[hidden email]> > > 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 <[hidden email]>: > > > > > > > >> 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 <[hidden email]> > > 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 <[hidden email]> > > > 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 <[hidden email]>: > > > >> >> > > > >> >> > 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 < > > > [hidden email]> > > > >> >> > 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 > > > >> >> > > > > >> >> > > > >> > > > > >> > > > > >> > > > > > > |
Free forum by Nabble | Edit this page |