[DISCUSS] FLIP-196: Source API stability guarantees

[Till Rohrmann <tr...@apache.org>]

Hi everyone,

I would like to start a discussion about what kind of API stability
guarantees we want to provide to our users. The Flink community already
agreed on some stability guarantees, but these guarantees were only
communicated via the ML and not properly documented [2]. Moreover, I've
seen more and more complaints about breaking changes (some rightful and
others not) on the ML recently [3] because we rarely mark our APIs as
stable. This motivated this FLIP.

The proposal first concentrates on source API stability guarantees. Binary
stability guarantees are explicitly left out for a follow-up discussion.

In essence, the proposal follows our current stability guarantees while
updating the guarantees for @PublicEvolving that we are already providing
w/o having stated them. Moreover, this FLIP proposes some guidelines for
determining the stability guarantees for an API object, how to evolve them
and some additional requirements for the return and parameter types of
methods.

All in all, I hope that this proposal is more reflecting our current
understanding of stability guarantees than being controversial. One of the
outcomes of this FLIP should be to properly document these guarantees so
that it is easily discoverable and understandable for our users. Moreover,
I hope that we can provide more stable APIs our users can rely and build
upon.

There will be a follow-up FLIP discussing the problem of how to make sure
that APIs become stable over time.

Looking forward to your feedback.

[1] https://cwiki.apache.org/confluence/x/IJeqCw
[2] https://lists.apache.org/thread/5jm25783oq5svyk7rr8g1gly2ooxqhjr
[3] https://lists.apache.org/thread/kzhfc3t6omzo2kyo8zj9yxoh8twq5fzr

Cheers,
Till

Re: [DISCUSS] FLIP-196: Source API stability guarantees

[Till Rohrmann <tr...@apache.org>]

Fyi: Ingo already implemented most of the FLIP by introducing the
ApiAnnotationRules.PUBLIC_API_METHODS_USE_ONLY_PUBLIC_API_TYPES and
.PUBLIC_EVOLVING_API_METHODS_USE_ONLY_PUBLIC_EVOLVING_API_TYPES [1] using
ArchUnit :-)

[1]
https://github.com/apache/flink/blob/master/flink-architecture-tests/src/test/java/org/apache/flink/architecture/rules/ApiAnnotationRules.java

Cheers,
Till

On Fri, Dec 3, 2021 at 11:59 AM Till Rohrmann <tr...@apache.org> wrote:

> Hi Konstantin and Francesco,
>
> > * Are you planning to implement the required tests via ArchUnit?
>
> If ArchUnit is the best tool for the job, then I will probably use it. But
> I haven't investigated it yet.
>
> > * Fixing existing "test failures" is in the scope of this FLIP, correct?
>
> Yes I think that fixing the inconsistencies discovered by the to be added
> tests is part of the FLIP.
>
> > I think it would be nice that these docs are actually Javadocs, since
> our PublicEvolving javadocs tend to be already complete. perhaps there is a
> way we can play around the javadoc tool to generate only pages with @Public
> and @PublicEvolving on it.
>
> My plan was to state what the
> different @Public, @PublicEvolving, @Experimental annotations mean in the
> documentation. One can also find the definitions in the code, but I think
> some users don't look too closely at the code and JavaDocs.
>
> > TCKs
>
> I agree that TCKs/test suites, which ensure that a certain behaviour does
> not change, are important. However, I don't think that we have to define
> all TCKs in the context of this FLIP since it would let the scope explode
> and grind this effort to a halt. Instead, I would suggest that component
> shepherds start follow up discussions on how they can ensure that Flink's
> behaviour does not change for stable APIs and then add the corresponding
> TCKs/test suites.
>
> Cheers,
> Till
>
> On Fri, Dec 3, 2021 at 10:14 AM Francesco Guardiani <
> francesco@ververica.com> wrote:
>
>> Hi Till,
>>
>> Thanks for starting this discussion, I think it's very beneficial for the
>> community to have stable APIs, in particular to develop connectors and
>> formats.
>>
>> A couple of comments:
>>
>> > I would suggest that we document these guarantees prominently under
>> /docs/dev/api_stability.
>>
>> I think it would be nice that these docs are actually Javadocs, since our
>> PublicEvolving javadocs tend to be already complete. perhaps there is a
>> way
>> we can play around the javadoc tool to generate only pages with @Public
>> and
>> @PublicEvolving on it.
>>
>> > Test Plan
>>
>> IMHO this proposal lacks what testing means from the user perspective: I
>> think a discussion about API stability should go hand to hand with a
>> discussion about providing TCKs to users. If the interfaces to build a
>> Source are claimed to be stable, then, from the flink runtime point of
>> view, a 3rd party source implementation is expected to have certain
>> behaviors. On the other hand, as a 3rd party developer, I expect flink
>> gives me some means to test if my source is compliant with the basic
>> "expected behaviors". This is even more relevant when you're building a
>> source for Table, where we have well defined behaviors through our
>> "abilities" interfaces.
>>
>> TCKs also have another very important aspect: they serve as a very
>> detailed
>> documentation of all the behaviors that comes in/out a specific interface.
>>
>> Another thing to mention is that we already have some of these test, they
>> just need to be polished to make them publicly available; e.g. look at the
>> JsonBatchFileSystemITCase, it uses all the test cases from
>> FileSystemITCaseBase to test that the format works correctly with the
>> filesystem source.
>>
>> A TCK is something that can take quite some time to be considered
>> "complete", so I'm not suggesting at all to get it done all at once inside
>> this FLIP. But I think that if we bootstrap it in this FLIP, we can then
>> progressively add new test cases every time we should add them internally,
>> or when we need to rework the existing ones.
>>
>>
>> FG
>>
>> On Fri, Dec 3, 2021 at 9:32 AM Konstantin Knauf <kn...@apache.org>
>> wrote:
>>
>> > Hi Till,
>> >
>> > Thank you for picking up this topic. Explicit and consistent stability
>> > guarantees are important for our users as well as any downstream
>> project of
>> > Apache Flink. Documenting the de-facto status quo and tackling existing
>> > inconsistencies sounds like a good first step. So, +1 from my side.
>> >
>> > Two questions for clarity:
>> > * Are you planning to implement the required tests via ArchUnit?
>> > * Fixing existing "test failures" is in the scope of this FLIP, correct?
>> >
>> > Cheers,
>> >
>> > Konstantin
>> >
>> > On Thu, Dec 2, 2021 at 3:48 PM Till Rohrmann <tr...@apache.org>
>> wrote:
>> >
>> > > Hi everyone,
>> > >
>> > > I would like to start a discussion about what kind of API stability
>> > > guarantees we want to provide to our users. The Flink community
>> already
>> > > agreed on some stability guarantees, but these guarantees were only
>> > > communicated via the ML and not properly documented [2]. Moreover,
>> I've
>> > > seen more and more complaints about breaking changes (some rightful
>> and
>> > > others not) on the ML recently [3] because we rarely mark our APIs as
>> > > stable. This motivated this FLIP.
>> > >
>> > > The proposal first concentrates on source API stability guarantees.
>> > Binary
>> > > stability guarantees are explicitly left out for a follow-up
>> discussion.
>> > >
>> > > In essence, the proposal follows our current stability guarantees
>> while
>> > > updating the guarantees for @PublicEvolving that we are already
>> providing
>> > > w/o having stated them. Moreover, this FLIP proposes some guidelines
>> for
>> > > determining the stability guarantees for an API object, how to evolve
>> > them
>> > > and some additional requirements for the return and parameter types of
>> > > methods.
>> > >
>> > > All in all, I hope that this proposal is more reflecting our current
>> > > understanding of stability guarantees than being controversial. One of
>> > the
>> > > outcomes of this FLIP should be to properly document these guarantees
>> so
>> > > that it is easily discoverable and understandable for our users.
>> > Moreover,
>> > > I hope that we can provide more stable APIs our users can rely and
>> build
>> > > upon.
>> > >
>> > > There will be a follow-up FLIP discussing the problem of how to make
>> sure
>> > > that APIs become stable over time.
>> > >
>> > > Looking forward to your feedback.
>> > >
>> > > [1] https://cwiki.apache.org/confluence/x/IJeqCw
>> > > [2] https://lists.apache.org/thread/5jm25783oq5svyk7rr8g1gly2ooxqhjr
>> > > [3] https://lists.apache.org/thread/kzhfc3t6omzo2kyo8zj9yxoh8twq5fzr
>> > >
>> > > Cheers,
>> > > Till
>> > >
>> >
>> >
>> > --
>> >
>> > Konstantin Knauf
>> >
>> > https://twitter.com/snntrable
>> >
>> > https://github.com/knaufk
>> >
>>
>

Re: [DISCUSS] FLIP-196: Source API stability guarantees

[Till Rohrmann <tr...@apache.org>]

Hi Konstantin and Francesco,

> * Are you planning to implement the required tests via ArchUnit?

If ArchUnit is the best tool for the job, then I will probably use it. But
I haven't investigated it yet.

> * Fixing existing "test failures" is in the scope of this FLIP, correct?

Yes I think that fixing the inconsistencies discovered by the to be added
tests is part of the FLIP.

> I think it would be nice that these docs are actually Javadocs, since our
PublicEvolving javadocs tend to be already complete. perhaps there is a way
we can play around the javadoc tool to generate only pages with @Public and
@PublicEvolving on it.

My plan was to state what the
different @Public, @PublicEvolving, @Experimental annotations mean in the
documentation. One can also find the definitions in the code, but I think
some users don't look too closely at the code and JavaDocs.

> TCKs

I agree that TCKs/test suites, which ensure that a certain behaviour does
not change, are important. However, I don't think that we have to define
all TCKs in the context of this FLIP since it would let the scope explode
and grind this effort to a halt. Instead, I would suggest that component
shepherds start follow up discussions on how they can ensure that Flink's
behaviour does not change for stable APIs and then add the corresponding
TCKs/test suites.

Cheers,
Till

On Fri, Dec 3, 2021 at 10:14 AM Francesco Guardiani <fr...@ververica.com>
wrote:

> Hi Till,
>
> Thanks for starting this discussion, I think it's very beneficial for the
> community to have stable APIs, in particular to develop connectors and
> formats.
>
> A couple of comments:
>
> > I would suggest that we document these guarantees prominently under
> /docs/dev/api_stability.
>
> I think it would be nice that these docs are actually Javadocs, since our
> PublicEvolving javadocs tend to be already complete. perhaps there is a way
> we can play around the javadoc tool to generate only pages with @Public and
> @PublicEvolving on it.
>
> > Test Plan
>
> IMHO this proposal lacks what testing means from the user perspective: I
> think a discussion about API stability should go hand to hand with a
> discussion about providing TCKs to users. If the interfaces to build a
> Source are claimed to be stable, then, from the flink runtime point of
> view, a 3rd party source implementation is expected to have certain
> behaviors. On the other hand, as a 3rd party developer, I expect flink
> gives me some means to test if my source is compliant with the basic
> "expected behaviors". This is even more relevant when you're building a
> source for Table, where we have well defined behaviors through our
> "abilities" interfaces.
>
> TCKs also have another very important aspect: they serve as a very detailed
> documentation of all the behaviors that comes in/out a specific interface.
>
> Another thing to mention is that we already have some of these test, they
> just need to be polished to make them publicly available; e.g. look at the
> JsonBatchFileSystemITCase, it uses all the test cases from
> FileSystemITCaseBase to test that the format works correctly with the
> filesystem source.
>
> A TCK is something that can take quite some time to be considered
> "complete", so I'm not suggesting at all to get it done all at once inside
> this FLIP. But I think that if we bootstrap it in this FLIP, we can then
> progressively add new test cases every time we should add them internally,
> or when we need to rework the existing ones.
>
>
> FG
>
> On Fri, Dec 3, 2021 at 9:32 AM Konstantin Knauf <kn...@apache.org> wrote:
>
> > Hi Till,
> >
> > Thank you for picking up this topic. Explicit and consistent stability
> > guarantees are important for our users as well as any downstream project
> of
> > Apache Flink. Documenting the de-facto status quo and tackling existing
> > inconsistencies sounds like a good first step. So, +1 from my side.
> >
> > Two questions for clarity:
> > * Are you planning to implement the required tests via ArchUnit?
> > * Fixing existing "test failures" is in the scope of this FLIP, correct?
> >
> > Cheers,
> >
> > Konstantin
> >
> > On Thu, Dec 2, 2021 at 3:48 PM Till Rohrmann <tr...@apache.org>
> wrote:
> >
> > > Hi everyone,
> > >
> > > I would like to start a discussion about what kind of API stability
> > > guarantees we want to provide to our users. The Flink community already
> > > agreed on some stability guarantees, but these guarantees were only
> > > communicated via the ML and not properly documented [2]. Moreover, I've
> > > seen more and more complaints about breaking changes (some rightful and
> > > others not) on the ML recently [3] because we rarely mark our APIs as
> > > stable. This motivated this FLIP.
> > >
> > > The proposal first concentrates on source API stability guarantees.
> > Binary
> > > stability guarantees are explicitly left out for a follow-up
> discussion.
> > >
> > > In essence, the proposal follows our current stability guarantees while
> > > updating the guarantees for @PublicEvolving that we are already
> providing
> > > w/o having stated them. Moreover, this FLIP proposes some guidelines
> for
> > > determining the stability guarantees for an API object, how to evolve
> > them
> > > and some additional requirements for the return and parameter types of
> > > methods.
> > >
> > > All in all, I hope that this proposal is more reflecting our current
> > > understanding of stability guarantees than being controversial. One of
> > the
> > > outcomes of this FLIP should be to properly document these guarantees
> so
> > > that it is easily discoverable and understandable for our users.
> > Moreover,
> > > I hope that we can provide more stable APIs our users can rely and
> build
> > > upon.
> > >
> > > There will be a follow-up FLIP discussing the problem of how to make
> sure
> > > that APIs become stable over time.
> > >
> > > Looking forward to your feedback.
> > >
> > > [1] https://cwiki.apache.org/confluence/x/IJeqCw
> > > [2] https://lists.apache.org/thread/5jm25783oq5svyk7rr8g1gly2ooxqhjr
> > > [3] https://lists.apache.org/thread/kzhfc3t6omzo2kyo8zj9yxoh8twq5fzr
> > >
> > > Cheers,
> > > Till
> > >
> >
> >
> > --
> >
> > Konstantin Knauf
> >
> > https://twitter.com/snntrable
> >
> > https://github.com/knaufk
> >
>

Re: [DISCUSS] FLIP-196: Source API stability guarantees

[Francesco Guardiani <fr...@ververica.com>]

Hi Till,

Thanks for starting this discussion, I think it's very beneficial for the
community to have stable APIs, in particular to develop connectors and
formats.

A couple of comments:

> I would suggest that we document these guarantees prominently under
/docs/dev/api_stability.

I think it would be nice that these docs are actually Javadocs, since our
PublicEvolving javadocs tend to be already complete. perhaps there is a way
we can play around the javadoc tool to generate only pages with @Public and
@PublicEvolving on it.

> Test Plan

