The discuss thread for tightening the stability guarantees for @PublicEvolving classes can be found here [1]
[1] https://lists.apache.org/thread.html/rb0d0f887b291a490ed3773352c90ddf5f11e3d882dc501e3b8cf0ed0%40%3Cdev.flink.apache.org%3E Cheers, Till On Thu, May 14, 2020 at 10:13 AM Till Rohrmann <trohrm...@apache.org> wrote: > I agree with the pragmatic solution. > > Concerning the stability guarantees I will start a separate discussion > thread whether to provide stricter guarantees or not. > > Cheers, > Till > > On Wed, May 13, 2020 at 10:13 PM Thomas Weise <t...@apache.org> wrote: > >> On Wed, May 13, 2020 at 1:02 PM Piotr Nowojski <pi...@ververica.com> >> wrote: >> >> > Hi, >> > >> > > 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. >> > >> > I guess the whole point of PublicEvolving classes is to allow us for the >> > breaking compatibility changes. >> > >> > >> The question here is whether such changes should occur in *patch* >> releases. >> I suspect most users would prefer stronger guarantees. >> >> Another example of "breaking" change that affects Beam (though class >> OptimizerPlanEnvironment isn't covered by annotation and it only affects >> test code): >> >> https://github.com/apache/beam/pull/11683 >> >> Piotrek >> > >> > > On 13 May 2020, at 18:29, 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 >> > >>>>>>>>>> >> > >>>>> >> > >> >> > >> >> > >> > >> >
