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 <i...@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 <trohrm...@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 <i...@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 <trohrm...@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 <i...@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 <trohrm...@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
> > > > > >
> > > > >
> > > >
> > >
> >
>
