Thanks for bringing up this discussion, Becket. My two cents:
1. Do we allow deprecation & removal of APIs without adding a new one as a replacement? The examples in the table give me an impression that marking an API as `@Deprecated` should only happen at the same time of introducing a new replacing API, which I think is true in most but not all the cases. If there is a major version bump before 2 minor releases in the current > major version are reached, the major version should keep the source code in > its own minor version until two minor versions are reached. For example, in > the above case, if Flink 2.0 is released after 1.20, then the deprecated > source code of foo will be kept in 2.0 and all the 2.x versions. It can > only be removed in 3.0. > 2. I think this might be a bit too strict. For an API that we already decided to remove, having to keep it for all the 2.x versions simply because there's less than 2 minor releases between making the decision and the major release bump seems not necessarily. Alternatively, I'd like to propose to remove the `@Public` annotation (or downgrade it to `@PublicEvolving`) in 2.0, and remove it in 2.2. Best, Xintong On Wed, Jun 14, 2023 at 3:56 PM Becket Qin <becket....@gmail.com> wrote: > Hi Jing, > > Thanks for the feedback. Please see the answers to your questions below: > > *"Always add a "Since X.X.X" comment to indicate when was a class / > > interface / method marked as deprecated."* > > Could you describe it with a code example? Do you mean Java comments? > > It is just a comment such as "Since 1.18. Use X > < > https://kafka.apache.org/31/javadoc/org/apache/kafka/clients/admin/Admin.html#incrementalAlterConfigs(java.util.Map) > >XX > instead.". And we can then look it up in the deprecated list[1] in each > release and see which method should / can be deprecated. > > *"At least 1 patch release for the affected minor release for > > Experimental APIs"* > > The rule is absolutely right. However, afaiac, deprecation is different > as > > modification. As a user/dev, I would appreciate, if I do not need to do > any > > migration work for any deprecated API between patch releases upgrade. > BTW, > > if experimental APIs are allowed to change between patches, could we just > > change them instead of marking them as deprecated and create new ones to > > replace them? > > Deprecating an API is just a more elegant way of replacing an API with a > new one. The only difference between the two is whether the old API is kept > and coexists with the new API for some releases or not. For end users, > deprecation-then-remove is much more friendly than direct replacement. > > 1. How to make sure the new APIs cover all functionality, i.e. backward > > compatible, before removing the deprecated APIs? Since the > > functionalities could only be built with the new APIs iteratively, there > > will be a while (might be longer than the migration period) that the new > > APIs are not backward compatible with the deprecated ones. > > This is orthogonal to the deprecation process, and may not even be required > in some cases if the function changes by itself. But in general, this > relies on the developer to decide. A simple test on readiness is to see if > all the UT / IT cases written with the old API can be migrated to the new > one and still work. If the new API is not ready, we probably should not > deprecate the old one to begin with. > > 2. Is it allowed to remove the deprecated APIs after the defined migration > > period expires while the new APis are still not backward compatible? > > By "backwards compatible", do you mean functionally equivalent? If the new > APIs are designed to be not backwards compatible, then removing the > deprecated source code is definitely allowed. If we don't think the new API > is ready to take over the place for the old one, then we should wait. The > migration period is the minimum time we have to wait before removing the > source code. A longer migration period is OK. > > 3. For the case of core API upgrade with downstream implementations, e.g. > > connectors, What is the feasible deprecation strategy? Option1 bottom-up: > > make sure the downstream implementation is backward compatible before > > removing the deprecated core APIs. Option2 top-down: once the downstream > > implementation of new APIs works fine, we can remove the deprecated core > > APIs after the migration period expires. The implementation of the > > deprecated APIs will not get any further update in upcoming releases(it > has > > been removed). There might be some missing features in the downstream > > implementation of new APIs compared to the old implementation. Both > options > > have their own pros and cons. > > The downstream projects such as connectors in Flink should also follow the > migration path we tell our users. i.e. If there is a cascading backwards > incompatible change in the connectors due to a backwards incompatible > change in the core, and as a result a longer migration period is required, > then I think we should postpone the removal of source code. But in general, > we should be able to provide the same migration period in the connectors as > the flink core, if the connectors are upgraded to the latest version of > core promptly. > > Thanks, > > Jiangjie (Becket) Qin > > [1] > > https://nightlies.apache.org/flink/flink-docs-master/api/java/deprecated-list.html > > > On Wed, Jun 14, 2023 at 1:15 AM Jing Ge <j...@ververica.com.invalid> > wrote: > > > > This is by design. Most of these are @Public APIs that we had to carry > > > around until Flink 2.0, because that was the initial guarantee that we > > > gave people. > > > > > > > True, I knew @Public APIs could not be removed before the next major > > release. I meant house cleaning without violation of these annotations' > > design concept. i.e especially cleaning up for @PublicEvolving APIs since > > they are customer-facing. Regular cleaning up with all other > @Experimental > > and @Internal APIs would be even better, if there might be some APIs > marked > > as @deprecated. > > > > Best regards, > > Jing > > > > > > > > On Tue, Jun 13, 2023 at 4:25 PM Chesnay Schepler <ches...@apache.org> > > wrote: > > > > > On 13/06/2023 12:50, Jing Ge wrote: > > > > One major issue we have, afaiu, is caused by the lack of > > > housekeeping/house > > > > cleaning, there are many APIs that were marked as deprecated a few > > years > > > > ago and still don't get removed. Some APIs should be easy to remove > and > > > > others will need some more clear rules, like the issue discussed at > > [1]. > > > > > This is by design. Most of these are @Public APIs that we had to carry > > > around until Flink 2.0, because that was the initial guarantee that we > > > gave people. > > > > > > > > > > As for the FLIP, I like the idea of explicitly writing down a > > > deprecation period for APIs, particularly PublicEvolving ones. > > > For Experimental I don't think it'd be a problem if we could change > them > > > right away, > > > but looking back a bit I don't think it hurts us to also enforce some > > > deprecation period. > > > 1 release for both of these sound fine to me. > > > > > > > > > My major point of contention is the removal of Public APIs between > minor > > > versions. > > > This to me would a major setback towards a simpler upgrade path for > > users. > > > If these can be removed in minor versions than what even is a major > > > release? > > > The very definition we have for Public APIs is that they stick around > > > until the next major one. > > > Any rule that theoretically allows for breaking changes in Public API > in > > > every minor release is in my opinion not a viable option. > > > > > > > > > The "carry recent Public APIs forward into the next major release" > thing > > > seems to presume a linear release history (aka, if 2.0 is released > after > > > 1.20, then there will be no 1.21), which I doubt will be the case. The > > > idea behind it is good, but I'd say the right conclusion would be to > not > > > make that API public if we know a new major release hits in 3 months > and > > > is about to modify it. With a regular schedule for major releases this > > > wouldn't be difficult to do. > > > > > >