IMHO this proposal lacks what testing means from the user perspective: I
think a discussion about API stability should go hand to hand with a
discussion about providing TCKs to users. If the interfaces to build a
Source are claimed to be stable, then, from the flink runtime point of
view, a 3rd party source implementation is expected to have certain
behaviors. On the other hand, as a 3rd party developer, I expect flink
gives me some means to test if my source is compliant with the basic
"expected behaviors". This is even more relevant when you're building a
source for Table, where we have well defined behaviors through our
"abilities" interfaces.

TCKs also have another very important aspect: they serve as a very detailed
documentation of all the behaviors that comes in/out a specific interface.

Another thing to mention is that we already have some of these test, they
just need to be polished to make them publicly available; e.g. look at the
JsonBatchFileSystemITCase, it uses all the test cases from
FileSystemITCaseBase to test that the format works correctly with the
filesystem source.

A TCK is something that can take quite some time to be considered
"complete", so I'm not suggesting at all to get it done all at once inside
this FLIP. But I think that if we bootstrap it in this FLIP, we can then
progressively add new test cases every time we should add them internally,
or when we need to rework the existing ones.


FG

On Fri, Dec 3, 2021 at 9:32 AM Konstantin Knauf <kn...@apache.org> wrote:

> Hi Till,
>
> Thank you for picking up this topic. Explicit and consistent stability
> guarantees are important for our users as well as any downstream project of
> Apache Flink. Documenting the de-facto status quo and tackling existing
> inconsistencies sounds like a good first step. So, +1 from my side.
>
> Two questions for clarity:
> * Are you planning to implement the required tests via ArchUnit?
> * Fixing existing "test failures" is in the scope of this FLIP, correct?
>
> Cheers,
>
> Konstantin
>
> On Thu, Dec 2, 2021 at 3:48 PM Till Rohrmann <tr...@apache.org> wrote:
>
> > Hi everyone,
> >
> > I would like to start a discussion about what kind of API stability
> > guarantees we want to provide to our users. The Flink community already
> > agreed on some stability guarantees, but these guarantees were only
> > communicated via the ML and not properly documented [2]. Moreover, I've
> > seen more and more complaints about breaking changes (some rightful and
> > others not) on the ML recently [3] because we rarely mark our APIs as
> > stable. This motivated this FLIP.
> >
> > The proposal first concentrates on source API stability guarantees.
> Binary
> > stability guarantees are explicitly left out for a follow-up discussion.
> >
> > In essence, the proposal follows our current stability guarantees while
> > updating the guarantees for @PublicEvolving that we are already providing
> > w/o having stated them. Moreover, this FLIP proposes some guidelines for
> > determining the stability guarantees for an API object, how to evolve
> them
> > and some additional requirements for the return and parameter types of
> > methods.
> >
> > All in all, I hope that this proposal is more reflecting our current
> > understanding of stability guarantees than being controversial. One of
> the
> > outcomes of this FLIP should be to properly document these guarantees so
> > that it is easily discoverable and understandable for our users.
> Moreover,
> > I hope that we can provide more stable APIs our users can rely and build
> > upon.
> >
> > There will be a follow-up FLIP discussing the problem of how to make sure
> > that APIs become stable over time.
> >
> > Looking forward to your feedback.
> >
> > [1] https://cwiki.apache.org/confluence/x/IJeqCw
> > [2] https://lists.apache.org/thread/5jm25783oq5svyk7rr8g1gly2ooxqhjr
> > [3] https://lists.apache.org/thread/kzhfc3t6omzo2kyo8zj9yxoh8twq5fzr
> >
> > Cheers,
> > Till
> >
>
>
> --
>
> Konstantin Knauf
>
> https://twitter.com/snntrable
>
> https://github.com/knaufk
>

Re: [DISCUSS] FLIP-196: Source API stability guarantees

[Konstantin Knauf <kn...@apache.org>]

Hi Till,

Thank you for picking up this topic. Explicit and consistent stability
guarantees are important for our users as well as any downstream project of
Apache Flink. Documenting the de-facto status quo and tackling existing
inconsistencies sounds like a good first step. So, +1 from my side.

Two questions for clarity:
* Are you planning to implement the required tests via ArchUnit?
* Fixing existing "test failures" is in the scope of this FLIP, correct?

Cheers,

Konstantin

On Thu, Dec 2, 2021 at 3:48 PM Till Rohrmann <tr...@apache.org> wrote:

> Hi everyone,
>
> I would like to start a discussion about what kind of API stability
> guarantees we want to provide to our users. The Flink community already
> agreed on some stability guarantees, but these guarantees were only
> communicated via the ML and not properly documented [2]. Moreover, I've
> seen more and more complaints about breaking changes (some rightful and
> others not) on the ML recently [3] because we rarely mark our APIs as
> stable. This motivated this FLIP.
>
> The proposal first concentrates on source API stability guarantees. Binary
> stability guarantees are explicitly left out for a follow-up discussion.
>
> In essence, the proposal follows our current stability guarantees while
> updating the guarantees for @PublicEvolving that we are already providing
> w/o having stated them. Moreover, this FLIP proposes some guidelines for
> determining the stability guarantees for an API object, how to evolve them
> and some additional requirements for the return and parameter types of
> methods.
>
> All in all, I hope that this proposal is more reflecting our current
> understanding of stability guarantees than being controversial. One of the
> outcomes of this FLIP should be to properly document these guarantees so
> that it is easily discoverable and understandable for our users. Moreover,
> I hope that we can provide more stable APIs our users can rely and build
> upon.
>
> There will be a follow-up FLIP discussing the problem of how to make sure
> that APIs become stable over time.
>
> Looking forward to your feedback.
>
> [1] https://cwiki.apache.org/confluence/x/IJeqCw
> [2] https://lists.apache.org/thread/5jm25783oq5svyk7rr8g1gly2ooxqhjr
> [3] https://lists.apache.org/thread/kzhfc3t6omzo2kyo8zj9yxoh8twq5fzr
>
> Cheers,
> Till
>


-- 

Konstantin Knauf

https://twitter.com/snntrable

https://github.com/knaufk

Re: [DISCUSS] FLIP-196: Source API stability guarantees

[Till Rohrmann <tr...@apache.org>]

Thanks a lot for all your input. I hope that I could resolve most of your
concerns. I will now start a vote on this FLIP.

Cheers,
Till

On Tue, Dec 7, 2021 at 4:28 PM Till Rohrmann <tr...@apache.org> wrote:

> Hi Fabian,
>
> Thanks for your input. Adding the definition of the different labels to
> our documentation is definitely part of this FLIP.
>
> I agree that deprecation is related to API stability. For the sake of
> keeping this FLIP narrowly scoped I left it out, though. For the time being
> I suggest that we stick to our current deprecation process that says that a
> deprecated API can only be removed after it has been deprecated for at
> least one release. Of course, deprecating and removing a @PublicEvolving
> API can only happen in minor or major releases, for example. This process
> is a bit stricter than what the API stability guarantees require because
> with the API stability alone we could remove the APIs directly w/o the
> deprecation period. Other details I would leave for a follow up FLIP
> which formally defines the deprecation process.
>
> Cheers,
> Till
>
> On Tue, Dec 7, 2021 at 10:08 AM Fabian Paul <fp...@apache.org> wrote:
>
>> Hi all,
>>
>> Thanks Till for starting this discussion. It is great to see these
>> facts written down since they definitely caused friction in the past
>> because of different interpretations. Overall I agree with everything
>> being said in this FLIP. I was just wondering whether we can put the
>> label explaining table somewhere into our docs. FLIPs are usually only
>> snapshots for the time being and are hard to search for. It would
>> allow users to instantly determine the meaning of the used annotation.
>>
>> I am also missing in this FLIP the connection to the Deprecated
>> annotation. I think we also need to clarify how this should be used in
>> conjunction with the API stability.
>>
>> Best,
>> Fabian
>>
>> On Mon, Dec 6, 2021 at 10:03 AM Ingo Bürk <in...@ververica.com> wrote:
>> >
>> > Hi Till,
>> >
>> > seems I misunderstood it then; thanks for the clarification! And yes,
>> with
>> > that I would fully agree.
>> >
>> >
>> > Ingo
>> >
>> > On Mon, Dec 6, 2021 at 9:59 AM Till Rohrmann <tr...@apache.org>
>> wrote:
>> >
>> > > Hi Ingo,
>> > >
>> > > No, the added method can have a weaker stability guarantee as long as
>> the
>> > > user does not have to implement it. In order to give an example the
>> > > following extension would be ok imo:
>> > >
>> > > @Public
>> > > interface Foobar {
>> > >     @Public
>> > >     int foo();
>> > >
>> > >     @Experimental
>> > >     default ExperimentalResult bar() {
>> > >       return ExperimentalResult.notSupported();
>> > >     }
>> > > }
>> > >
>> > > The following extension would not be ok because here the user needs to
>> > > implement something new:
>> > >
>> > > @Public
>> > > interface Foobar {
>> > >     @Public
>> > >     int foo();
>> > >
>> > >     @Experimental
>> > >     ExperimentalResult bar();
>> > > }
>> > >
>> > > Moreover, if the user uses bar(), then he opts-in to only get
>> @Experimental
>> > > stability guarantees.
>> > >
>> > > I will add this example to the FLIP for illustrative purposes.
>> > >
>> > > Cheers,
>> > > Till
>> > >
>> > > On Fri, Dec 3, 2021 at 6:52 PM Ingo Bürk <in...@ververica.com> wrote:
>> > >
>> > > > > Would it be enough to say that for example all classes in the
>> module
>> > > > flink-java have to be annotated? What I would like to avoid is
>> having to
>> > > > annotate all classes in some internal module like flink-rpc.
>> > > >
>> > > > I don't think it is, but we certainly could restrict it to certain
>> top
>> > > > level o.a.f.xyz packages.
>> > > >
>> > > > > Extending existing classes will only be possible if you can
>> provide a
>> > > > default implementation
>> > > >
>> > > > That I'm totally fine with, but based on that sentence in the FLIP
>> if I
>> > > > have a public interface and extend it, even with a default
>> > > implementation,
>> > > > I _have_ to have this method be stable already as well, right? I
>> couldn't
>> > > > for example add an experimental method to an interface.
>> > > >
>> > > > This would also include all classes used as argument and return
>> type of
>> > > > such methods too, which seems quite restrictive.
>> > > >
>> > > >
>> > > > Best
>> > > > Ingo
>> > > >
>> > > > On Fri, Dec 3, 2021, 17:51 Till Rohrmann <tr...@apache.org>
>> wrote:
>> > > >
>> > > > > >That's still a much weaker requirement, though, as classes can
>> just be
>> > > > > left unannotated, which is why I prefer annotating all classes
>> > > regardless
>> > > > > of location.
>> > > > >
>> > > > > Would it be enough to say that for example all classes in the
>> module
>> > > > > flink-java have to be annotated? What I would like to avoid is
>> having
>> > > to
>> > > > > annotate all classes in some internal module like flink-rpc.
>> > > > >
>> > > > > > How would you handle e.g. extending an existing Public interface
>> > > with a
>> > > > > new method in this case, though? You'd be forced to immediately
>> make
>> > > the
>> > > > > new method Public as well, or place it somewhere else entirely,
>> which
>> > > > leads
>> > > > > to unfavorable design. I don't think we should disallow extending
>> > > classes
>> > > > > with methods of a weaker stability.
>> > > > >
>> > > > > Extending existing classes will only be possible if you can
>> provide a
>> > > > > default implementation. If the user needs to do something, then
>> it is
>> > > not
>> > > > > compatible and needs to be handled differently (e.g. by offering
>> a new
>> > > > > experimental interface that one can use). If we don't enforce
>> this,
>> > > then
>> > > > I
>> > > > > don't see how we can provide source stability guarantees.
>> > > > >
>> > > > > Cheers,
>> > > > > Till
>> > > > >
>> > > > > On Fri, Dec 3, 2021 at 5:22 PM Ingo Bürk <in...@ververica.com>
>> wrote:
>> > > > >
>> > > > > > Hi Till,
>> > > > > >
>> > > > > > > Personally, I'd be fine to say that in API modules (tbd what
>> this
>> > > is
>> > > > > > > (probably transitive closure of all APIs)) we require every
>> class
>> > > to
>> > > > be
>> > > > > > > annotated.
>> > > > > >
>> > > > > > At least we'll then need the reverse rule: no classes outside
>> *.api.*
>> > > > > > packages CAN have an API annotation (other than Internal), of
>> course
>> > > > with
>> > > > > > many existing violations that need to be accapted.
>> > > > > >
>> > > > > > That's still a much weaker requirement, though, as classes can
>> just
>> > > be
>> > > > > left
>> > > > > > unannotated, which is why I prefer annotating all classes
>> regardless
>> > > of
>> > > > > > location.
>> > > > > >
>> > > > > > > If we have cases that violate the guideline, then I think we
>> either
>> > > > > have
>> > > > > > to
>> > > > > > > remove these methods
>> > > > > >
>> > > > > > How would you handle e.g. extending an existing Public interface
>> > > with a
>> > > > > new
>> > > > > > method in this case, though? You'd be forced to immediately
>> make the
>> > > > new
>> > > > > > method Public as well, or place it somewhere else entirely,
>> which
>> > > leads
>> > > > > to
>> > > > > > unfavorable design. I don't think we should disallow extending
>> > > classes
>> > > > > with
>> > > > > > methods of a weaker stability.
>> > > > > >
>> > > > > >
>> > > > > > Best
>> > > > > > Ingo
>> > > > > >
>> > > > > > On Fri, Dec 3, 2021 at 4:53 PM Till Rohrmann <
>> trohrmann@apache.org>
>> > > > > wrote:
>> > > > > >
>> > > > > > > Hi Ingo, thanks a lot for your thoughts and the work you've
>> spent
>> > > on
>> > > > > this
>> > > > > > > topic already.
>> > > > > > >
>> > > > > > > Personally, I'd be fine to say that in API modules (tbd what
>> this
>> > > is
>> > > > > > > (probably transitive closure of all APIs)) we require every
>> class
>> > > to
>> > > > be
>> > > > > > > annotated.
>> > > > > > >
>> > > > > > > For sake of clarity and having clear rules, this should then
>> also
>> > > > apply
>> > > > > > to
>> > > > > > > nested types. As you've said this would have some additional
>> > > benefits
>> > > > > > when
>> > > > > > > doing refactorings and seems to be actually required by
>> japicmp.
>> > > > > > >
>> > > > > > > >This seems to be quite incompatible with the current
>> > > interpretation
>> > > > in
>> > > > > > the
>> > > > > > > code base, and it would prevent valid (and in-use) use cases
>> like
>> > > > > marking
>> > > > > > > e.g. a single method experimental (or even internal) in an
>> > > otherwise
>> > > > > > public
>> > > > > > > (evolving) API.
>> > > > > > >
>> > > > > > > If we have cases that violate the guideline, then I think we
>> either
>> > > > > have
>> > > > > > to
>> > > > > > > remove these methods or adjust their stability guarantees
>> (time of
>> > > > > > > reckoning ;-). Otherwise, we won't be able to give stability
>> > > > > guarantees,
>> > > > > > I
>> > > > > > > fear.
>> > > > > > >
>> > > > > > > Cheers,
>> > > > > > > Till
>> > > > > > >
>> > > > > > > On Fri, Dec 3, 2021 at 4:23 PM Ingo Bürk <in...@ververica.com>
>> > > wrote:
>> > > > > > >
>> > > > > > > > Hi Till,
>> > > > > > > >
>> > > > > > > > Thanks for starting this discussion, and as you noticed, I
>> qutie
>> > > > care
>> > > > > > for
>> > > > > > > > it as well. :-)
>> > > > > > > >
>> > > > > > > > When implementing the ArchUnit rules, I noticed that
>> > > project-wide,
>> > > > > > > > different teams / modules use and understand the API
>> annotations
>> > > in
>> > > > > > > > different ways. Therefore, I think it is important to both
>> get
>> > > > > clarity
>> > > > > > on
>> > > > > > > > the meaning of these annotations (= your FLIP), but also on
>> how
>> > > to
>> > > > > > > actually
>> > > > > > > > use them.
>> > > > > > > >
>> > > > > > > > One thing I would like to bring up for discussion is
>> requiring an
>> > > > API
>> > > > > > > > annotation on every class. This seems to be a controversial
>> > > > > suggestion,
>> > > > > > > > though personally I think it would allow our tooling do its
>> job
>> > > > best.
>> > > > > > An
>> > > > > > > > obvious downside is "it's annoying", but so is the current
>> > > > > requirement
>> > > > > > of
>> > > > > > > > having JavaDocs on every class (including tests). If this
>> were to
>> > > > > ever
>> > > > > > be
>> > > > > > > > considered, this should be implemented (also) in Checkstyle,
>> > > > though,
>> > > > > to
>> > > > > > > > have it as immediate IDE feedback, which IMO would also
>> mostly
>> > > > > > eliminate
>> > > > > > > > this downside. The other downside is that it encourages
>> throwing
>> > > > > > > > Public(Evolving) around, but that would really only make an
>> > > > existing
>> > > > > > > > invisible problem actually visible; ultimately, I think
>> this is
>> > > > > > something
>> > > > > > > > that committers just need to consider.
>> > > > > > > > It would be nice to only require this in *.api.* packages,
>> but
>> > > > that's
>> > > > > > > just
>> > > > > > > > not the world we live in, nor will it be anytime soon;
>> however,
>> > > it
>> > > > > > would
>> > > > > > > > allow us to make sure that new non-internal APIs are only
>> added
>> > > in
>> > > > > > > *.api.*
>> > > > > > > > packages, or that no public APIs are added in *.internal.*
>> > > > packages.
>> > > > > > > >
>> > > > > > > > Another aspect that came up both when implementing the
>> rules and
>> > > > > again
>> > > > > > > in a
>> > > > > > > > discussion is whether (or when) nested types (classes,
>> > > interfaces –
>> > > > > > > > not members) need to be annotated explicitly. My
>> understanding[1]
>> > > > is
>> > > > > > that
>> > > > > > > > this is actually needed for japicmp to work correctly.
>> Besides
>> > > > that,
>> > > > > I
>> > > > > > > > think doing so adds benefits such as making the breaking
>> change
>> > > > > visible
>> > > > > > > > when promoting a nested type to a top-level type, which
>> would
>> > > > likely
>> > > > > go
>> > > > > > > > unquestioned if the type is not annotated but "inherited"
>> its API
>> > > > > > > > stability, losing it in the process.
>> > > > > > > >
>> > > > > > > > Now to actually comment on your FLIP:
>> > > > > > > >
>> > > > > > > > > In order to determine the stability guarantee a given
>> > > > > class/interface
>> > > > > > > can
>> > > > > > > > provide, we have to take a look at all methods a user has to
>> > > > > implement
>> > > > > > > and
>> > > > > > > > use for using the given class/interface. The weakest
>> guarantee of
>> > > > > these
>> > > > > > > > methods specifies the guarantee we can give for the
>> containing
>> > > > > > > > class/interface.
>> > > > > > > >
>> > > > > > > > This seems to be quite incompatible with the current
>> > > interpretation
>> > > > > in
>> > > > > > > the
>> > > > > > > > code base, and it would prevent valid (and in-use) use
>> cases like
>> > > > > > marking
>> > > > > > > > e.g. a single method experimental (or even internal) in an
>> > > > otherwise
>> > > > > > > public
>> > > > > > > > (evolving) API.
>> > > > > > > >
>> > > > > > > > [1]
>> > > > > https://github.com/apache/flink/pull/17133#issuecomment-925738694
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > Best
>> > > > > > > > Ingo
>> > > > > > > >
>> > > > > > > > On Thu, Dec 2, 2021 at 3:48 PM Till Rohrmann <
>> > > trohrmann@apache.org
>> > > > >
>> > > > > > > wrote:
>> > > > > > > >
>> > > > > > > > > Hi everyone,
>> > > > > > > > >
>> > > > > > > > > I would like to start a discussion about what kind of API
>> > > > stability
>> > > > > > > > > guarantees we want to provide to our users. The Flink
>> community
>> > > > > > already
>> > > > > > > > > agreed on some stability guarantees, but these guarantees
>> were
>> > > > only
>> > > > > > > > > communicated via the ML and not properly documented [2].
>> > > > Moreover,
>> > > > > > I've
>> > > > > > > > > seen more and more complaints about breaking changes (some
>> > > > rightful
>> > > > > > and
>> > > > > > > > > others not) on the ML recently [3] because we rarely mark
>> our
>> > > > APIs
>> > > > > as
>> > > > > > > > > stable. This motivated this FLIP.
>> > > > > > > > >
>> > > > > > > > > The proposal first concentrates on source API stability
>> > > > guarantees.
>> > > > > > > > Binary
>> > > > > > > > > stability guarantees are explicitly left out for a
>> follow-up
>> > > > > > > discussion.
>> > > > > > > > >
>> > > > > > > > > In essence, the proposal follows our current stability
>> > > guarantees
>> > > > > > while
>> > > > > > > > > updating the guarantees for @PublicEvolving that we are
>> already
>> > > > > > > providing
>> > > > > > > > > w/o having stated them. Moreover, this FLIP proposes some
>> > > > > guidelines
>> > > > > > > for
>> > > > > > > > > determining the stability guarantees for an API object,
>> how to
>> > > > > evolve
>> > > > > > > > them
>> > > > > > > > > and some additional requirements for the return and
>> parameter
>> > > > types
>> > > > > > of
>> > > > > > > > > methods.
>> > > > > > > > >
>> > > > > > > > > All in all, I hope that this proposal is more reflecting
>> our
>> > > > > current
>> > > > > > > > > understanding of stability guarantees than being
>> controversial.
>> > > > One
>> > > > > > of
>> > > > > > > > the
>> > > > > > > > > outcomes of this FLIP should be to properly document these
>> > > > > guarantees
>> > > > > > > so
>> > > > > > > > > that it is easily discoverable and understandable for our
>> > > users.
>> > > > > > > > Moreover,
>> > > > > > > > > I hope that we can provide more stable APIs our users can
>> rely
>> > > > and
>> > > > > > > build
>> > > > > > > > > upon.
>> > > > > > > > >
>> > > > > > > > > There will be a follow-up FLIP discussing the problem of
>> how to
>> > > > > make
>> > > > > > > sure
>> > > > > > > > > that APIs become stable over time.
>> > > > > > > > >
>> > > > > > > > > Looking forward to your feedback.
>> > > > > > > > >
>> > > > > > > > > [1] https://cwiki.apache.org/confluence/x/IJeqCw
>> > > > > > > > > [2]
>> > > > > https://lists.apache.org/thread/5jm25783oq5svyk7rr8g1gly2ooxqhjr
>> > > > > > > > > [3]
>> > > > > https://lists.apache.org/thread/kzhfc3t6omzo2kyo8zj9yxoh8twq5fzr
>> > > > > > > > >
>> > > > > > > > > Cheers,
>> > > > > > > > > Till
>> > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>>
>

