For release branches, it would be good to not exclude PublicEvolving from checking, by default [1].
There may be instances where it is necessary to make exceptions to fix a critical bug, but those could be whitelisted. Similarly, it might be good to blacklist dependency changes for patch releases, by default. [1] https://github.com/apache/flink/blob/e1e7d7f7ecc080c850a264021bf1b20e3d27d373/pom.xml#L1959 On Wed, May 13, 2020 at 9:16 AM Chesnay Schepler <ches...@apache.org> wrote: > We are running japicmp in CI; we just don't check anything that is > PublicEvolving. > > On 13/05/2020 18:04, Thomas Weise wrote: > > +1 for the release note addition > > > > Historically Flink X.Y.0 releases have been prone to issues like this. > And > > I suspect that many users would prefer to upgrade to X.Y.1 due to other > > bugs that need to be shaken out. > > > > For the future, it would be great to eliminate the possibility of > > incompatible changes to user-facing classes for patch releases though. > > Running japicmp as part of CI might be a good option. > > > > Thanks, > > Thomas > > > > On Wed, May 13, 2020 at 7:54 AM Seth Wiesman <sjwies...@gmail.com> > wrote: > > > >> +1 Ufuk's pragmatic solution to update the release notes with a public > >> notice, I have commented on the release notes PR. > >> > >> https://github.com/apache/flink-web/pull/330 > >> > >> Seth > >> > >> On Wed, May 13, 2020 at 8:30 AM Chesnay Schepler <ches...@apache.org> > >> wrote: > >> > >>> IIRC @PublicEvolving had no /real/ guarantees, from the 1.0.0 > >> announcement: > >>> /"Flink 1.0.0 introduces interface stability annotations for API > classes > >>> and methods. Interfaces defined as //|@Public|//are guaranteed to > remain > >>> stable across all releases of the 1.x series. The > >>> //|@PublicEvolving|//annotation marks API features that may be subject > >>> to change in future versions."/ > >>> > >>> 1) We don't have a process for promoting things to @Public because we > >>> have actually never done that. Based on our current rules, it is an > >>> addition to the public API (technically even more so than most FLIPs), > >>> and hence should go through a FLIP and vote. > >>> Ideally we evaluate existing @PublicEvolving API's for promotion on > each > >>> release. > >>> > >>> 2) Yes, I think this is an excellent idea; as Ufuk said users should > not > >>> be worried about upgrading to the next minor version. Not sure how easy > >>> this is to implement; but it basically means either modifying or adding > >>> new new japicmp execution when forking the release-X.Y branches. > >>> > >>> On 13/05/2020 15:19, Till Rohrmann wrote: > >>>> A small addendum: The project currently enforces only binary > >>> compatibility > >>>> for classes which are annotated with @Public. The StreamingFileSink is > >>>> annotated with @PublicEvolving. > >>>> > >>>> I am not sure whether the community properly defined what kind of > >>>> stability/compatibility guarantees we provide for @PublicEvolving > >>> classes. > >>>> We can derive two follow-up discussions from this: > >>>> > >>>> 1) Should the StreamingFileSink be made @Public? Should > >>>> other @PublicEvolving classes be promoted? What is the process here? > >>>> > >>>> 2) Should we enforce binary compatibility of @PublicEvolving classes > >>>> between bug fix releases and allow breaking changes between > minor/major > >>>> releases? > >>>> > >>>> Cheers, > >>>> Till > >>>> > >>>> On Wed, May 13, 2020 at 2:30 PM Ufuk Celebi <u...@apache.org> wrote: > >>>> > >>>>> Thanks for the analysis Till. > >>>>> > >>>>> 1/ I think breaking binary compatibility is acceptable between minor > >>>>> releases (1.10 -> 1.11) as you suggested since the API is marked > >>>>> as @PublicEvolving. > >>>>> > >>>>> 2/ I'm quite torn about how to proceed with the 1.10.x patch release, > >>> but > >>>>> slightly leaning towards the "pragmatic" solution of keeping the > >> change > >>> and > >>>>> adding a big warning to the 1.10.1 release announcement*. What do > >> others > >>>>> think? If we do want to revert the change and follow up with a 1.10.2 > >>>>> release we should do it _before_ we announce 1.10.1 publicly. Doing > it > >>>>> after 1.10.1 has been announced would only cause more friction for > >>> users. > >>>>> – Ufuk > >>>>> > >>>>> *I hope we don't lose user trust with this. I really would like users > >>> to be > >>>>> able to upgrade to the latest patch release without hesitation. > >>>>> > >>>>> On Wed, May 13, 2020 at 11:45 AM Till Rohrmann <trohrm...@apache.org > > > >>>>> wrote: > >>>>> > >>>>>> Thanks for reporting this issue Seth. This is indeed a big problem. > >>>>>> > >>>>>> I looked into the problem and it seems we have the following > >> situation: > >>>>>> # 1.9.x --> 1.10.0 > >>>>>> > >>>>>> There is an API breaking change between 1.9.x and 1.10 due > >> FLINK-13864 > >>>>>> because it introduced another generic parameter. I expected only few > >>>>> people > >>>>>> will be affected by this because one would have to store the builder > >>> in a > >>>>>> variable. > >>>>>> > >>>>>> 1.9.x is binary compatible with 1.10.0. > >>>>>> > >>>>>> # 1.10.0 -> 1.10.1 > >>>>>> > >>>>>> There is no API breaking change between 1.10.0 and 1.10.1. > >>>>>> > >>>>>> I changed the return type of the StreamingFileSink.forRowFormat and > >>>>>> .forBulkFormat to a subtype of StreamingFileSink.BulkFormatBuilder > in > >>>>>> FLINK-16684. This causes the java.lang.NoSuchMethodError and the > >> binary > >>>>>> incompatibility between 1.10.0 and 1.10.1. This is should not happen > >>> for > >>>>>> bug fix releases. > >>>>>> > >>>>>> W/o FLINK-16684, the StreamingFileSink builders cannot be used with > >>>>> Flink's > >>>>>> Scala API. > >>>>>> > >>>>>> # Options > >>>>>> > >>>>>> I think we should keep the fix for 1.11.0 and add a release note > that > >>> we > >>>>>> are violating binary compatibility between 1.10 and 1.11. > >>>>>> > >>>>>> Now the question is what to do with the 1.10.x branch: > >>>>>> > >>>>>> a) We can revert the change to re-establish binary compatibility > >>> between > >>>>>> 1.9.x, 1.10.0 and 1.10.2 but not between 1.10.1 and 1.10.2. This > >> would > >>>>> also > >>>>>> imply that we cannot use the StreamingFileSink builders with the > >> Scala > >>>>> API. > >>>>>> b) Keep the change and add a release note that our users need to > >>>>> recompile > >>>>>> their jobs. This would allow Flink 1.10.x with x >= 1 users to use > >>>>>> StreamingFileSink builders with the Scala API. > >>>>>> > >>>>>> I am not entirely sure which need weighs more: binary compatibility > >>>>> between > >>>>>> bug fix releases or a working API (small subset of it). Another > >> aspect > >>> to > >>>>>> consider is how many people will migrate from 1.10.0 to 1.10.1 > >> compared > >>>>> to > >>>>>> 1.y to 1.10.1 with y <= 9. If the former is very small then one > might > >>>>> make > >>>>>> a case for keeping the change. > >>>>>> > >>>>>> What do the others think? > >>>>>> > >>>>>> https://issues.apache.org/jira/browse/FLINK-13864 > >>>>>> https://issues.apache.org/jira/browse/FLINK-16684 > >>>>>> > >>>>>> Cheers, > >>>>>> Till > >>>>>> > >>>>>> On Tue, May 12, 2020 at 7:26 PM Thomas Weise <t...@apache.org> > wrote: > >>>>>> > >>>>>>> We also noticed that and had to make an adjustment downstream. > >>>>>>> > >>>>>>> It would be good to mention this in the release notes (if that's > not > >>>>>>> already the case). > >>>>>>> > >>>>>>> Thomas > >>>>>>> > >>>>>>> > >>>>>>> On Tue, May 12, 2020 at 10:06 AM Seth Wiesman <sjwies...@gmail.com > > > >>>>>> wrote: > >>>>>>>> Hi Everyone, > >>>>>>>> > >>>>>>>> I realize I'm about 7 hours late but I just realized there is a > >>>>>> breaking > >>>>>>>> API change in 1.10.1. FLINK-16684 changes the API of the streaming > >>>>> file > >>>>>>>> sink in a binary incompatible way. Since the release has been > >>>>> approved > >>>>>>> I'm > >>>>>>>> not sure the correct course of action but I wanted to bring this > to > >>>>> the > >>>>>>>> communities attention. > >>>>>>>> > >>>>>>>> Seth > >>>>>>>> > >>>>>>>> https://issues.apache.org/jira/browse/FLINK-16684 > >>>>>>>> > >>> > >
