Hi Ferenc, Thank you for creating this FLIP, having some kind of CLI deprecation process would be very useful for developers and users. The FLIP does a good job explaining the deprecation of actions, but there aren't any mentions of command options. I think it would add some value to also include CLI options to this deprecation process, as we have seen them being changed/removed in the past (e.g. -yd and -restoreMode/-claimMode).
Also the FLIP does not mention @PublicEvolving and @Experimental. Will new CLI actions be expected to be annotated as @Public on first release? I don't have any objections against that and I don't want to overcomplicate this FLIP, but it means that any new CLI action will need to stay stable until the next major release. Thanks, Mate Ferenc Csaky <ferenc.cs...@pm.me.invalid> ezt írta (időpont: 2024. júl. 26., P, 10:16): > Hi, > > I prepared an initial FLIP with the already covered details, PTAL > [1]. > > Best, > Ferenc > > [1] > https://docs.google.com/document/d/1heInCDO7WrVKRVDfRzRzrMF9ljS3Tt_JH5TLdKwHLvU/edit?usp=sharing > > > > On Wednesday, 24 July 2024 at 03:39, Xintong Song <tonysong...@gmail.com> > wrote: > > > > > > > Sounds good to me. Thank you, Ferenc. > > > > > > Best, > > > > Xintong > > > > > > > > On Tue, Jul 23, 2024 at 9:42 PM Ferenc Csaky ferenc.cs...@pm.me.invalid > > > > wrote: > > > > > > IIUC, you mean applying the strict mode for the programming APIs > > > > as well? > > > > > > Correct! The original thought that came to my mind that this could > > > be achieved with something similar to the "jdeprscan" Java tool, > > > that checks for deprecated usage. In our case, the presence of > > > @Deprecated and @Public on a used entity would trigger the block > > > in strict mode. > > > > > > Although I definitely agree with that this is more pressing and > > > straightforward for the CLI interfaces, so I am good to only > > > consider those for now. > > > > > > I think I will prepare an initial FLIP with the agreed parts and > > > will post it to discuss further. > > > > > > Thanks, > > > Ferenc > > > > > > On Monday, 22 July 2024 at 03:44, Xintong Song tonysong...@gmail.com > > > wrote: > > > > > > > > As I think about it, most likely orthogonal to the current > > > > > discussion, but this idea seems to be applicable for the > > > > > submitted job JARs as well. The same check could be done for all > > > > > the submitted files and their deps. IDK if that was your original > > > > > thought Xintong or not? > > > > > > > > IIUC, you mean applying the strict mode for the programming APIs as > well? > > > > > > > > I'm not entirely sure about this. The programming APIs are much less > > > > stable > > > > compared to CLI interfaces. There are APIs being deprecated in almost > > > > every > > > > release. My concern is that this may end up with users never turning > the > > > > strict mode on due to too many breaks. > > > > > > > > I'm a bit leaning towards starting with only applying the strict > mode to > > > > CLI interfaces, and see how it goes. But I'm also open to other > opinions. > > > > > > > > Best, > > > > > > > > Xintong > > > > > > > > On Wed, Jul 17, 2024 at 7:59 PM Ferenc Csaky > ferenc.cs...@pm.me.invalid > > > > > > > > wrote: > > > > > > > > > Hi Xintong, Muhammet, > > > > > > > > > > Thank you for your thoughts! > > > > > > > > > > I agree with Xintong regarding 1), and 2). > > > > > > > > > > About making users aware of these compatibility guarantees, > > > > > documentation and CLI print deprecation is what I already aimed > > > > > for when I was merging the "run" and "run-application" actions > > > > > functionality [1], but I missed the CLI page, which is a very good > > > > > point to include. I will address that in a follow-up change. > > > > > > > > > > I also like the idea of a strict, and compatibility mode. I guess > > > > > that should kick in when a "@Public" AND "@Deprecated" annotation > > > > > combination is present on the called CLI action. > > > > > > > > > > With something like this at hand, I think applying the same > > > > > deprecation process and annotations for CLI actions that we already > > > > > have would make sense. Regarding the strict/compatibility strategy > > > > > itself, the straightforward and easy way IMO: > > > > > > > > > > - Strict mode: In case a @Public entity gets @Deprecated, execution > > > > > breaks right away. > > > > > - Compatibility mode: This should allow execution until the code > > > > > exists. > > > > > > > > > > I am not sure if bringing in more complexity would be beneficial, > > > > > as it would also make it harder for the users to understand. > > > > > > > > > > As I think about it, most likely orthogonal to the current > > > > > discussion, but this idea seems to be applicable for the > > > > > submitted job JARs as well. The same check could be done for all > > > > > the submitted files and their deps. IDK if that was your original > > > > > thought Xintong or not? > > > > > > > > > > Best, > > > > > Ferenc > > > > > > > > > > [1] > > > > > > > https://github.com/apache/flink/commit/e56b54db40a2afac420d8d8952707c2644ba633a > > > > > > > > On Tuesday, 9 July 2024 at 06:02, Xintong Song > tonysong...@gmail.com > > > > > wrote: > > > > > > > > > > > I think there are three questions to be anwsered, and here are > my two > > > > > > cents. > > > > > > > > > > > > 1) How do we define the compatibility guarantees for cli > interfaces? > > > > > > > > > > > > I'd be +1 for reusing the existing @Public and @PublicEvolving > > > > > > annotations, > > > > > > as suggested by Ferenc. Having multiple sets of rules in the same > > > > > > project > > > > > > may easily confuse users. > > > > > > > > > > > > 2) What deprecation process is required for cli interfaces? > > > > > > > > > > > > If we are reusing the existing annotations, I think we'd better > to > > > > > > have > > > > > > the > > > > > > same deprecation process as well, for the same reason not to > confuse > > > > > > users. > > > > > > > > > > > > 3) How do we make user aware of the compatibility guarantees and > > > > > > deprecations of cli interfaces? > > > > > > > > > > > > I think there are several things that we can do. > > > > > > - In addition to codes and JavaDocs, we should also display the > > > > > > annotations > > > > > > (Public / PublicEvolving / Experimental Dprecated) in > documentation > > > > > > [1] > > > > > > and > > > > > > cli helps (things you see when execute `<FLINK_HOME>/bin/flink > -h`). > > > > > > > > > > > > - Print a warning message if a deprecated command / argument is > > > > > > used, as > > > > > > suggested by Muhammet. > > > > > > - We can also provide a strict / compatible mode. E.g., we use > strict > > > > > > mode > > > > > > by default, which fails if any deprecated interface is used. In > the > > > > > > error > > > > > > message, we hint user that he/she can manually enable the > compatible > > > > > > mode, > > > > > > which allows deprecated interfaces. This should draw users' > > > > > > attention to > > > > > > the deprecation of the interfaces, while not block the adoption > of a > > > > > > new > > > > > > Flink version with breaking changes. There are probably more > details > > > > > > to > > > > > > be > > > > > > considered, e.g., should we fail immediately once an interface is > > > > > > deprecated in strict mode, how long do we support a deprecated > > > > > > interface > > > > > > in > > > > > > compatible mode, how to properly suggest users about the > compatible > > > > > > mode > > > > > > while not encourage them to always stay in that mode, etc. > > > > > > > > > > > > Best, > > > > > > > > > > > > Xintong > > > > > > > > > > > > [1] > > > > > > > https://nightlies.apache.org/flink/flink-docs-master/docs/deployment/cli/ > > > > > > > > > On Mon, Jul 8, 2024 at 5:54 PM Muhammet Orazov > > > > > > mor+fl...@morazow.com.invalid wrote: > > > > > > > > > > > > > Hey Ferenc, > > > > > > > > > > > > > > Yes correct. My thoughts were based on finding tradeoff between > > > > > > > following fully deprecation process and leaner one for CLIs. > > > > > > > > > > > > > > Since cli are not like APIs, I think users would be aware of > > > > > > > deprecation only when were remove the commands. That is they > > > > > > > try to run their jobs with upgrade and it fails with action > > > > > > > not available. > > > > > > > > > > > > > > So maybe we don't have to follow fully API `@PublicEvolving` > > > > > > > process for this. > > > > > > > > > > > > > > Another maybe user friendly approach would be to inform with > > > > > > > warning that the `run-application` cli action will be dropped, > > > > > > > and suggest new action and migration on the log message. > > > > > > > > > > > > > > Best, > > > > > > > Muhammet > > > > > > > > > > > > > > On 2024-07-04 20:17, Ferenc Csaky wrote: > > > > > > > > > > > > > > > Hi Muhammet, > > > > > > > > > > > > > > > > Thank you for your thoughts! > > > > > > > > > > > > > > > > > After two minor releases, and on next major version bump, > > > > > > > > > we could drop the `run-application` method as suggested > > > > > > > > > on discussion by Xintong. > > > > > > > > > > > > > > > > Here, you describe the deprecation/removal process of a > public > > > > > > > > API by the definition we have in the project now. So if the > same > > > > > > > > applies to a CLI action, why should we not enforce such > behavior > > > > > > > > for those as well? > > > > > > > > > > > > > > > > If following the same process for CLIs make sense, we should > also > > > > > > > > enforce the same compatibility guarantees IMO. > > > > > > > > > > > > > > > > Best, > > > > > > > > Ferenc > > > > > > > > > > > > > > > > On Friday, 28 June 2024 at 09:30, Muhammet Orazov > > > > > > > > mor+fl...@morazow.com.INVALID wrote: > > > > > > > > > > > > > > > > > Hey Ferenc, > > > > > > > > > > > > > > > > > > Thanks for starting the discussion! > > > > > > > > > > > > > > > > > > I agree that the CLI is user facing, but I think we don't > > > > > > > > > have to treat it as other public APIs. > > > > > > > > > > > > > > > > > > I'd propose to throw user friendly exception for > > > > > > > > > `run-application` with suggestion to use `run` case > instead. > > > > > > > > > This would make users aware of the change and require them > > > > > > > > > to migrate their scripts. > > > > > > > > > > > > > > > > > > After two minor releases, and on next major version bump, > > > > > > > > > we could drop the `run-application` method as suggested > > > > > > > > > on discussion by Xintong. > > > > > > > > > > > > > > > > > > Best, > > > > > > > > > Muhammet > > > > > > > > > > > > > > > > > > On 2024-06-26 15:33, Ferenc Csaky wrote: > > > > > > > > > > > > > > > > > > > Hello devs, > > > > > > > > > > I would like to open a discussion about considerations > > > > > > > > > > regarding > > > > > > > > > > how > > > > > > > > > > to > > > > > > > > > > deprecate CLI > > > > > > > > > > actions, and what compatibility guarantees should apply > to > > > > > > > > > > such > > > > > > > > > > cases. > > > > > > > > > > The topic came up in > > > > > > > > > > a previous discussion [1] about a current FLIP to merge > the > > > > > > > > > > run > > > > > > > > > > and > > > > > > > > > > run-application > > > > > > > > > > behavior [2]. > > > > > > > > > > > > > > > > > > > > According to Xintong's previous inputs, currently the > Flink > > > > > > > > > > CLI, > > > > > > > > > > or > > > > > > > > > > its > > > > > > > > > > actions are not handled > > > > > > > > > > as public APIs by the existing definition (@Public or > > > > > > > > > > @PublicEvolving > > > > > > > > > > annotated). So > > > > > > > > > > legally it would be possible to change CLIs anytime. I > agree > > > > > > > > > > with > > > > > > > > > > Xintong that CLI actions > > > > > > > > > > should be considered as public APIs, and as such, > > > > > > > > > > compatibility > > > > > > > > > > guarantees should be > > > > > > > > > > provided. > > > > > > > > > > > > > > > > > > > > CLI actions are defined as private constants in > CliFrontend > > > > > > > > > > [3], > > > > > > > > > > so > > > > > > > > > > IMO the existing rules > > > > > > > > > > are not perfectly applicable as is. Both @Public and > > > > > > > > > > @PublicEvolving > > > > > > > > > > can be applied to > > > > > > > > > > fields (although for @Public that is only true as per the > > > > > > > > > > javadoc, > > > > > > > > > > technically it can only be > > > > > > > > > > applied to classes), so I think that could be a good > approach > > > > > > > > > > that is > > > > > > > > > > achievable with minimal > > > > > > > > > > effort and changes. > > > > > > > > > > > > > > > > > > > > It would also be possible to define a different process, > but > > > > > > > > > > IMO > > > > > > > > > > the > > > > > > > > > > more lightweight and > > > > > > > > > > maintainable the process the better, so it is a good > thing > > > > > > > > > > if it > > > > > > > > > > is > > > > > > > > > > not > > > > > > > > > > necessary to bring in > > > > > > > > > > new entities and/or rules to check and enforce > compatibility. > > > > > > > > > > > > > > > > > > > > WDYT? > > > > > > > > > > > > > > > > > > > > Best, > > > > > > > > > > Ferenc > > > > > > > > > > > > > > > > > > > > [1] > > > > > > https://lists.apache.org/thread/0vc8v3t7fr6w9hmwf9zbjbyk5c3bcj50 > > > > > > > > > > > > > [2] > > > > > > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=311626179 > > > > > > > > > > > > > [3] > > > > > > > https://github.com/apache/flink/blob/27287a105f6585e89795e2a6cbffa8254abb6e57/flink-clients/src/main/java/org/apache/flink/client/cli/CliFrontend.java#L98 >