Re: [DISCUSS] FLIP-196: Source API stability guarantees

[Till Rohrmann <tr...@apache.org>]

Hi Fabian,

Thanks for your input. Adding the definition of the different labels to our
documentation is definitely part of this FLIP.

I agree that deprecation is related to API stability. For the sake of
keeping this FLIP narrowly scoped I left it out, though. For the time being
I suggest that we stick to our current deprecation process that says that a
deprecated API can only be removed after it has been deprecated for at
least one release. Of course, deprecating and removing a @PublicEvolving
API can only happen in minor or major releases, for example. This process
is a bit stricter than what the API stability guarantees require because
with the API stability alone we could remove the APIs directly w/o the
deprecation period. Other details I would leave for a follow up FLIP
which formally defines the deprecation process.

Cheers,
Till

On Tue, Dec 7, 2021 at 10:08 AM Fabian Paul <fp...@apache.org> wrote:

> Hi all,
>
> Thanks Till for starting this discussion. It is great to see these
> facts written down since they definitely caused friction in the past
> because of different interpretations. Overall I agree with everything
> being said in this FLIP. I was just wondering whether we can put the
> label explaining table somewhere into our docs. FLIPs are usually only
> snapshots for the time being and are hard to search for. It would
> allow users to instantly determine the meaning of the used annotation.
>
> I am also missing in this FLIP the connection to the Deprecated
> annotation. I think we also need to clarify how this should be used in
> conjunction with the API stability.
>
> Best,
> Fabian
>
> On Mon, Dec 6, 2021 at 10:03 AM Ingo Bürk <in...@ververica.com> wrote:
> >
> > Hi Till,
> >
> > seems I misunderstood it then; thanks for the clarification! And yes,
> with
> > that I would fully agree.
> >
> >
> > Ingo
> >
> > On Mon, Dec 6, 2021 at 9:59 AM Till Rohrmann <tr...@apache.org>
> wrote:
> >
> > > Hi Ingo,
> > >
> > > No, the added method can have a weaker stability guarantee as long as
> the
> > > user does not have to implement it. In order to give an example the
> > > following extension would be ok imo:
> > >
> > > @Public
> > > interface Foobar {
> > >     @Public
> > >     int foo();
> > >
> > >     @Experimental
> > >     default ExperimentalResult bar() {
> > >       return ExperimentalResult.notSupported();
> > >     }
> > > }
> > >
> > > The following extension would not be ok because here the user needs to
> > > implement something new:
> > >
> > > @Public
> > > interface Foobar {
> > >     @Public
> > >     int foo();
> > >
> > >     @Experimental
> > >     ExperimentalResult bar();
> > > }
> > >
> > > Moreover, if the user uses bar(), then he opts-in to only get
> @Experimental
> > > stability guarantees.
> > >
> > > I will add this example to the FLIP for illustrative purposes.
> > >
> > > Cheers,
> > > Till
> > >
> > > On Fri, Dec 3, 2021 at 6:52 PM Ingo Bürk <in...@ververica.com> wrote:
> > >
> > > > > Would it be enough to say that for example all classes in the
> module
> > > > flink-java have to be annotated? What I would like to avoid is
> having to
> > > > annotate all classes in some internal module like flink-rpc.
> > > >
> > > > I don't think it is, but we certainly could restrict it to certain
> top
> > > > level o.a.f.xyz packages.
> > > >
> > > > > Extending existing classes will only be possible if you can
> provide a
> > > > default implementation
> > > >
> > > > That I'm totally fine with, but based on that sentence in the FLIP
> if I
> > > > have a public interface and extend it, even with a default
> > > implementation,
> > > > I _have_ to have this method be stable already as well, right? I
> couldn't
> > > > for example add an experimental method to an interface.
> > > >
> > > > This would also include all classes used as argument and return type
> of
> > > > such methods too, which seems quite restrictive.
> > > >
> > > >
> > > > Best
> > > > Ingo
> > > >
> > > > On Fri, Dec 3, 2021, 17:51 Till Rohrmann <tr...@apache.org>
> wrote:
> > > >
> > > > > >That's still a much weaker requirement, though, as classes can
> just be
> > > > > left unannotated, which is why I prefer annotating all classes
> > > regardless
> > > > > of location.
> > > > >
> > > > > Would it be enough to say that for example all classes in the
> module
> > > > > flink-java have to be annotated? What I would like to avoid is
> having
> > > to
> > > > > annotate all classes in some internal module like flink-rpc.
> > > > >
> > > > > > How would you handle e.g. extending an existing Public interface
> > > with a
> > > > > new method in this case, though? You'd be forced to immediately
> make
> > > the
> > > > > new method Public as well, or place it somewhere else entirely,
> which
> > > > leads
> > > > > to unfavorable design. I don't think we should disallow extending
> > > classes
> > > > > with methods of a weaker stability.
> > > > >
> > > > > Extending existing classes will only be possible if you can
> provide a
> > > > > default implementation. If the user needs to do something, then it
> is
> > > not
> > > > > compatible and needs to be handled differently (e.g. by offering a
> new
> > > > > experimental interface that one can use). If we don't enforce this,
> > > then
> > > > I
> > > > > don't see how we can provide source stability guarantees.
> > > > >
> > > > > Cheers,
> > > > > Till
> > > > >
> > > > > On Fri, Dec 3, 2021 at 5:22 PM Ingo Bürk <in...@ververica.com>
> wrote:
> > > > >
> > > > > > Hi Till,
> > > > > >
> > > > > > > Personally, I'd be fine to say that in API modules (tbd what
> this
> > > is
> > > > > > > (probably transitive closure of all APIs)) we require every
> class
> > > to
> > > > be
> > > > > > > annotated.
> > > > > >
> > > > > > At least we'll then need the reverse rule: no classes outside
> *.api.*
> > > > > > packages CAN have an API annotation (other than Internal), of
> course
> > > > with
> > > > > > many existing violations that need to be accapted.
> > > > > >
> > > > > > That's still a much weaker requirement, though, as classes can
> just
> > > be
> > > > > left
> > > > > > unannotated, which is why I prefer annotating all classes
> regardless
> > > of
> > > > > > location.
> > > > > >
> > > > > > > If we have cases that violate the guideline, then I think we
> either
> > > > > have
> > > > > > to
> > > > > > > remove these methods
> > > > > >
> > > > > > How would you handle e.g. extending an existing Public interface
> > > with a
> > > > > new
> > > > > > method in this case, though? You'd be forced to immediately make
> the
> > > > new
> > > > > > method Public as well, or place it somewhere else entirely, which
> > > leads
> > > > > to
> > > > > > unfavorable design. I don't think we should disallow extending
> > > classes
> > > > > with
> > > > > > methods of a weaker stability.
> > > > > >
> > > > > >
> > > > > > Best
> > > > > > Ingo
> > > > > >
> > > > > > On Fri, Dec 3, 2021 at 4:53 PM Till Rohrmann <
> trohrmann@apache.org>
> > > > > wrote:
> > > > > >
> > > > > > > Hi Ingo, thanks a lot for your thoughts and the work you've
> spent
> > > on
> > > > > this
> > > > > > > topic already.
> > > > > > >
> > > > > > > Personally, I'd be fine to say that in API modules (tbd what
> this
> > > is
> > > > > > > (probably transitive closure of all APIs)) we require every
> class
> > > to
> > > > be
> > > > > > > annotated.
> > > > > > >
> > > > > > > For sake of clarity and having clear rules, this should then
> also
> > > > apply
> > > > > > to
> > > > > > > nested types. As you've said this would have some additional
> > > benefits
> > > > > > when
> > > > > > > doing refactorings and seems to be actually required by
> japicmp.
> > > > > > >
> > > > > > > >This seems to be quite incompatible with the current
> > > interpretation
> > > > in
> > > > > > the
> > > > > > > code base, and it would prevent valid (and in-use) use cases
> like
> > > > > marking
> > > > > > > e.g. a single method experimental (or even internal) in an
> > > otherwise
> > > > > > public
> > > > > > > (evolving) API.
> > > > > > >
> > > > > > > If we have cases that violate the guideline, then I think we
> either
> > > > > have
> > > > > > to
> > > > > > > remove these methods or adjust their stability guarantees
> (time of
> > > > > > > reckoning ;-). Otherwise, we won't be able to give stability
> > > > > guarantees,
> > > > > > I
> > > > > > > fear.
> > > > > > >
> > > > > > > Cheers,
> > > > > > > Till
> > > > > > >
> > > > > > > On Fri, Dec 3, 2021 at 4:23 PM Ingo Bürk <in...@ververica.com>
> > > wrote:
> > > > > > >
> > > > > > > > Hi Till,
> > > > > > > >
> > > > > > > > Thanks for starting this discussion, and as you noticed, I
> qutie
> > > > care
> > > > > > for
> > > > > > > > it as well. :-)
> > > > > > > >
> > > > > > > > When implementing the ArchUnit rules, I noticed that
> > > project-wide,
> > > > > > > > different teams / modules use and understand the API
> annotations
> > > in
> > > > > > > > different ways. Therefore, I think it is important to both
> get
> > > > > clarity
> > > > > > on
> > > > > > > > the meaning of these annotations (= your FLIP), but also on
> how
> > > to
> > > > > > > actually
> > > > > > > > use them.
> > > > > > > >
> > > > > > > > One thing I would like to bring up for discussion is
> requiring an
> > > > API
> > > > > > > > annotation on every class. This seems to be a controversial
> > > > > suggestion,
> > > > > > > > though personally I think it would allow our tooling do its
> job
> > > > best.
> > > > > > An
> > > > > > > > obvious downside is "it's annoying", but so is the current
> > > > > requirement
> > > > > > of
> > > > > > > > having JavaDocs on every class (including tests). If this
> were to
> > > > > ever
> > > > > > be
> > > > > > > > considered, this should be implemented (also) in Checkstyle,
> > > > though,
> > > > > to
> > > > > > > > have it as immediate IDE feedback, which IMO would also
> mostly
> > > > > > eliminate
> > > > > > > > this downside. The other downside is that it encourages
> throwing
> > > > > > > > Public(Evolving) around, but that would really only make an
> > > > existing
> > > > > > > > invisible problem actually visible; ultimately, I think this
> is
> > > > > > something
> > > > > > > > that committers just need to consider.
> > > > > > > > It would be nice to only require this in *.api.* packages,
> but
> > > > that's
> > > > > > > just
> > > > > > > > not the world we live in, nor will it be anytime soon;
> however,
> > > it
> > > > > > would
> > > > > > > > allow us to make sure that new non-internal APIs are only
> added
> > > in
> > > > > > > *.api.*
> > > > > > > > packages, or that no public APIs are added in *.internal.*
> > > > packages.
> > > > > > > >
> > > > > > > > Another aspect that came up both when implementing the rules
> and
> > > > > again
> > > > > > > in a
> > > > > > > > discussion is whether (or when) nested types (classes,
> > > interfaces –
> > > > > > > > not members) need to be annotated explicitly. My
> understanding[1]
> > > > is
> > > > > > that
> > > > > > > > this is actually needed for japicmp to work correctly.
> Besides
> > > > that,
> > > > > I
> > > > > > > > think doing so adds benefits such as making the breaking
> change
> > > > > visible
> > > > > > > > when promoting a nested type to a top-level type, which would
> > > > likely
> > > > > go
> > > > > > > > unquestioned if the type is not annotated but "inherited"
> its API
> > > > > > > > stability, losing it in the process.
> > > > > > > >
> > > > > > > > Now to actually comment on your FLIP:
> > > > > > > >
> > > > > > > > > In order to determine the stability guarantee a given
> > > > > class/interface
> > > > > > > can
> > > > > > > > provide, we have to take a look at all methods a user has to
> > > > > implement
> > > > > > > and
> > > > > > > > use for using the given class/interface. The weakest
> guarantee of
> > > > > these
> > > > > > > > methods specifies the guarantee we can give for the
> containing
> > > > > > > > class/interface.
> > > > > > > >
> > > > > > > > This seems to be quite incompatible with the current
> > > interpretation
> > > > > in
> > > > > > > the
> > > > > > > > code base, and it would prevent valid (and in-use) use cases
> like
> > > > > > marking
> > > > > > > > e.g. a single method experimental (or even internal) in an
> > > > otherwise
> > > > > > > public
> > > > > > > > (evolving) API.
> > > > > > > >
> > > > > > > > [1]
> > > > > https://github.com/apache/flink/pull/17133#issuecomment-925738694
> > > > > > > >
> > > > > > > >
> > > > > > > > Best
> > > > > > > > Ingo
> > > > > > > >
> > > > > > > > On Thu, Dec 2, 2021 at 3:48 PM Till Rohrmann <
> > > trohrmann@apache.org
> > > > >
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi everyone,
> > > > > > > > >
> > > > > > > > > I would like to start a discussion about what kind of API
> > > > stability
> > > > > > > > > guarantees we want to provide to our users. The Flink
> community
> > > > > > already
> > > > > > > > > agreed on some stability guarantees, but these guarantees
> were
> > > > only
> > > > > > > > > communicated via the ML and not properly documented [2].
> > > > Moreover,
> > > > > > I've
> > > > > > > > > seen more and more complaints about breaking changes (some
> > > > rightful
> > > > > > and
> > > > > > > > > others not) on the ML recently [3] because we rarely mark
> our
> > > > APIs
> > > > > as
> > > > > > > > > stable. This motivated this FLIP.
> > > > > > > > >
> > > > > > > > > The proposal first concentrates on source API stability
> > > > guarantees.
> > > > > > > > Binary
> > > > > > > > > stability guarantees are explicitly left out for a
> follow-up
> > > > > > > discussion.
> > > > > > > > >
> > > > > > > > > In essence, the proposal follows our current stability
> > > guarantees
> > > > > > while
> > > > > > > > > updating the guarantees for @PublicEvolving that we are
> already
> > > > > > > providing
> > > > > > > > > w/o having stated them. Moreover, this FLIP proposes some
> > > > > guidelines
> > > > > > > for
> > > > > > > > > determining the stability guarantees for an API object,
> how to
> > > > > evolve
> > > > > > > > them
> > > > > > > > > and some additional requirements for the return and
> parameter
> > > > types
> > > > > > of
> > > > > > > > > methods.
> > > > > > > > >
> > > > > > > > > All in all, I hope that this proposal is more reflecting
> our
> > > > > current
> > > > > > > > > understanding of stability guarantees than being
> controversial.
> > > > One
> > > > > > of
> > > > > > > > the
> > > > > > > > > outcomes of this FLIP should be to properly document these
> > > > > guarantees
> > > > > > > so
> > > > > > > > > that it is easily discoverable and understandable for our
> > > users.
> > > > > > > > Moreover,
> > > > > > > > > I hope that we can provide more stable APIs our users can
> rely
> > > > and
> > > > > > > build
> > > > > > > > > upon.
> > > > > > > > >
> > > > > > > > > There will be a follow-up FLIP discussing the problem of
> how to
> > > > > make
> > > > > > > sure
> > > > > > > > > that APIs become stable over time.
> > > > > > > > >
> > > > > > > > > Looking forward to your feedback.
> > > > > > > > >
> > > > > > > > > [1] https://cwiki.apache.org/confluence/x/IJeqCw
> > > > > > > > > [2]
> > > > > https://lists.apache.org/thread/5jm25783oq5svyk7rr8g1gly2ooxqhjr
> > > > > > > > > [3]
> > > > > https://lists.apache.org/thread/kzhfc3t6omzo2kyo8zj9yxoh8twq5fzr
> > > > > > > > >
> > > > > > > > > Cheers,
> > > > > > > > > Till
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
>

