> > I think we have a compile time checks for breaking changes in `@Public` > marked classes/interfaces using japicmp [1].
Nice, thanks for pointing that out, I'll take a closer look at it ;) Best, D. On Tue, Sep 28, 2021 at 4:14 PM Piotr Nowojski <pnowoj...@apache.org> wrote: > > - We don't have any safeguards for stable API breaks. Big +1 for Ingo's > effort with architectural tests [3]. > > I think we have a compile time checks for breaking changes in `@Public` > marked classes/interfaces using japicmp [1]. > > Piotrek > > [1] https://github.com/apache/flink/blob/master/pom.xml#L2014:L2084 > > wt., 28 wrz 2021 o 15:59 David Morávek <d...@apache.org> napisał(a): > > > This is a super interesting topic and there is already a great > discussion. > > Here are few thoughts: > > > > - There is a delicate balance between fast delivery of the new features > and > > API stability. Even though we should be careful with breaking evolving > > interfaces, it shouldn't stop us from making fast progress / iterate on > > features. > > - There are two camps of users. One camp prefers more frequent releases / > > new features (early adopters) and second that prefer less frequent stable > > releases. There was already a great discussion about this at Flink 1.14 > > thread [1]. > > - We're already trying to be explicit about which API's may break via > > annotations and the feature radar [2]. Stability annotations are a well > > known concept used by many projects. I think we still can go bit further > > here and aim for an IDE support (for example usages of guava > @Experimental > > interfaces get highlighted, raising more awareness about potential > issues). > > I'm not sure how this IDE integration works though. > > - We don't have any safeguards for stable API breaks. Big +1 for Ingo's > > effort with architectural tests [3]. > > > > [1] > > > > > https://lists.apache.org/thread.html/r76e1cdba577c6f4d6c86b23fdaeb53c4e3744c20d0b3e850fc2e14a7%40%3Cdev.flink.apache.org%3E > > [2] https://flink.apache.org/roadmap.html > > [3] https://issues.apache.org/jira/browse/FLINK-24138 > > > > Best, > > D. > > > > On Tue, Sep 28, 2021 at 3:49 PM Leonard Xu <xbjt...@gmail.com> wrote: > > > > > Thanks Piotr for the kindly reply, what confused me is that > > > `SourceReaderContext` was marked @Public when it was born in flink > 1.11, > > > and then it was corrected to @PublicEvolving in 1.11 -_-, and finally > it > > > was changed to @Public again... > > > > > > As Flink and Flink ecosystem(Flink CDC connectors) developer, I think > > what > > > we're discussing is meaningful and I’d like to help improve those > Public > > > API check for those changes. > > > Chesnay’s tips is a good idea that maybe we can use the tool like > japicmp > > > to do the check for every PR in CI phase. > > > > > > Best, > > > Leonard > > > > > > > > > > 在 2021年9月28日，21:15，Piotr Nowojski <pnowoj...@apache.org> 写道： > > > > > > > > Hi Leonard, > > > > > > > > Sorry for this causing you troubles, however that change in the > return > > > type > > > > was done while this class still has been marked as > > `@PublicEvolving`[1]. > > > As > > > > of 1.13.x `SourceReaderContext` was `@PublicEvolving` and it was > marked > > > as > > > > `@Public` only starting from Flink 1.14.0 [2]. Probably what confused > > you > > > > was that both of those changes (changing the return type and making > it > > > > `@Public`) happened in the same release. > > > > > > > > However, those changes (`SourceReaderContext` and > > `ResolvedCatalogTable`) > > > > should have been clearly mentioned in the release notes with an > upgrade > > > > guide. > > > > > > > > Best, Piotrek > > > > > > > > [1] > > > > > > > > > > https://github.com/apache/flink/commit/7f3636f6b4f8bac415a7db85917ad849636bd730#diff-a247a24ccd1afc07c5d690a8a58b1f6584329925fdf0d7dc89361b90d621b7f2R31 > > > > [2] https://issues.apache.org/jira/browse/FLINK-22357 > > > > > > > > wt., 28 wrz 2021 o 14:49 Leonard Xu <xbjt...@gmail.com> napisał(a): > > > > > > > >>>> > > > >>>> Not sure if this will happen in 1.15 already. We will needed > > automated > > > >>>> compatibility tests and a well-defined list of stable API. > > > >> > > > >>> We are > > > >>> trying to provide forward compatibility: applications using > `@Public` > > > >> APIs > > > >>> compiled against Flink 1.12.x, should work fine in Flink 1.13.x > > > >> > > > >> Unfortunately, I also meet forward compatibility issue, when I do > the > > > >> release 1.14 check, I try to use mysql-cdc connector[1] which > compiled > > > >> against 1.13.1in SQL Client, but it can not work in flink 1.14.0 > > > cluster, > > > >> it failed due to the metric API compatibility broken. > > > >> > > > >> @Public > > > >> public interface SourceReaderContext { > > > >> > > > >> MetricGroup metricGroup(); > > > >> > > > >> > > > >> @Public > > > >> public interface SourceReaderContext { > > > >> > > > >> SourceReaderMetricGroup metricGroup(); > > > >> > > > >> > > > >> Shouldn't we mark it as @Deprecated and then delete it util 2.0.0 > > > version > > > >> for @Public API as the our community rule [2] described? At least we > > > should > > > >> keep them across server minor versions (<major>.<minor>.<patch>). > > > >> > > > >> Although these changes can be tracked to voted FLIPs and it’s not > the > > > >> fault of a few developers, it show us the fact that we didn’t pay > > enough > > > >> attention to back compatibility/forward compatibility. > > > >> > > > >> Best, > > > >> Leonard > > > >> [1] > > > >> > > > > > > https://github.com/ververica/flink-cdc-connectors/tree/master/flink-connector-mysql-cdc > > > >> [2] > > > >> > > https://cwiki.apache.org/confluence/display/FLINK/Stability+Annotations > > > >> > > > >> > > > > > > > > >
