+1 to improve and eliminate the possibility of incompatible changes to user-facing classes for patch releases.
The PR for 1.10.1 release [1] is already updated, please kindly let me know if any further comments. I plan to merge the PR and officially announce the release in ~ half an hour if no objections. Thanks. Best Regards, Yu On Thu, 14 May 2020 at 00:30, Thomas Weise <t...@apache.org> wrote: > 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 > > >>>>>>>> > > >>> > > > > >