Re: [DISCUSS] FLIP-196: Source API stability guarantees

[Fabian Paul <fp...@apache.org>]

Hi all,

Thanks Till for starting this discussion. It is great to see these
facts written down since they definitely caused friction in the past
because of different interpretations. Overall I agree with everything
being said in this FLIP. I was just wondering whether we can put the
label explaining table somewhere into our docs. FLIPs are usually only
snapshots for the time being and are hard to search for. It would
allow users to instantly determine the meaning of the used annotation.

I am also missing in this FLIP the connection to the Deprecated
annotation. I think we also need to clarify how this should be used in
conjunction with the API stability.

Best,
Fabian

On Mon, Dec 6, 2021 at 10:03 AM Ingo Bürk <in...@ververica.com> wrote:
>
> Hi Till,
>
> seems I misunderstood it then; thanks for the clarification! And yes, with
> that I would fully agree.
>
>
> Ingo
>
> On Mon, Dec 6, 2021 at 9:59 AM Till Rohrmann <tr...@apache.org> wrote:
>
> > Hi Ingo,
> >
> > No, the added method can have a weaker stability guarantee as long as the
> > user does not have to implement it. In order to give an example the
> > following extension would be ok imo:
> >
> > @Public
> > interface Foobar {
> >     @Public
> >     int foo();
> >
> >     @Experimental
> >     default ExperimentalResult bar() {
> >       return ExperimentalResult.notSupported();
> >     }
> > }
> >
> > The following extension would not be ok because here the user needs to
> > implement something new:
> >
> > @Public
> > interface Foobar {
> >     @Public
> >     int foo();
> >
> >     @Experimental
> >     ExperimentalResult bar();
> > }
> >
> > Moreover, if the user uses bar(), then he opts-in to only get @Experimental
> > stability guarantees.
> >
> > I will add this example to the FLIP for illustrative purposes.
> >
> > Cheers,
> > Till
> >
> > On Fri, Dec 3, 2021 at 6:52 PM Ingo Bürk <in...@ververica.com> wrote:
> >
> > > > Would it be enough to say that for example all classes in the module
> > > flink-java have to be annotated? What I would like to avoid is having to
> > > annotate all classes in some internal module like flink-rpc.
> > >
> > > I don't think it is, but we certainly could restrict it to certain top
> > > level o.a.f.xyz packages.
> > >
> > > > Extending existing classes will only be possible if you can provide a
> > > default implementation
> > >
> > > That I'm totally fine with, but based on that sentence in the FLIP if I
> > > have a public interface and extend it, even with a default
> > implementation,
> > > I _have_ to have this method be stable already as well, right? I couldn't
> > > for example add an experimental method to an interface.
> > >
> > > This would also include all classes used as argument and return type of
> > > such methods too, which seems quite restrictive.
> > >
> > >
> > > Best
> > > Ingo
> > >
> > > On Fri, Dec 3, 2021, 17:51 Till Rohrmann <tr...@apache.org> wrote:
> > >
> > > > >That's still a much weaker requirement, though, as classes can just be
> > > > left unannotated, which is why I prefer annotating all classes
> > regardless
> > > > of location.
> > > >
> > > > Would it be enough to say that for example all classes in the module
> > > > flink-java have to be annotated? What I would like to avoid is having
> > to
> > > > annotate all classes in some internal module like flink-rpc.
> > > >
> > > > > How would you handle e.g. extending an existing Public interface
> > with a
> > > > new method in this case, though? You'd be forced to immediately make
> > the
> > > > new method Public as well, or place it somewhere else entirely, which
> > > leads
> > > > to unfavorable design. I don't think we should disallow extending
> > classes
> > > > with methods of a weaker stability.
> > > >
> > > > Extending existing classes will only be possible if you can provide a
> > > > default implementation. If the user needs to do something, then it is
> > not
> > > > compatible and needs to be handled differently (e.g. by offering a new
> > > > experimental interface that one can use). If we don't enforce this,
> > then
> > > I
> > > > don't see how we can provide source stability guarantees.
> > > >
> > > > Cheers,
> > > > Till
> > > >
> > > > On Fri, Dec 3, 2021 at 5:22 PM Ingo Bürk <in...@ververica.com> wrote:
> > > >
> > > > > Hi Till,
> > > > >
> > > > > > Personally, I'd be fine to say that in API modules (tbd what this
> > is
> > > > > > (probably transitive closure of all APIs)) we require every class
> > to
> > > be
> > > > > > annotated.
> > > > >
> > > > > At least we'll then need the reverse rule: no classes outside *.api.*
> > > > > packages CAN have an API annotation (other than Internal), of course
> > > with
> > > > > many existing violations that need to be accapted.
> > > > >
> > > > > That's still a much weaker requirement, though, as classes can just
> > be
> > > > left
> > > > > unannotated, which is why I prefer annotating all classes regardless
> > of
> > > > > location.
> > > > >
> > > > > > If we have cases that violate the guideline, then I think we either
> > > > have
> > > > > to
> > > > > > remove these methods
> > > > >
> > > > > How would you handle e.g. extending an existing Public interface
> > with a
> > > > new
> > > > > method in this case, though? You'd be forced to immediately make the
> > > new
> > > > > method Public as well, or place it somewhere else entirely, which
> > leads
> > > > to
> > > > > unfavorable design. I don't think we should disallow extending
> > classes
> > > > with
> > > > > methods of a weaker stability.
> > > > >
> > > > >
> > > > > Best
> > > > > Ingo
> > > > >
> > > > > On Fri, Dec 3, 2021 at 4:53 PM Till Rohrmann <tr...@apache.org>
> > > > wrote:
> > > > >
> > > > > > Hi Ingo, thanks a lot for your thoughts and the work you've spent
> > on
> > > > this
> > > > > > topic already.
> > > > > >
> > > > > > Personally, I'd be fine to say that in API modules (tbd what this
> > is
> > > > > > (probably transitive closure of all APIs)) we require every class
> > to
> > > be
> > > > > > annotated.
> > > > > >
> > > > > > For sake of clarity and having clear rules, this should then also
> > > apply
> > > > > to
> > > > > > nested types. As you've said this would have some additional
> > benefits
> > > > > when
> > > > > > doing refactorings and seems to be actually required by japicmp.
> > > > > >
> > > > > > >This seems to be quite incompatible with the current
> > interpretation
> > > in
> > > > > the
> > > > > > code base, and it would prevent valid (and in-use) use cases like
> > > > marking
> > > > > > e.g. a single method experimental (or even internal) in an
> > otherwise
> > > > > public
> > > > > > (evolving) API.
> > > > > >
> > > > > > If we have cases that violate the guideline, then I think we either
> > > > have
> > > > > to
> > > > > > remove these methods or adjust their stability guarantees (time of
> > > > > > reckoning ;-). Otherwise, we won't be able to give stability
> > > > guarantees,
> > > > > I
> > > > > > fear.
> > > > > >
> > > > > > Cheers,
> > > > > > Till
> > > > > >
> > > > > > On Fri, Dec 3, 2021 at 4:23 PM Ingo Bürk <in...@ververica.com>
> > wrote:
> > > > > >
> > > > > > > Hi Till,
> > > > > > >
> > > > > > > Thanks for starting this discussion, and as you noticed, I qutie
> > > care
> > > > > for
> > > > > > > it as well. :-)
> > > > > > >
> > > > > > > When implementing the ArchUnit rules, I noticed that
> > project-wide,
> > > > > > > different teams / modules use and understand the API annotations
> > in
> > > > > > > different ways. Therefore, I think it is important to both get
> > > > clarity
> > > > > on
> > > > > > > the meaning of these annotations (= your FLIP), but also on how
> > to
> > > > > > actually
> > > > > > > use them.
> > > > > > >
> > > > > > > One thing I would like to bring up for discussion is requiring an
> > > API
> > > > > > > annotation on every class. This seems to be a controversial
> > > > suggestion,
> > > > > > > though personally I think it would allow our tooling do its job
> > > best.
> > > > > An
> > > > > > > obvious downside is "it's annoying", but so is the current
> > > > requirement
> > > > > of
> > > > > > > having JavaDocs on every class (including tests). If this were to
> > > > ever
> > > > > be
> > > > > > > considered, this should be implemented (also) in Checkstyle,
> > > though,
> > > > to
> > > > > > > have it as immediate IDE feedback, which IMO would also mostly
> > > > > eliminate
> > > > > > > this downside. The other downside is that it encourages throwing
> > > > > > > Public(Evolving) around, but that would really only make an
> > > existing
> > > > > > > invisible problem actually visible; ultimately, I think this is
> > > > > something
> > > > > > > that committers just need to consider.
> > > > > > > It would be nice to only require this in *.api.* packages, but
> > > that's
> > > > > > just
> > > > > > > not the world we live in, nor will it be anytime soon; however,
> > it
> > > > > would
> > > > > > > allow us to make sure that new non-internal APIs are only added
> > in
> > > > > > *.api.*
> > > > > > > packages, or that no public APIs are added in *.internal.*
> > > packages.
> > > > > > >
> > > > > > > Another aspect that came up both when implementing the rules and
> > > > again
> > > > > > in a
> > > > > > > discussion is whether (or when) nested types (classes,
> > interfaces –
> > > > > > > not members) need to be annotated explicitly. My understanding[1]
> > > is
> > > > > that
> > > > > > > this is actually needed for japicmp to work correctly. Besides
> > > that,
> > > > I
> > > > > > > think doing so adds benefits such as making the breaking change
> > > > visible
> > > > > > > when promoting a nested type to a top-level type, which would
> > > likely
> > > > go
> > > > > > > unquestioned if the type is not annotated but "inherited" its API
> > > > > > > stability, losing it in the process.
> > > > > > >
> > > > > > > Now to actually comment on your FLIP:
> > > > > > >
> > > > > > > > In order to determine the stability guarantee a given
> > > > class/interface
> > > > > > can
> > > > > > > provide, we have to take a look at all methods a user has to
> > > > implement
> > > > > > and
> > > > > > > use for using the given class/interface. The weakest guarantee of
> > > > these
> > > > > > > methods specifies the guarantee we can give for the containing
> > > > > > > class/interface.
> > > > > > >
> > > > > > > This seems to be quite incompatible with the current
> > interpretation
> > > > in
> > > > > > the
> > > > > > > code base, and it would prevent valid (and in-use) use cases like
> > > > > marking
> > > > > > > e.g. a single method experimental (or even internal) in an
> > > otherwise
> > > > > > public
> > > > > > > (evolving) API.
> > > > > > >
> > > > > > > [1]
> > > > https://github.com/apache/flink/pull/17133#issuecomment-925738694
> > > > > > >
> > > > > > >
> > > > > > > Best
> > > > > > > Ingo
> > > > > > >
> > > > > > > On Thu, Dec 2, 2021 at 3:48 PM Till Rohrmann <
> > trohrmann@apache.org
> > > >
> > > > > > wrote:
> > > > > > >
> > > > > > > > Hi everyone,
> > > > > > > >
> > > > > > > > I would like to start a discussion about what kind of API
> > > stability
> > > > > > > > guarantees we want to provide to our users. The Flink community
> > > > > already
> > > > > > > > agreed on some stability guarantees, but these guarantees were
> > > only
> > > > > > > > communicated via the ML and not properly documented [2].
> > > Moreover,
> > > > > I've
> > > > > > > > seen more and more complaints about breaking changes (some
> > > rightful
> > > > > and
> > > > > > > > others not) on the ML recently [3] because we rarely mark our
> > > APIs
> > > > as
> > > > > > > > stable. This motivated this FLIP.
> > > > > > > >
> > > > > > > > The proposal first concentrates on source API stability
> > > guarantees.
> > > > > > > Binary
> > > > > > > > stability guarantees are explicitly left out for a follow-up
> > > > > > discussion.
> > > > > > > >
> > > > > > > > In essence, the proposal follows our current stability
> > guarantees
> > > > > while
> > > > > > > > updating the guarantees for @PublicEvolving that we are already
> > > > > > providing
> > > > > > > > w/o having stated them. Moreover, this FLIP proposes some
> > > > guidelines
> > > > > > for
> > > > > > > > determining the stability guarantees for an API object, how to
> > > > evolve
> > > > > > > them
> > > > > > > > and some additional requirements for the return and parameter
> > > types
> > > > > of
> > > > > > > > methods.
> > > > > > > >
> > > > > > > > All in all, I hope that this proposal is more reflecting our
> > > > current
> > > > > > > > understanding of stability guarantees than being controversial.
> > > One
> > > > > of
> > > > > > > the
> > > > > > > > outcomes of this FLIP should be to properly document these
> > > > guarantees
> > > > > > so
> > > > > > > > that it is easily discoverable and understandable for our
> > users.
> > > > > > > Moreover,
> > > > > > > > I hope that we can provide more stable APIs our users can rely
> > > and
> > > > > > build
> > > > > > > > upon.
> > > > > > > >
> > > > > > > > There will be a follow-up FLIP discussing the problem of how to
> > > > make
> > > > > > sure
> > > > > > > > that APIs become stable over time.
> > > > > > > >
> > > > > > > > Looking forward to your feedback.
> > > > > > > >
> > > > > > > > [1] https://cwiki.apache.org/confluence/x/IJeqCw
> > > > > > > > [2]
> > > > https://lists.apache.org/thread/5jm25783oq5svyk7rr8g1gly2ooxqhjr
> > > > > > > > [3]
> > > > https://lists.apache.org/thread/kzhfc3t6omzo2kyo8zj9yxoh8twq5fzr
> > > > > > > >
> > > > > > > > Cheers,
> > > > > > > > Till
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >

