c)
@Ryan:
I'm generally fine with leaving it up to the connector on how to
implement Flink version-specific behavior, so long as the branching
models stays consistent (primarily so that the release process is
identical and we can share infrastructure).
@Danny:
In a single branch per version model we should update the version when
making a change specific to a given Flink version.
That a change for the 1.16 code can affect the 1.15 version is a price
I'm willing to pay.
If you use separate branches this becomes more complicated (surprise!),
because then you likely end up with converging code-bases using the same
version.
Because either
a) you have 1.0.0 for both with diverging Flink version-specific code,
b) create 1.1.0-1.16, but then what of 1.1.0-1.15? Do you skip
1.1.0 for 1.15? That would just be strange.
c1-3)
I don't think we need to limit ourselves to only adding patches for
Flink n-1.
In my mind it would progress like this:
- 1.0.0-1.14 (features & patches)
===> 1.15 is released, let's support it; adding some version-specific code
- 1.1.0-1.14 (feature and patches)
- 1.1.0-1.15 (feature and patches)
===> general improvements to the v1 connector
- 1.2.0-1.14 (feature and patches)
- 1.2.0-1.15 (feature and patches)
===> new major release targeting 1.16
- 1.2.0-1.14 (no support; unsupported Flink version)
- 1.2.0-1.15 (patches; supported until either 3.0 of 1.17, whichever happens
first)
- 2.0.0-1.15 (feature and patches)
- 2.0.0-1.16 (feature and patches)
d)
Does the social aspect actually require a "main" branch, or just a
_default_ branch on which development actually takes place?
Unless we develop a habit of creating a significant number of major
releases per year, we could use the branch of the latest connector
version as the default.
On 16/09/2022 20:03, Danny Cranmer wrote:
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.
