Hi all, Thanks for sharing your opinions!
I'd like to draw a conclusion. I had an offline discussion with @Hangxiang Yu and @Junrui Lee today, and we agreed that option 2 could make the package cleaner, thus it is a better way to go. We will follow option 2, and the deprecation will be discussed later in 2.0. If there is any objection, please let me know, thanks. Best, Zakelly On Wed, Feb 28, 2024 at 7:49 PM Xintong Song <tonysong...@gmail.com> wrote: > Personally, I'd be in favor of option 2. And based on the fact that > migrating from the deprecated CheckpointingMode to the new one takes barely > any effort (simply re-import the class), I'd be fine with removing the > deprecated class in 2.0. > > But I'd also be fine with the other options. > > Either way, agree that we should not block 1.19 on this. > > Best, > > Xintong > > > > On Wed, Feb 28, 2024 at 6:16 PM Junrui Lee <jrlee....@gmail.com> wrote: > > > Hi Zakelly, > > > > +1 for option 1. I prefer to minimize unnecessary additional development > > and discussions due to internal code relocations and to avoid imposing > > migration costs on users. > > > > Best regards, > > Junrui > > > > Zakelly Lan <zakelly....@gmail.com> 于2024年2月28日周三 14:46写道： > > > > > Hi Lincoln, > > > > > > Given that we have finished the testing for 1.19, I agree it is better > > not > > > merge this into 1.19. Thanks for RMs' attention! > > > > > > Hi Chesney and Junrui, > > > > > > Thanks for your advice. My original intention is to move the class as > > well > > > as change the package to make it clean. But it involves much more > effort. > > > Here are several options we have: > > > > > > 1. Move CheckpointingMode to flink-core and keep the same package. > No > > > more deprecation and API changes. But it will leave a > > > 'org.apache.flink.streaming.api' package in flink-core. > > > 2. Introduce new CheckpointingMode in package > > > 'org.apache.flink.core.execution' and deprecate the old one. > Deprecate > > > the > > > corresponding getter/setter of 'CheckpointConfig' and introduce new > > ones > > > with a similar but different name (e.g. set/getCheckpointMode). We > > will > > > discuss the removal of those deprecation later in 2.x. > > > 3. Based on 1, move CheckpointingMode to package > > > 'org.apache.flink.core.execution' in 2.0. This is a breaking change > > that > > > needs more discussion. > > > > > > Both ways work. I'm slightly inclined to option 1, or option 3 if we > all > > > agree, since the new getter/setter may also bring in confusions thus we > > > cannot make the API purely clean. WDYT? > > > > > > > > > Best, > > > Zakelly > > > > > > On Wed, Feb 28, 2024 at 10:14 AM Junrui Lee <jrlee....@gmail.com> > wrote: > > > > > > > Hi Zakelly, > > > > > > > > I agree with Chesnay's response. I would suggest that during the > > process > > > of > > > > moving CheckpointingMode from the flink-streaming-java module to the > > > > flink-core module, we should keep the package name unchanged. This > > > approach > > > > would be completely transparent to users. In fact, this practice > should > > > be > > > > applicable to many of our desired moves from flink-streaming-java to > > > > higher-level modules, such as flink-runtime and flink-core. > > > > > > > > Best, > > > > Junrui > > > > > > > > Chesnay Schepler <ches...@apache.org> 于2024年2月28日周三 05:18写道： > > > > > > > > > Moving classes (== keep the same package) to a module higher up in > > the > > > > > dependency tree should not be a breaking change and can imo be done > > > > > anytime without any risk to users. > > > > > > > > > > On 27/02/2024 17:01, Lincoln Lee wrote: > > > > > > Hi Zakelly, > > > > > > > > > > > > Thanks for letting us 1.19 RMs know about this! > > > > > > > > > > > > This change has been discussed during today's release sync > meeting, > > > we > > > > > > suggest not merge it into 1.19. > > > > > > We can continue discussing the removal in 2.x separately. > > > > > > > > > > > > Best, > > > > > > Lincoln Lee > > > > > > > > > > > > > > > > > > Hangxiang Yu <master...@gmail.com> 于2024年2月27日周二 11:28写道： > > > > > > > > > > > >> Hi, Zakelly. > > > > > >> Thanks for driving this. > > > > > >> Moving this class to flink-core makes sense to me which could > make > > > the > > > > > code > > > > > >> path and configs clearer. > > > > > >> It's marked as @Public from 1.0 and 1.20 should be the next > > > long-term > > > > > >> version, so 1.19 should have been a suitable version to do it. > > > > > >> And also look forward to thoughts of other developers/RMs since > > 1.19 > > > > is > > > > > >> currently under a feature freeze status. > > > > > >> > > > > > >> On Mon, Feb 26, 2024 at 6:42 PM Zakelly Lan < > > zakelly....@gmail.com> > > > > > wrote: > > > > > >> > > > > > >>> Hi devs, > > > > > >>> > > > > > >>> When working on the FLIP-406[1], I realized that moving all > > options > > > > of > > > > > >>> ExecutionCheckpointingOptions(flink-streaming-java) to > > > > > >>> CheckpointingOptions(flink-core) depends on relocating the > > > > > >>> enum CheckpointingMode(flink-streaming-java) to flink-core > > module. > > > > > >> However, > > > > > >>> the CheckpointingMode is annotated as @Public and used by > > > datastream > > > > > api > > > > > >>> like 'CheckpointConfig#setCheckpointingMode'. So I'd like to > > start > > > a > > > > > >>> discussion on moving the CheckpointingMode to flink-core. It is > > in > > > a > > > > > >> little > > > > > >>> bit of a hurry if we want the old enum to be entirely removed > in > > > > Flink > > > > > >> 2.x > > > > > >>> series, since the deprecation should be shipped in the upcoming > > > Flink > > > > > >> 1.19. > > > > > >>> I suggest not creating a dedicated FLIP and treating this as a > > > > sub-task > > > > > >> of > > > > > >>> FLIP-406. > > > > > >>> > > > > > >>> I prepared a minimal change of providing new APIs and > deprecating > > > the > > > > > old > > > > > >>> ones[2], which could be merged to 1.19 if we agree to do so. > > > > > >>> > > > > > >>> Looking forward to your thoughts! Also cc RMs of 1.19 about > this. > > > > > >>> > > > > > >>> [1] > > > > > >>> > > > > > >> > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=284789560 > > > > > >>> [2] > > > > > >>> > > > > > >>> > > > > > >> > > > > > > > > > > > > > > > https://github.com/apache/flink/commit/9bdd237d0322df8853f1b9e6ae658f77b9175237 > > > > > >>> Best, > > > > > >>> Zakelly > > > > > >>> > > > > > >> > > > > > >> -- > > > > > >> Best, > > > > > >> Hangxiang. > > > > > >> > > > > > > > > > > > > > > > > > > > >