Re: [DISCUSS] FLIP-196: Source API stability guarantees

[Ingo Bürk <in...@ververica.com>]

Hi Till,

seems I misunderstood it then; thanks for the clarification! And yes, with
that I would fully agree.


Ingo

On Mon, Dec 6, 2021 at 9:59 AM Till Rohrmann <tr...@apache.org> wrote:

> Hi Ingo,
>
> No, the added method can have a weaker stability guarantee as long as the
> user does not have to implement it. In order to give an example the
> following extension would be ok imo:
>
> @Public
> interface Foobar {
>     @Public
>     int foo();
>
>     @Experimental
>     default ExperimentalResult bar() {
>       return ExperimentalResult.notSupported();
>     }
> }
>
> The following extension would not be ok because here the user needs to
> implement something new:
>
> @Public
> interface Foobar {
>     @Public
>     int foo();
>
>     @Experimental
>     ExperimentalResult bar();
> }
>
> Moreover, if the user uses bar(), then he opts-in to only get @Experimental
> stability guarantees.
>
> I will add this example to the FLIP for illustrative purposes.
>
> Cheers,
> Till
>
> On Fri, Dec 3, 2021 at 6:52 PM Ingo Bürk <in...@ververica.com> wrote:
>
> > > Would it be enough to say that for example all classes in the module
> > flink-java have to be annotated? What I would like to avoid is having to
> > annotate all classes in some internal module like flink-rpc.
> >
> > I don't think it is, but we certainly could restrict it to certain top
> > level o.a.f.xyz packages.
> >
> > > Extending existing classes will only be possible if you can provide a
> > default implementation
> >
> > That I'm totally fine with, but based on that sentence in the FLIP if I
> > have a public interface and extend it, even with a default
> implementation,
> > I _have_ to have this method be stable already as well, right? I couldn't
> > for example add an experimental method to an interface.
> >
> > This would also include all classes used as argument and return type of
> > such methods too, which seems quite restrictive.
> >
> >
> > Best
> > Ingo
> >
> > On Fri, Dec 3, 2021, 17:51 Till Rohrmann <tr...@apache.org> wrote:
> >
> > > >That's still a much weaker requirement, though, as classes can just be
> > > left unannotated, which is why I prefer annotating all classes
> regardless
> > > of location.
> > >
> > > Would it be enough to say that for example all classes in the module
> > > flink-java have to be annotated? What I would like to avoid is having
> to
> > > annotate all classes in some internal module like flink-rpc.
> > >
> > > > How would you handle e.g. extending an existing Public interface
> with a
> > > new method in this case, though? You'd be forced to immediately make
> the
> > > new method Public as well, or place it somewhere else entirely, which
> > leads
> > > to unfavorable design. I don't think we should disallow extending
> classes
> > > with methods of a weaker stability.
> > >
> > > Extending existing classes will only be possible if you can provide a
> > > default implementation. If the user needs to do something, then it is
> not
> > > compatible and needs to be handled differently (e.g. by offering a new
> > > experimental interface that one can use). If we don't enforce this,
> then
> > I
> > > don't see how we can provide source stability guarantees.
> > >
> > > Cheers,
> > > Till
> > >
> > > On Fri, Dec 3, 2021 at 5:22 PM Ingo Bürk <in...@ververica.com> wrote:
> > >
> > > > Hi Till,
> > > >
> > > > > Personally, I'd be fine to say that in API modules (tbd what this
> is
> > > > > (probably transitive closure of all APIs)) we require every class
> to
> > be
> > > > > annotated.
> > > >
> > > > At least we'll then need the reverse rule: no classes outside *.api.*
> > > > packages CAN have an API annotation (other than Internal), of course
> > with
> > > > many existing violations that need to be accapted.
> > > >
> > > > That's still a much weaker requirement, though, as classes can just
> be
> > > left
> > > > unannotated, which is why I prefer annotating all classes regardless
> of
> > > > location.
> > > >
> > > > > If we have cases that violate the guideline, then I think we either
> > > have
> > > > to
> > > > > remove these methods
> > > >
> > > > How would you handle e.g. extending an existing Public interface
> with a
> > > new
> > > > method in this case, though? You'd be forced to immediately make the
> > new
> > > > method Public as well, or place it somewhere else entirely, which
> leads
> > > to
> > > > unfavorable design. I don't think we should disallow extending
> classes
> > > with
> > > > methods of a weaker stability.
> > > >
> > > >
> > > > Best
> > > > Ingo
> > > >
> > > > On Fri, Dec 3, 2021 at 4:53 PM Till Rohrmann <tr...@apache.org>
> > > wrote:
> > > >
> > > > > Hi Ingo, thanks a lot for your thoughts and the work you've spent
> on
> > > this
> > > > > topic already.
> > > > >
> > > > > Personally, I'd be fine to say that in API modules (tbd what this
> is
> > > > > (probably transitive closure of all APIs)) we require every class
> to
> > be
> > > > > annotated.
> > > > >
> > > > > For sake of clarity and having clear rules, this should then also
> > apply
> > > > to
> > > > > nested types. As you've said this would have some additional
> benefits
> > > > when
> > > > > doing refactorings and seems to be actually required by japicmp.
> > > > >
> > > > > >This seems to be quite incompatible with the current
> interpretation
> > in
> > > > the
> > > > > code base, and it would prevent valid (and in-use) use cases like
> > > marking
> > > > > e.g. a single method experimental (or even internal) in an
> otherwise
> > > > public
> > > > > (evolving) API.
> > > > >
> > > > > If we have cases that violate the guideline, then I think we either
> > > have
> > > > to
> > > > > remove these methods or adjust their stability guarantees (time of
> > > > > reckoning ;-). Otherwise, we won't be able to give stability
> > > guarantees,
> > > > I
> > > > > fear.
> > > > >
> > > > > Cheers,
> > > > > Till
> > > > >
> > > > > On Fri, Dec 3, 2021 at 4:23 PM Ingo Bürk <in...@ververica.com>
> wrote:
> > > > >
> > > > > > Hi Till,
> > > > > >
> > > > > > Thanks for starting this discussion, and as you noticed, I qutie
> > care
> > > > for
> > > > > > it as well. :-)
> > > > > >
> > > > > > When implementing the ArchUnit rules, I noticed that
> project-wide,
> > > > > > different teams / modules use and understand the API annotations
> in
> > > > > > different ways. Therefore, I think it is important to both get
> > > clarity
> > > > on
> > > > > > the meaning of these annotations (= your FLIP), but also on how
> to
> > > > > actually
> > > > > > use them.
> > > > > >
> > > > > > One thing I would like to bring up for discussion is requiring an
> > API
> > > > > > annotation on every class. This seems to be a controversial
> > > suggestion,
> > > > > > though personally I think it would allow our tooling do its job
> > best.
> > > > An
> > > > > > obvious downside is "it's annoying", but so is the current
> > > requirement
> > > > of
> > > > > > having JavaDocs on every class (including tests). If this were to
> > > ever
> > > > be
> > > > > > considered, this should be implemented (also) in Checkstyle,
> > though,
> > > to
> > > > > > have it as immediate IDE feedback, which IMO would also mostly
> > > > eliminate
> > > > > > this downside. The other downside is that it encourages throwing
> > > > > > Public(Evolving) around, but that would really only make an
> > existing
> > > > > > invisible problem actually visible; ultimately, I think this is
> > > > something
> > > > > > that committers just need to consider.
> > > > > > It would be nice to only require this in *.api.* packages, but
> > that's
> > > > > just
> > > > > > not the world we live in, nor will it be anytime soon; however,
> it
> > > > would
> > > > > > allow us to make sure that new non-internal APIs are only added
> in
> > > > > *.api.*
> > > > > > packages, or that no public APIs are added in *.internal.*
> > packages.
> > > > > >
> > > > > > Another aspect that came up both when implementing the rules and
> > > again
> > > > > in a
> > > > > > discussion is whether (or when) nested types (classes,
> interfaces –
> > > > > > not members) need to be annotated explicitly. My understanding[1]
> > is
> > > > that
> > > > > > this is actually needed for japicmp to work correctly. Besides
> > that,
> > > I
> > > > > > think doing so adds benefits such as making the breaking change
> > > visible
> > > > > > when promoting a nested type to a top-level type, which would
> > likely
> > > go
> > > > > > unquestioned if the type is not annotated but "inherited" its API
> > > > > > stability, losing it in the process.
> > > > > >
> > > > > > Now to actually comment on your FLIP:
> > > > > >
> > > > > > > In order to determine the stability guarantee a given
> > > class/interface
> > > > > can
> > > > > > provide, we have to take a look at all methods a user has to
> > > implement
> > > > > and
> > > > > > use for using the given class/interface. The weakest guarantee of
> > > these
> > > > > > methods specifies the guarantee we can give for the containing
> > > > > > class/interface.
> > > > > >
> > > > > > This seems to be quite incompatible with the current
> interpretation
> > > in
> > > > > the
> > > > > > code base, and it would prevent valid (and in-use) use cases like
> > > > marking
> > > > > > e.g. a single method experimental (or even internal) in an
> > otherwise
> > > > > public
> > > > > > (evolving) API.
> > > > > >
> > > > > > [1]
> > > https://github.com/apache/flink/pull/17133#issuecomment-925738694
> > > > > >
> > > > > >
> > > > > > Best
> > > > > > Ingo
> > > > > >
> > > > > > On Thu, Dec 2, 2021 at 3:48 PM Till Rohrmann <
> trohrmann@apache.org
> > >
> > > > > wrote:
> > > > > >
> > > > > > > Hi everyone,
> > > > > > >
> > > > > > > I would like to start a discussion about what kind of API
> > stability
> > > > > > > guarantees we want to provide to our users. The Flink community
> > > > already
> > > > > > > agreed on some stability guarantees, but these guarantees were
> > only
> > > > > > > communicated via the ML and not properly documented [2].
> > Moreover,
> > > > I've
> > > > > > > seen more and more complaints about breaking changes (some
> > rightful
> > > > and
> > > > > > > others not) on the ML recently [3] because we rarely mark our
> > APIs
> > > as
> > > > > > > stable. This motivated this FLIP.
> > > > > > >
> > > > > > > The proposal first concentrates on source API stability
> > guarantees.
> > > > > > Binary
> > > > > > > stability guarantees are explicitly left out for a follow-up
> > > > > discussion.
> > > > > > >
> > > > > > > In essence, the proposal follows our current stability
> guarantees
> > > > while
> > > > > > > updating the guarantees for @PublicEvolving that we are already
> > > > > providing
> > > > > > > w/o having stated them. Moreover, this FLIP proposes some
> > > guidelines
> > > > > for
> > > > > > > determining the stability guarantees for an API object, how to
> > > evolve
> > > > > > them
> > > > > > > and some additional requirements for the return and parameter
> > types
> > > > of
> > > > > > > methods.
> > > > > > >
> > > > > > > All in all, I hope that this proposal is more reflecting our
> > > current
> > > > > > > understanding of stability guarantees than being controversial.
> > One
> > > > of
> > > > > > the
> > > > > > > outcomes of this FLIP should be to properly document these
> > > guarantees
> > > > > so
> > > > > > > that it is easily discoverable and understandable for our
> users.
> > > > > > Moreover,
> > > > > > > I hope that we can provide more stable APIs our users can rely
> > and
> > > > > build
> > > > > > > upon.
> > > > > > >
> > > > > > > There will be a follow-up FLIP discussing the problem of how to
> > > make
> > > > > sure
> > > > > > > that APIs become stable over time.
> > > > > > >
> > > > > > > Looking forward to your feedback.
> > > > > > >
> > > > > > > [1] https://cwiki.apache.org/confluence/x/IJeqCw
> > > > > > > [2]
> > > https://lists.apache.org/thread/5jm25783oq5svyk7rr8g1gly2ooxqhjr
> > > > > > > [3]
> > > https://lists.apache.org/thread/kzhfc3t6omzo2kyo8zj9yxoh8twq5fzr
> > > > > > >
> > > > > > > Cheers,
> > > > > > > Till
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] FLIP-196: Source API stability guarantees

