c) I am torn here. I do not like the idea that the connector code could diverge for the same connector version, ie 2.1.0-1.15 and 2.1.0-1.16. If the Flink version change requires a change to the connector code, then this should result in a new connector version in my opinion. Going back to your example of API changes, this means we could end up supporting new features for multiple Flink versions with potentially different @Internal implementations. Where do we draw the line here? Hypothetically if the @Internal Flink apis change substantially, but do not impact the public connector interface, this could still end up with the same connector version. Example:
- 1.0.0-1.14 (uses @Internal connector API v1) - 1.0.0-1.15 (uses @Internal connector API v2) But on the flip side, this example is very good "let's say exclusive to 1.15; some workaround for a bug or smth", and I am not sure how the single branch per version approach would solve it. c1) We would likely need to do something like this: - 1.0.0-1.14 (patches) - 1.1.0-1.15 (contains some 1.15 bugfix) (feature and patches) c2) When 1.16 is released, we would need to remove the 1.15 bugfix, and therefore change the connector code: - 1.0.0-1.14 - 1.1.0-1.15 (contains some 1.15 bugfix) (patches) - 1.2.0-1.16 (features and patches) c3) But following this approach I can see a case where we end up with a version gap to support Flink (n-1) patches: - 1.0.0-1.14 - 1.1.0-1.15 (contains some 1.15 bugfix) (patches) - 1.2.0-1.16 - 1.3.0-1.16 - 1.4.0-1.16 (patches) - 1.5.0-1.16 (features and patches) I think we need some animations to help with these! d) Agree with the "Socially" point generally. Thanks, On Fri, Sep 16, 2022 at 1:30 PM Ryan Skraba <ryan.skr...@aiven.io> wrote: > I had to write down a diagram to fully understand the discussion :D > > If I recall correctly, during the externalization discussion, the "price > to pay" for the (many) advantages of taking connectors out of the main repo > was to maintain and manually consult a compatibility matrix per connector. > I'm not a fan of that approach, and your example of diverging code between > 2.1.0-1.15 and 2.1.0-1.16 is a good reason why. > > b) I think your proposal here is a viable alternative. > > c) In my experience, the extra effort of correctly cherry-picking to the > "right" branches adds a small burden to each commit and release event. > > The biggest challenge will be for committers for each connector to be > mindful of which versions are "still in the game" (but this is also true > for the compatibility matrix approach). Two major versions of connectors > multiplied by two versions of Flink is up to three cherry-picks per commit > -- plus one if the connector is currently being migrated and exists > simultaneously inside and outside the main repo, plus another for the > previous still-supported version of flink. It's going to take some > education effort! > > Weighing in on the shim approach: this might be something to leave up to > each connector -- I can see it being easier or more relevant for some > connectors than others to use dedicated branches versus dedicated modules > per flink version, and this might evolve with the connector. > > d) Socially, it's a nice convention to have the `main` branch point to the > "newest and best" variant for new users to check out, test, build and > create a PR for quick fixes without diving deep into the policies of the > project, even if it is qualified for just the newest version of Flink. > > Danny: I like the common connector parent approach too. Looks tidy! > > All my best, Ryan > > > > > On Fri, Sep 16, 2022 at 10:41 AM Chesnay Schepler <ches...@apache.org> > wrote: > >> a) 2 Flink versions would be the obvious answer. I don't think anything >> else makes much sense. >> >> I don't want us to increment versions just because the Flink versions >> change, so in your example I'd go with 2.0.0-1.16. >> >> c) >> >> Generally speaking I would love to avoid the Flink versions in branch >> names, because it simplifies the git branching (everything, really) and >> as you said makes main useful. >> >> However the devil is in the details. >> >> Imagine we have 2.0.0 for 1.15, and 1.16 is released with now a new API >> (maybe even just a test util) that we want to use. >> >> For the sake of arguments let's say we go with 2.1.0-1.16, >> and at the same time we also had some pending changes for the 1.15 >> connector (let's say exclusive to 1.15; some workaround for a bug or >> smth), >> so we also have 2.1.0-1.15. >> >> We can't have 1 module use 2 different Flink API versions to satisfy >> this. Build profiles wouldn't solve this. >> >> We could do something like adding Flink-version specific shims though, >> somewhat similar to how we support ES 6/7. >> >> >> On 15/09/2022 23:23, Danny Cranmer wrote: >> > Thanks for starting this discussion. I am working on the early stages of >> > the new DynamoDB connector and have been pondering the same thing. >> > >> > a) Makes sense. On the flip side, how many Flink versions will we >> > support? Right now we support 2 versions for Flink, so it makes sense to >> > follow this rule. >> > >> > For example if the latest connector version is 2.0.0, we would only >> publish >> > 2.0.0-1.15 and 2.0.0-1.14. >> > Then once we want to ship connector 2.1.0 if Flink 1.16 is out, we would >> > publish 2.1.0-1.16 and 2.1.0-1.15. >> > Which leaves the case when a new Flink version is released (1.16 for >> > example) and connector 2.0.0 is already published. We do not have any >> > connector changes so could consider adding 2.0.0-1.16 (resulting in >> > 2.0.0-1.16, 2.0.0-1.15 and 2.0.0-1.14 (no longer supported)) or >> requiring a >> > version bump to 2.1.0-1.16. I would prefer adding 2.0.0-1.16 if there >> are >> > no changes to the code, and this is possible. If a connector code never >> > changes, we would end up with 2.0.0-1.14, 2.0.0-1.15 ... 2.0.0-n.m >> > >> > b) I like this suggestion, even though the Flink dependencies are >> usually >> > "provided" and therefore the builds are identical. It gives the users a >> > clear indicator of what is supported, and allows us to target tests >> against >> > different Flink versions consistently. >> > >> > c) Instead of having a branch per Flink version can we have multiple >> build >> > profiles like the Scala variable? Having 1.0-1.15 and 1.0-1.16 branches >> > would likely be duplicate code and increase the maintenance burden >> (having >> > to merge PRs into multiple branches). If the connector code is not >> > compatible with both Flink versions we can bump the connector version at >> > this point. I would propose following the Flink branching strategy >> > "release-1.0" unless this will not work. >> > >> > d) If we remove the Flink qualifier from the branch as discussed above, >> > then main can be the next major version, like in Flink. >> > >> >>