[Till Rohrmann <tr...@apache.org>]

Hi Ingo,

No, the added method can have a weaker stability guarantee as long as the
user does not have to implement it. In order to give an example the
following extension would be ok imo:

@Public
interface Foobar {
    @Public
    int foo();

    @Experimental
    default ExperimentalResult bar() {
      return ExperimentalResult.notSupported();
    }
}

The following extension would not be ok because here the user needs to
implement something new:

@Public
interface Foobar {
    @Public
    int foo();

    @Experimental
    ExperimentalResult bar();
}

Moreover, if the user uses bar(), then he opts-in to only get @Experimental
stability guarantees.

I will add this example to the FLIP for illustrative purposes.

Cheers,
Till

On Fri, Dec 3, 2021 at 6:52 PM Ingo Bürk <in...@ververica.com> wrote:

> > Would it be enough to say that for example all classes in the module
> flink-java have to be annotated? What I would like to avoid is having to
> annotate all classes in some internal module like flink-rpc.
>
> I don't think it is, but we certainly could restrict it to certain top
> level o.a.f.xyz packages.
>
> > Extending existing classes will only be possible if you can provide a
> default implementation
>
> That I'm totally fine with, but based on that sentence in the FLIP if I
> have a public interface and extend it, even with a default implementation,
> I _have_ to have this method be stable already as well, right? I couldn't
> for example add an experimental method to an interface.
>
> This would also include all classes used as argument and return type of
> such methods too, which seems quite restrictive.
>
>
> Best
> Ingo
>
> On Fri, Dec 3, 2021, 17:51 Till Rohrmann <tr...@apache.org> wrote:
>
> > >That's still a much weaker requirement, though, as classes can just be
> > left unannotated, which is why I prefer annotating all classes regardless
> > of location.
> >
> > Would it be enough to say that for example all classes in the module
> > flink-java have to be annotated? What I would like to avoid is having to
> > annotate all classes in some internal module like flink-rpc.
> >
> > > How would you handle e.g. extending an existing Public interface with a
> > new method in this case, though? You'd be forced to immediately make the
> > new method Public as well, or place it somewhere else entirely, which
> leads
> > to unfavorable design. I don't think we should disallow extending classes
> > with methods of a weaker stability.
> >
> > Extending existing classes will only be possible if you can provide a
> > default implementation. If the user needs to do something, then it is not
> > compatible and needs to be handled differently (e.g. by offering a new
> > experimental interface that one can use). If we don't enforce this, then
> I
> > don't see how we can provide source stability guarantees.
> >
> > Cheers,
> > Till
> >
> > On Fri, Dec 3, 2021 at 5:22 PM Ingo Bürk <in...@ververica.com> wrote:
> >
> > > Hi Till,
> > >
> > > > Personally, I'd be fine to say that in API modules (tbd what this is
> > > > (probably transitive closure of all APIs)) we require every class to
> be
> > > > annotated.
> > >
> > > At least we'll then need the reverse rule: no classes outside *.api.*
> > > packages CAN have an API annotation (other than Internal), of course
> with
> > > many existing violations that need to be accapted.
> > >
> > > That's still a much weaker requirement, though, as classes can just be
> > left
> > > unannotated, which is why I prefer annotating all classes regardless of
> > > location.
> > >
> > > > If we have cases that violate the guideline, then I think we either
> > have
> > > to
> > > > remove these methods
> > >
> > > How would you handle e.g. extending an existing Public interface with a
> > new
> > > method in this case, though? You'd be forced to immediately make the
> new
> > > method Public as well, or place it somewhere else entirely, which leads
> > to
> > > unfavorable design. I don't think we should disallow extending classes
> > with
> > > methods of a weaker stability.
> > >
> > >
> > > Best
> > > Ingo
> > >
> > > On Fri, Dec 3, 2021 at 4:53 PM Till Rohrmann <tr...@apache.org>
> > wrote:
> > >
> > > > Hi Ingo, thanks a lot for your thoughts and the work you've spent on
> > this
> > > > topic already.
> > > >
> > > > Personally, I'd be fine to say that in API modules (tbd what this is
> > > > (probably transitive closure of all APIs)) we require every class to
> be
> > > > annotated.
> > > >
> > > > For sake of clarity and having clear rules, this should then also
> apply
> > > to
> > > > nested types. As you've said this would have some additional benefits
> > > when
> > > > doing refactorings and seems to be actually required by japicmp.
> > > >
> > > > >This seems to be quite incompatible with the current interpretation
> in
> > > the
> > > > code base, and it would prevent valid (and in-use) use cases like
> > marking
> > > > e.g. a single method experimental (or even internal) in an otherwise
> > > public
> > > > (evolving) API.
> > > >
> > > > If we have cases that violate the guideline, then I think we either
> > have
> > > to
> > > > remove these methods or adjust their stability guarantees (time of
> > > > reckoning ;-). Otherwise, we won't be able to give stability
> > guarantees,
> > > I
> > > > fear.
> > > >
> > > > Cheers,
> > > > Till
> > > >
> > > > On Fri, Dec 3, 2021 at 4:23 PM Ingo Bürk <in...@ververica.com> wrote:
> > > >
> > > > > Hi Till,
> > > > >
> > > > > Thanks for starting this discussion, and as you noticed, I qutie
> care
> > > for
> > > > > it as well. :-)
> > > > >
> > > > > When implementing the ArchUnit rules, I noticed that project-wide,
> > > > > different teams / modules use and understand the API annotations in
> > > > > different ways. Therefore, I think it is important to both get
> > clarity
> > > on
> > > > > the meaning of these annotations (= your FLIP), but also on how to
> > > > actually
> > > > > use them.
> > > > >
> > > > > One thing I would like to bring up for discussion is requiring an
> API
> > > > > annotation on every class. This seems to be a controversial
> > suggestion,
> > > > > though personally I think it would allow our tooling do its job
> best.
> > > An
> > > > > obvious downside is "it's annoying", but so is the current
> > requirement
> > > of
> > > > > having JavaDocs on every class (including tests). If this were to
> > ever
> > > be
> > > > > considered, this should be implemented (also) in Checkstyle,
> though,
> > to
> > > > > have it as immediate IDE feedback, which IMO would also mostly
> > > eliminate
> > > > > this downside. The other downside is that it encourages throwing
> > > > > Public(Evolving) around, but that would really only make an
> existing
> > > > > invisible problem actually visible; ultimately, I think this is
> > > something
> > > > > that committers just need to consider.
> > > > > It would be nice to only require this in *.api.* packages, but
> that's
> > > > just
> > > > > not the world we live in, nor will it be anytime soon; however, it
> > > would
> > > > > allow us to make sure that new non-internal APIs are only added in
> > > > *.api.*
> > > > > packages, or that no public APIs are added in *.internal.*
> packages.
> > > > >
> > > > > Another aspect that came up both when implementing the rules and
> > again
> > > > in a
> > > > > discussion is whether (or when) nested types (classes, interfaces –
> > > > > not members) need to be annotated explicitly. My understanding[1]
> is
> > > that
> > > > > this is actually needed for japicmp to work correctly. Besides
> that,
> > I
> > > > > think doing so adds benefits such as making the breaking change
> > visible
> > > > > when promoting a nested type to a top-level type, which would
> likely
> > go
> > > > > unquestioned if the type is not annotated but "inherited" its API
> > > > > stability, losing it in the process.
> > > > >
> > > > > Now to actually comment on your FLIP:
> > > > >
> > > > > > In order to determine the stability guarantee a given
> > class/interface
> > > > can
> > > > > provide, we have to take a look at all methods a user has to
> > implement
> > > > and
> > > > > use for using the given class/interface. The weakest guarantee of
> > these
> > > > > methods specifies the guarantee we can give for the containing
> > > > > class/interface.
> > > > >
> > > > > This seems to be quite incompatible with the current interpretation
> > in
> > > > the
> > > > > code base, and it would prevent valid (and in-use) use cases like
> > > marking
> > > > > e.g. a single method experimental (or even internal) in an
> otherwise
> > > > public
> > > > > (evolving) API.
> > > > >
> > > > > [1]
> > https://github.com/apache/flink/pull/17133#issuecomment-925738694
> > > > >
> > > > >
> > > > > Best
> > > > > Ingo
> > > > >
> > > > > On Thu, Dec 2, 2021 at 3:48 PM Till Rohrmann <trohrmann@apache.org
> >
> > > > wrote:
> > > > >
> > > > > > Hi everyone,
> > > > > >
> > > > > > I would like to start a discussion about what kind of API
> stability
> > > > > > guarantees we want to provide to our users. The Flink community
> > > already
> > > > > > agreed on some stability guarantees, but these guarantees were
> only
> > > > > > communicated via the ML and not properly documented [2].
> Moreover,
> > > I've
> > > > > > seen more and more complaints about breaking changes (some
> rightful
> > > and
> > > > > > others not) on the ML recently [3] because we rarely mark our
> APIs
> > as
> > > > > > stable. This motivated this FLIP.
> > > > > >
> > > > > > The proposal first concentrates on source API stability
> guarantees.
> > > > > Binary
> > > > > > stability guarantees are explicitly left out for a follow-up
> > > > discussion.
> > > > > >
> > > > > > In essence, the proposal follows our current stability guarantees
> > > while
> > > > > > updating the guarantees for @PublicEvolving that we are already
> > > > providing
> > > > > > w/o having stated them. Moreover, this FLIP proposes some
> > guidelines
> > > > for
> > > > > > determining the stability guarantees for an API object, how to
> > evolve
> > > > > them
> > > > > > and some additional requirements for the return and parameter
> types
> > > of
> > > > > > methods.
> > > > > >
> > > > > > All in all, I hope that this proposal is more reflecting our
> > current
> > > > > > understanding of stability guarantees than being controversial.
> One
> > > of
> > > > > the
> > > > > > outcomes of this FLIP should be to properly document these
> > guarantees
> > > > so
> > > > > > that it is easily discoverable and understandable for our users.
> > > > > Moreover,
> > > > > > I hope that we can provide more stable APIs our users can rely
> and
> > > > build
> > > > > > upon.
> > > > > >
> > > > > > There will be a follow-up FLIP discussing the problem of how to
> > make
> > > > sure
> > > > > > that APIs become stable over time.
> > > > > >
> > > > > > Looking forward to your feedback.
> > > > > >
> > > > > > [1] https://cwiki.apache.org/confluence/x/IJeqCw
> > > > > > [2]
> > https://lists.apache.org/thread/5jm25783oq5svyk7rr8g1gly2ooxqhjr
> > > > > > [3]
> > https://lists.apache.org/thread/kzhfc3t6omzo2kyo8zj9yxoh8twq5fzr
> > > > > >
> > > > > > Cheers,
> > > > > > Till
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] FLIP-196: Source API stability guarantees

[Ingo Bürk <in...@ververica.com>]

> Would it be enough to say that for example all classes in the module
flink-java have to be annotated? What I would like to avoid is having to
annotate all classes in some internal module like flink-rpc.

I don't think it is, but we certainly could restrict it to certain top
level o.a.f.xyz packages.

> Extending existing classes will only be possible if you can provide a
default implementation

That I'm totally fine with, but based on that sentence in the FLIP if I
have a public interface and extend it, even with a default implementation,
I _have_ to have this method be stable already as well, right? I couldn't
for example add an experimental method to an interface.

This would also include all classes used as argument and return type of
such methods too, which seems quite restrictive.


Best
Ingo

On Fri, Dec 3, 2021, 17:51 Till Rohrmann <tr...@apache.org> wrote:

> >That's still a much weaker requirement, though, as classes can just be
> left unannotated, which is why I prefer annotating all classes regardless
> of location.
>
> Would it be enough to say that for example all classes in the module
> flink-java have to be annotated? What I would like to avoid is having to
> annotate all classes in some internal module like flink-rpc.
>
> > How would you handle e.g. extending an existing Public interface with a
> new method in this case, though? You'd be forced to immediately make the
> new method Public as well, or place it somewhere else entirely, which leads
> to unfavorable design. I don't think we should disallow extending classes
> with methods of a weaker stability.
>
> Extending existing classes will only be possible if you can provide a
> default implementation. If the user needs to do something, then it is not
> compatible and needs to be handled differently (e.g. by offering a new
> experimental interface that one can use). If we don't enforce this, then I
> don't see how we can provide source stability guarantees.
>
> Cheers,
> Till
>
> On Fri, Dec 3, 2021 at 5:22 PM Ingo Bürk <in...@ververica.com> wrote:
>
> > Hi Till,
> >
> > > Personally, I'd be fine to say that in API modules (tbd what this is
> > > (probably transitive closure of all APIs)) we require every class to be
> > > annotated.
> >
> > At least we'll then need the reverse rule: no classes outside *.api.*
> > packages CAN have an API annotation (other than Internal), of course with
> > many existing violations that need to be accapted.
> >
> > That's still a much weaker requirement, though, as classes can just be
> left
> > unannotated, which is why I prefer annotating all classes regardless of
> > location.
> >
> > > If we have cases that violate the guideline, then I think we either
> have
> > to
> > > remove these methods
> >
> > How would you handle e.g. extending an existing Public interface with a
> new
> > method in this case, though? You'd be forced to immediately make the new
> > method Public as well, or place it somewhere else entirely, which leads
> to
> > unfavorable design. I don't think we should disallow extending classes
> with
> > methods of a weaker stability.
> >
> >
> > Best
> > Ingo
> >
> > On Fri, Dec 3, 2021 at 4:53 PM Till Rohrmann <tr...@apache.org>
> wrote:
> >
> > > Hi Ingo, thanks a lot for your thoughts and the work you've spent on
> this
> > > topic already.
> > >
> > > Personally, I'd be fine to say that in API modules (tbd what this is
> > > (probably transitive closure of all APIs)) we require every class to be
> > > annotated.
> > >
> > > For sake of clarity and having clear rules, this should then also apply
> > to
> > > nested types. As you've said this would have some additional benefits
> > when
> > > doing refactorings and seems to be actually required by japicmp.
> > >
> > > >This seems to be quite incompatible with the current interpretation in
> > the
> > > code base, and it would prevent valid (and in-use) use cases like
> marking
> > > e.g. a single method experimental (or even internal) in an otherwise
> > public
> > > (evolving) API.
> > >
> > > If we have cases that violate the guideline, then I think we either
> have
> > to
> > > remove these methods or adjust their stability guarantees (time of
> > > reckoning ;-). Otherwise, we won't be able to give stability
> guarantees,
> > I
> > > fear.
> > >
> > > Cheers,
> > > Till
> > >
> > > On Fri, Dec 3, 2021 at 4:23 PM Ingo Bürk <in...@ververica.com> wrote:
> > >
> > > > Hi Till,
> > > >
> > > > Thanks for starting this discussion, and as you noticed, I qutie care
> > for
> > > > it as well. :-)
> > > >
> > > > When implementing the ArchUnit rules, I noticed that project-wide,
> > > > different teams / modules use and understand the API annotations in
> > > > different ways. Therefore, I think it is important to both get
> clarity
> > on
> > > > the meaning of these annotations (= your FLIP), but also on how to
> > > actually
> > > > use them.
> > > >
> > > > One thing I would like to bring up for discussion is requiring an API
> > > > annotation on every class. This seems to be a controversial
> suggestion,
> > > > though personally I think it would allow our tooling do its job best.
> > An
> > > > obvious downside is "it's annoying", but so is the current
> requirement
> > of
> > > > having JavaDocs on every class (including tests). If this were to
> ever
> > be
> > > > considered, this should be implemented (also) in Checkstyle, though,
> to
> > > > have it as immediate IDE feedback, which IMO would also mostly
> > eliminate
> > > > this downside. The other downside is that it encourages throwing
> > > > Public(Evolving) around, but that would really only make an existing
> > > > invisible problem actually visible; ultimately, I think this is
> > something
> > > > that committers just need to consider.
> > > > It would be nice to only require this in *.api.* packages, but that's
> > > just
> > > > not the world we live in, nor will it be anytime soon; however, it
> > would
> > > > allow us to make sure that new non-internal APIs are only added in
> > > *.api.*
> > > > packages, or that no public APIs are added in *.internal.* packages.
> > > >
> > > > Another aspect that came up both when implementing the rules and
> again
> > > in a
> > > > discussion is whether (or when) nested types (classes, interfaces –
> > > > not members) need to be annotated explicitly. My understanding[1] is
> > that
> > > > this is actually needed for japicmp to work correctly. Besides that,
> I
> > > > think doing so adds benefits such as making the breaking change
> visible
> > > > when promoting a nested type to a top-level type, which would likely
> go
> > > > unquestioned if the type is not annotated but "inherited" its API
> > > > stability, losing it in the process.
> > > >
> > > > Now to actually comment on your FLIP:
> > > >
> > > > > In order to determine the stability guarantee a given
> class/interface
> > > can
> > > > provide, we have to take a look at all methods a user has to
> implement
> > > and
> > > > use for using the given class/interface. The weakest guarantee of
> these
> > > > methods specifies the guarantee we can give for the containing
> > > > class/interface.
> > > >
> > > > This seems to be quite incompatible with the current interpretation
> in
> > > the
> > > > code base, and it would prevent valid (and in-use) use cases like
> > marking
> > > > e.g. a single method experimental (or even internal) in an otherwise
> > > public
> > > > (evolving) API.
> > > >
> > > > [1]
> https://github.com/apache/flink/pull/17133#issuecomment-925738694
> > > >
> > > >
> > > > Best
> > > > Ingo
> > > >
> > > > On Thu, Dec 2, 2021 at 3:48 PM Till Rohrmann <tr...@apache.org>
> > > wrote:
> > > >
> > > > > Hi everyone,
> > > > >
> > > > > I would like to start a discussion about what kind of API stability
> > > > > guarantees we want to provide to our users. The Flink community
> > already
> > > > > agreed on some stability guarantees, but these guarantees were only
> > > > > communicated via the ML and not properly documented [2]. Moreover,
> > I've
> > > > > seen more and more complaints about breaking changes (some rightful
> > and
> > > > > others not) on the ML recently [3] because we rarely mark our APIs
> as
> > > > > stable. This motivated this FLIP.
> > > > >
> > > > > The proposal first concentrates on source API stability guarantees.
> > > > Binary
> > > > > stability guarantees are explicitly left out for a follow-up
> > > discussion.
> > > > >
> > > > > In essence, the proposal follows our current stability guarantees
> > while
> > > > > updating the guarantees for @PublicEvolving that we are already
> > > providing
> > > > > w/o having stated them. Moreover, this FLIP proposes some
> guidelines
> > > for
> > > > > determining the stability guarantees for an API object, how to
> evolve
> > > > them
> > > > > and some additional requirements for the return and parameter types
> > of
> > > > > methods.
> > > > >
> > > > > All in all, I hope that this proposal is more reflecting our
> current
> > > > > understanding of stability guarantees than being controversial. One
> > of
> > > > the
> > > > > outcomes of this FLIP should be to properly document these
> guarantees
> > > so
> > > > > that it is easily discoverable and understandable for our users.
> > > > Moreover,
> > > > > I hope that we can provide more stable APIs our users can rely and
> > > build
> > > > > upon.
> > > > >
> > > > > There will be a follow-up FLIP discussing the problem of how to
> make
> > > sure
> > > > > that APIs become stable over time.
> > > > >
> > > > > Looking forward to your feedback.
> > > > >
> > > > > [1] https://cwiki.apache.org/confluence/x/IJeqCw
> > > > > [2]
> https://lists.apache.org/thread/5jm25783oq5svyk7rr8g1gly2ooxqhjr
> > > > > [3]
> https://lists.apache.org/thread/kzhfc3t6omzo2kyo8zj9yxoh8twq5fzr
> > > > >
> > > > > Cheers,
> > > > > Till
> > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] FLIP-196: Source API stability guarantees

[Till Rohrmann <tr...@apache.org>]

>That's still a much weaker requirement, though, as classes can just be
left unannotated, which is why I prefer annotating all classes regardless
of location.

Would it be enough to say that for example all classes in the module
flink-java have to be annotated? What I would like to avoid is having to
annotate all classes in some internal module like flink-rpc.

> How would you handle e.g. extending an existing Public interface with a
new method in this case, though? You'd be forced to immediately make the
new method Public as well, or place it somewhere else entirely, which leads
to unfavorable design. I don't think we should disallow extending classes
with methods of a weaker stability.

Extending existing classes will only be possible if you can provide a
default implementation. If the user needs to do something, then it is not
compatible and needs to be handled differently (e.g. by offering a new
experimental interface that one can use). If we don't enforce this, then I
don't see how we can provide source stability guarantees.

Cheers,
Till

On Fri, Dec 3, 2021 at 5:22 PM Ingo Bürk <in...@ververica.com> wrote:

> Hi Till,
>
> > Personally, I'd be fine to say that in API modules (tbd what this is
> > (probably transitive closure of all APIs)) we require every class to be
> > annotated.
>
> At least we'll then need the reverse rule: no classes outside *.api.*
> packages CAN have an API annotation (other than Internal), of course with
> many existing violations that need to be accapted.
>
> That's still a much weaker requirement, though, as classes can just be left
> unannotated, which is why I prefer annotating all classes regardless of
> location.
>
> > If we have cases that violate the guideline, then I think we either have
> to
> > remove these methods
>
> How would you handle e.g. extending an existing Public interface with a new
> method in this case, though? You'd be forced to immediately make the new
> method Public as well, or place it somewhere else entirely, which leads to
> unfavorable design. I don't think we should disallow extending classes with
> methods of a weaker stability.
>
>
> Best
> Ingo
>
> On Fri, Dec 3, 2021 at 4:53 PM Till Rohrmann <tr...@apache.org> wrote:
>
> > Hi Ingo, thanks a lot for your thoughts and the work you've spent on this
> > topic already.
> >
> > Personally, I'd be fine to say that in API modules (tbd what this is
> > (probably transitive closure of all APIs)) we require every class to be
> > annotated.
> >
> > For sake of clarity and having clear rules, this should then also apply
> to
> > nested types. As you've said this would have some additional benefits
> when
> > doing refactorings and seems to be actually required by japicmp.
> >
> > >This seems to be quite incompatible with the current interpretation in
> the
> > code base, and it would prevent valid (and in-use) use cases like marking
> > e.g. a single method experimental (or even internal) in an otherwise
> public
> > (evolving) API.
> >
> > If we have cases that violate the guideline, then I think we either have
> to
> > remove these methods or adjust their stability guarantees (time of
> > reckoning ;-). Otherwise, we won't be able to give stability guarantees,
> I
> > fear.
> >
> > Cheers,
> > Till
> >
> > On Fri, Dec 3, 2021 at 4:23 PM Ingo Bürk <in...@ververica.com> wrote:
> >
> > > Hi Till,
> > >
> > > Thanks for starting this discussion, and as you noticed, I qutie care
> for
> > > it as well. :-)
> > >
> > > When implementing the ArchUnit rules, I noticed that project-wide,
> > > different teams / modules use and understand the API annotations in
> > > different ways. Therefore, I think it is important to both get clarity
> on
> > > the meaning of these annotations (= your FLIP), but also on how to
> > actually
> > > use them.
> > >
> > > One thing I would like to bring up for discussion is requiring an API
> > > annotation on every class. This seems to be a controversial suggestion,
> > > though personally I think it would allow our tooling do its job best.
> An
> > > obvious downside is "it's annoying", but so is the current requirement
> of
> > > having JavaDocs on every class (including tests). If this were to ever
> be
> > > considered, this should be implemented (also) in Checkstyle, though, to
> > > have it as immediate IDE feedback, which IMO would also mostly
> eliminate
> > > this downside. The other downside is that it encourages throwing
> > > Public(Evolving) around, but that would really only make an existing
> > > invisible problem actually visible; ultimately, I think this is
> something
> > > that committers just need to consider.
> > > It would be nice to only require this in *.api.* packages, but that's
> > just
> > > not the world we live in, nor will it be anytime soon; however, it
> would
> > > allow us to make sure that new non-internal APIs are only added in
> > *.api.*
> > > packages, or that no public APIs are added in *.internal.* packages.
> > >
> > > Another aspect that came up both when implementing the rules and again
> > in a
> > > discussion is whether (or when) nested types (classes, interfaces –
> > > not members) need to be annotated explicitly. My understanding[1] is
> that
> > > this is actually needed for japicmp to work correctly. Besides that, I
> > > think doing so adds benefits such as making the breaking change visible
> > > when promoting a nested type to a top-level type, which would likely go
> > > unquestioned if the type is not annotated but "inherited" its API
> > > stability, losing it in the process.
> > >
> > > Now to actually comment on your FLIP:
> > >
> > > > In order to determine the stability guarantee a given class/interface
> > can
> > > provide, we have to take a look at all methods a user has to implement
> > and
> > > use for using the given class/interface. The weakest guarantee of these
> > > methods specifies the guarantee we can give for the containing
> > > class/interface.
> > >
> > > This seems to be quite incompatible with the current interpretation in
> > the
> > > code base, and it would prevent valid (and in-use) use cases like
> marking
> > > e.g. a single method experimental (or even internal) in an otherwise
> > public
> > > (evolving) API.
> > >
> > > [1] https://github.com/apache/flink/pull/17133#issuecomment-925738694
> > >
> > >
> > > Best
> > > Ingo
> > >
> > > On Thu, Dec 2, 2021 at 3:48 PM Till Rohrmann <tr...@apache.org>
> > wrote:
> > >
> > > > Hi everyone,
> > > >
> > > > I would like to start a discussion about what kind of API stability
> > > > guarantees we want to provide to our users. The Flink community
> already
> > > > agreed on some stability guarantees, but these guarantees were only
> > > > communicated via the ML and not properly documented [2]. Moreover,
> I've
> > > > seen more and more complaints about breaking changes (some rightful
> and
> > > > others not) on the ML recently [3] because we rarely mark our APIs as
> > > > stable. This motivated this FLIP.
> > > >
> > > > The proposal first concentrates on source API stability guarantees.
> > > Binary
> > > > stability guarantees are explicitly left out for a follow-up
> > discussion.
> > > >
> > > > In essence, the proposal follows our current stability guarantees
> while
> > > > updating the guarantees for @PublicEvolving that we are already
> > providing
> > > > w/o having stated them. Moreover, this FLIP proposes some guidelines
> > for
> > > > determining the stability guarantees for an API object, how to evolve
> > > them
> > > > and some additional requirements for the return and parameter types
> of
> > > > methods.
> > > >
> > > > All in all, I hope that this proposal is more reflecting our current
> > > > understanding of stability guarantees than being controversial. One
> of
> > > the
> > > > outcomes of this FLIP should be to properly document these guarantees
> > so
> > > > that it is easily discoverable and understandable for our users.
> > > Moreover,
> > > > I hope that we can provide more stable APIs our users can rely and
> > build
> > > > upon.
> > > >
> > > > There will be a follow-up FLIP discussing the problem of how to make
> > sure
> > > > that APIs become stable over time.
> > > >
> > > > Looking forward to your feedback.
> > > >
> > > > [1] https://cwiki.apache.org/confluence/x/IJeqCw
> > > > [2] https://lists.apache.org/thread/5jm25783oq5svyk7rr8g1gly2ooxqhjr
> > > > [3] https://lists.apache.org/thread/kzhfc3t6omzo2kyo8zj9yxoh8twq5fzr
> > > >
> > > > Cheers,
> > > > Till
> > > >
> > >
> >
>

Re: [DISCUSS] FLIP-196: Source API stability guarantees

[Ingo Bürk <in...@ververica.com>]

Hi Till,

> Personally, I'd be fine to say that in API modules (tbd what this is
> (probably transitive closure of all APIs)) we require every class to be
> annotated.

At least we'll then need the reverse rule: no classes outside *.api.*
packages CAN have an API annotation (other than Internal), of course with
many existing violations that need to be accapted.

That's still a much weaker requirement, though, as classes can just be left
unannotated, which is why I prefer annotating all classes regardless of
location.

> If we have cases that violate the guideline, then I think we either have
to
> remove these methods

How would you handle e.g. extending an existing Public interface with a new
method in this case, though? You'd be forced to immediately make the new
method Public as well, or place it somewhere else entirely, which leads to
unfavorable design. I don't think we should disallow extending classes with
methods of a weaker stability.


Best
Ingo

On Fri, Dec 3, 2021 at 4:53 PM Till Rohrmann <tr...@apache.org> wrote:

> Hi Ingo, thanks a lot for your thoughts and the work you've spent on this
> topic already.
>
> Personally, I'd be fine to say that in API modules (tbd what this is
> (probably transitive closure of all APIs)) we require every class to be
> annotated.
>
> For sake of clarity and having clear rules, this should then also apply to
> nested types. As you've said this would have some additional benefits when
> doing refactorings and seems to be actually required by japicmp.
>
> >This seems to be quite incompatible with the current interpretation in the
> code base, and it would prevent valid (and in-use) use cases like marking
> e.g. a single method experimental (or even internal) in an otherwise public
> (evolving) API.
>
> If we have cases that violate the guideline, then I think we either have to
> remove these methods or adjust their stability guarantees (time of
> reckoning ;-). Otherwise, we won't be able to give stability guarantees, I
> fear.
>
> Cheers,
> Till
>
> On Fri, Dec 3, 2021 at 4:23 PM Ingo Bürk <in...@ververica.com> wrote:
>
> > Hi Till,
> >
> > Thanks for starting this discussion, and as you noticed, I qutie care for
> > it as well. :-)
> >
> > When implementing the ArchUnit rules, I noticed that project-wide,
> > different teams / modules use and understand the API annotations in
> > different ways. Therefore, I think it is important to both get clarity on
> > the meaning of these annotations (= your FLIP), but also on how to
> actually
> > use them.
> >
> > One thing I would like to bring up for discussion is requiring an API
> > annotation on every class. This seems to be a controversial suggestion,
> > though personally I think it would allow our tooling do its job best. An
> > obvious downside is "it's annoying", but so is the current requirement of
> > having JavaDocs on every class (including tests). If this were to ever be
> > considered, this should be implemented (also) in Checkstyle, though, to
> > have it as immediate IDE feedback, which IMO would also mostly eliminate
> > this downside. The other downside is that it encourages throwing
> > Public(Evolving) around, but that would really only make an existing
> > invisible problem actually visible; ultimately, I think this is something
> > that committers just need to consider.
> > It would be nice to only require this in *.api.* packages, but that's
> just
> > not the world we live in, nor will it be anytime soon; however, it would
> > allow us to make sure that new non-internal APIs are only added in
> *.api.*
> > packages, or that no public APIs are added in *.internal.* packages.
> >
> > Another aspect that came up both when implementing the rules and again
> in a
> > discussion is whether (or when) nested types (classes, interfaces –
> > not members) need to be annotated explicitly. My understanding[1] is that
> > this is actually needed for japicmp to work correctly. Besides that, I
> > think doing so adds benefits such as making the breaking change visible
> > when promoting a nested type to a top-level type, which would likely go
> > unquestioned if the type is not annotated but "inherited" its API
> > stability, losing it in the process.
> >
> > Now to actually comment on your FLIP:
> >
> > > In order to determine the stability guarantee a given class/interface
> can
> > provide, we have to take a look at all methods a user has to implement
> and
> > use for using the given class/interface. The weakest guarantee of these
> > methods specifies the guarantee we can give for the containing
> > class/interface.
> >
> > This seems to be quite incompatible with the current interpretation in
> the
> > code base, and it would prevent valid (and in-use) use cases like marking
> > e.g. a single method experimental (or even internal) in an otherwise
> public
> > (evolving) API.
> >
> > [1] https://github.com/apache/flink/pull/17133#issuecomment-925738694
> >
> >
> > Best
> > Ingo
> >
> > On Thu, Dec 2, 2021 at 3:48 PM Till Rohrmann <tr...@apache.org>
> wrote:
> >
> > > Hi everyone,
> > >
> > > I would like to start a discussion about what kind of API stability
> > > guarantees we want to provide to our users. The Flink community already
> > > agreed on some stability guarantees, but these guarantees were only
> > > communicated via the ML and not properly documented [2]. Moreover, I've
> > > seen more and more complaints about breaking changes (some rightful and
> > > others not) on the ML recently [3] because we rarely mark our APIs as
> > > stable. This motivated this FLIP.
> > >
> > > The proposal first concentrates on source API stability guarantees.
> > Binary
> > > stability guarantees are explicitly left out for a follow-up
> discussion.
> > >
> > > In essence, the proposal follows our current stability guarantees while
> > > updating the guarantees for @PublicEvolving that we are already
> providing
> > > w/o having stated them. Moreover, this FLIP proposes some guidelines
> for
> > > determining the stability guarantees for an API object, how to evolve
> > them
> > > and some additional requirements for the return and parameter types of
> > > methods.
> > >
> > > All in all, I hope that this proposal is more reflecting our current
> > > understanding of stability guarantees than being controversial. One of
> > the
> > > outcomes of this FLIP should be to properly document these guarantees
> so
> > > that it is easily discoverable and understandable for our users.
> > Moreover,
> > > I hope that we can provide more stable APIs our users can rely and
> build
> > > upon.
> > >
> > > There will be a follow-up FLIP discussing the problem of how to make
> sure
> > > that APIs become stable over time.
> > >
> > > Looking forward to your feedback.
> > >
> > > [1] https://cwiki.apache.org/confluence/x/IJeqCw
> > > [2] https://lists.apache.org/thread/5jm25783oq5svyk7rr8g1gly2ooxqhjr
> > > [3] https://lists.apache.org/thread/kzhfc3t6omzo2kyo8zj9yxoh8twq5fzr
> > >
> > > Cheers,
> > > Till
> > >
> >
>

Re: [DISCUSS] FLIP-196: Source API stability guarantees

[Till Rohrmann <tr...@apache.org>]

Hi Ingo, thanks a lot for your thoughts and the work you've spent on this
topic already.

Personally, I'd be fine to say that in API modules (tbd what this is
(probably transitive closure of all APIs)) we require every class to be
annotated.

For sake of clarity and having clear rules, this should then also apply to
nested types. As you've said this would have some additional benefits when
doing refactorings and seems to be actually required by japicmp.

>This seems to be quite incompatible with the current interpretation in the
code base, and it would prevent valid (and in-use) use cases like marking
e.g. a single method experimental (or even internal) in an otherwise public
(evolving) API.

If we have cases that violate the guideline, then I think we either have to
remove these methods or adjust their stability guarantees (time of
reckoning ;-). Otherwise, we won't be able to give stability guarantees, I
fear.

Cheers,
Till

On Fri, Dec 3, 2021 at 4:23 PM Ingo Bürk <in...@ververica.com> wrote:

> Hi Till,
>
> Thanks for starting this discussion, and as you noticed, I qutie care for
> it as well. :-)
>
> When implementing the ArchUnit rules, I noticed that project-wide,
> different teams / modules use and understand the API annotations in
> different ways. Therefore, I think it is important to both get clarity on
> the meaning of these annotations (= your FLIP), but also on how to actually
> use them.
>
> One thing I would like to bring up for discussion is requiring an API
> annotation on every class. This seems to be a controversial suggestion,
> though personally I think it would allow our tooling do its job best. An
> obvious downside is "it's annoying", but so is the current requirement of
> having JavaDocs on every class (including tests). If this were to ever be
> considered, this should be implemented (also) in Checkstyle, though, to
> have it as immediate IDE feedback, which IMO would also mostly eliminate
> this downside. The other downside is that it encourages throwing
> Public(Evolving) around, but that would really only make an existing
> invisible problem actually visible; ultimately, I think this is something
> that committers just need to consider.
> It would be nice to only require this in *.api.* packages, but that's just
> not the world we live in, nor will it be anytime soon; however, it would
> allow us to make sure that new non-internal APIs are only added in *.api.*
> packages, or that no public APIs are added in *.internal.* packages.
>
> Another aspect that came up both when implementing the rules and again in a
> discussion is whether (or when) nested types (classes, interfaces –
> not members) need to be annotated explicitly. My understanding[1] is that
> this is actually needed for japicmp to work correctly. Besides that, I
> think doing so adds benefits such as making the breaking change visible
> when promoting a nested type to a top-level type, which would likely go
> unquestioned if the type is not annotated but "inherited" its API
> stability, losing it in the process.
>
> Now to actually comment on your FLIP:
>
> > In order to determine the stability guarantee a given class/interface can
> provide, we have to take a look at all methods a user has to implement and
> use for using the given class/interface. The weakest guarantee of these
> methods specifies the guarantee we can give for the containing
> class/interface.
>
> This seems to be quite incompatible with the current interpretation in the
> code base, and it would prevent valid (and in-use) use cases like marking
> e.g. a single method experimental (or even internal) in an otherwise public
> (evolving) API.
>
> [1] https://github.com/apache/flink/pull/17133#issuecomment-925738694
>
>
> Best
> Ingo
>
> On Thu, Dec 2, 2021 at 3:48 PM Till Rohrmann <tr...@apache.org> wrote:
>
> > Hi everyone,
> >
> > I would like to start a discussion about what kind of API stability
> > guarantees we want to provide to our users. The Flink community already
> > agreed on some stability guarantees, but these guarantees were only
> > communicated via the ML and not properly documented [2]. Moreover, I've
> > seen more and more complaints about breaking changes (some rightful and
> > others not) on the ML recently [3] because we rarely mark our APIs as
> > stable. This motivated this FLIP.
> >
> > The proposal first concentrates on source API stability guarantees.
> Binary
> > stability guarantees are explicitly left out for a follow-up discussion.
> >
> > In essence, the proposal follows our current stability guarantees while
> > updating the guarantees for @PublicEvolving that we are already providing
> > w/o having stated them. Moreover, this FLIP proposes some guidelines for
> > determining the stability guarantees for an API object, how to evolve
> them
> > and some additional requirements for the return and parameter types of
> > methods.
> >
> > All in all, I hope that this proposal is more reflecting our current
> > understanding of stability guarantees than being controversial. One of
> the
> > outcomes of this FLIP should be to properly document these guarantees so
> > that it is easily discoverable and understandable for our users.
> Moreover,
> > I hope that we can provide more stable APIs our users can rely and build
> > upon.
> >
> > There will be a follow-up FLIP discussing the problem of how to make sure
> > that APIs become stable over time.
> >
> > Looking forward to your feedback.
> >
> > [1] https://cwiki.apache.org/confluence/x/IJeqCw
> > [2] https://lists.apache.org/thread/5jm25783oq5svyk7rr8g1gly2ooxqhjr
> > [3] https://lists.apache.org/thread/kzhfc3t6omzo2kyo8zj9yxoh8twq5fzr
> >
> > Cheers,
> > Till
> >
>

Re: [DISCUSS] FLIP-196: Source API stability guarantees

[Ingo Bürk <in...@ververica.com>]

Hi Till,

Thanks for starting this discussion, and as you noticed, I qutie care for
it as well. :-)

When implementing the ArchUnit rules, I noticed that project-wide,
different teams / modules use and understand the API annotations in
different ways. Therefore, I think it is important to both get clarity on
the meaning of these annotations (= your FLIP), but also on how to actually
use them.

One thing I would like to bring up for discussion is requiring an API
annotation on every class. This seems to be a controversial suggestion,
though personally I think it would allow our tooling do its job best. An
obvious downside is "it's annoying", but so is the current requirement of
having JavaDocs on every class (including tests). If this were to ever be
considered, this should be implemented (also) in Checkstyle, though, to
have it as immediate IDE feedback, which IMO would also mostly eliminate
this downside. The other downside is that it encourages throwing
Public(Evolving) around, but that would really only make an existing
invisible problem actually visible; ultimately, I think this is something
that committers just need to consider.
It would be nice to only require this in *.api.* packages, but that's just
not the world we live in, nor will it be anytime soon; however, it would
allow us to make sure that new non-internal APIs are only added in *.api.*
packages, or that no public APIs are added in *.internal.* packages.

Another aspect that came up both when implementing the rules and again in a
discussion is whether (or when) nested types (classes, interfaces –
not members) need to be annotated explicitly. My understanding[1] is that
this is actually needed for japicmp to work correctly. Besides that, I
think doing so adds benefits such as making the breaking change visible
when promoting a nested type to a top-level type, which would likely go
unquestioned if the type is not annotated but "inherited" its API
stability, losing it in the process.

Now to actually comment on your FLIP:

> In order to determine the stability guarantee a given class/interface can
provide, we have to take a look at all methods a user has to implement and
use for using the given class/interface. The weakest guarantee of these
methods specifies the guarantee we can give for the containing
class/interface.

This seems to be quite incompatible with the current interpretation in the
code base, and it would prevent valid (and in-use) use cases like marking
e.g. a single method experimental (or even internal) in an otherwise public
(evolving) API.

[1] https://github.com/apache/flink/pull/17133#issuecomment-925738694


Best
Ingo

On Thu, Dec 2, 2021 at 3:48 PM Till Rohrmann <tr...@apache.org> wrote:

> Hi everyone,
>
> I would like to start a discussion about what kind of API stability
> guarantees we want to provide to our users. The Flink community already
> agreed on some stability guarantees, but these guarantees were only
> communicated via the ML and not properly documented [2]. Moreover, I've
> seen more and more complaints about breaking changes (some rightful and
> others not) on the ML recently [3] because we rarely mark our APIs as
> stable. This motivated this FLIP.
>
> The proposal first concentrates on source API stability guarantees. Binary
> stability guarantees are explicitly left out for a follow-up discussion.
>
> In essence, the proposal follows our current stability guarantees while
> updating the guarantees for @PublicEvolving that we are already providing
> w/o having stated them. Moreover, this FLIP proposes some guidelines for
> determining the stability guarantees for an API object, how to evolve them
> and some additional requirements for the return and parameter types of
> methods.
>
> All in all, I hope that this proposal is more reflecting our current
> understanding of stability guarantees than being controversial. One of the
> outcomes of this FLIP should be to properly document these guarantees so
> that it is easily discoverable and understandable for our users. Moreover,
> I hope that we can provide more stable APIs our users can rely and build
> upon.
>
> There will be a follow-up FLIP discussing the problem of how to make sure
> that APIs become stable over time.
>
> Looking forward to your feedback.
>
> [1] https://cwiki.apache.org/confluence/x/IJeqCw
> [2] https://lists.apache.org/thread/5jm25783oq5svyk7rr8g1gly2ooxqhjr
> [3] https://lists.apache.org/thread/kzhfc3t6omzo2kyo8zj9yxoh8twq5fzr
>
> Cheers,
> Till
>