Thanks Ahmed I think I didn't express myself fully. The issue with a major release is not just timing, We should also put this from the perspective of the user. Particularly not-so-expert users.
Because the change breaks the interface of @PublicEvolving class, in particular the interface KinesisDeserializationSchema, from the user perspective upgrading requires code changes, even if the user is not interested in using de-aggregation. This is not user friendly. Major version changes are always scary for users running stable production workloads. Because AWS connectors are all in the same repo, they should all get bumped to a new major version. Even all connectors not getting any major changes (or without any changes at all). This will be confusing and not a great user experience. On top of that, there are a number of bug fixes in the line, in Kinesis and DynamoDB connectors, that should get merged as soon as possible. Ideally, these should be released in a minor version. Perceived as less "scary" and requiring no code changes. If a new major version is released, with both the Kinesis source de-aggregation new feature and bug fixes, users will be forced to a "scary" major version upgrade to get the bug fixes. And if they implemented their own KinesisDeserializationSchema, they will be forced to code changes, even if they are not interested in de-aggregation. Again, not user friendly. Alternatively, the community will have to maintain two versions: say, a 6.0.0 with the new breaking changes + bug fixes, and a 5.0.1, containing only the bug fixes but no de-aggregation. This does not sound practical and will confuse users about "which version should I use" Consider that the proposed solution is not a hacky workaround. It may be less elegant. But from the perspective of the user application evolution, if a user does not need de-aggregation they can continue to use their existing code. This new optional feature will be used either in completely new applications which require de-aggregation, or applications currently using the legacy FlinkKinesisConsumer blocked from upgrading to KinesisStreamSource because they need de-aggregation. Unless a way of implementing de-aggregation without breaking the public interface is found, I think the solution of a separate and optional deserialization schema wrapper is more user friendly. The drawbacks I see are more: - it requires a docs update, to explain how to enable de-aggregation (adding the wrapper to their existing DeserializationSchema). But I'd be happy to take care of it. - it is not transparent. The user must positively "enable" de-aggregation if needed. If someone turns on aggregation upstream, they will notice the problem immediately because the application will just break. Lorenzo On Tue, 29 Apr 2025 at 14:47, Ahmed Hamdy <hamdy10...@gmail.com> wrote: > Hi Lorenzo, > I am not sure I follow the issue of awaiting a major release, we have seen > the major releases for AWS connectors being picked up quite fast so we can > just push for one. Plus the contribution is not a bug fix, additionally the > legacy source is not yet removed. hence I would personally not rush any > workaround in the apache AWS connectors repo but rather push for a release > once the feature is merged. > Best Regards > Ahmed Hamdy > > > On Mon, 28 Apr 2025 at 17:22, Lorenzo Nicora <lorenzo.nic...@gmail.com> > wrote: > > > Hello > > the current version of Kinesis source (5.0.0) lacks support for KLP > > de-aggregation [1] > > There is a pending PR [2] but it breaks the API and will require a new > > major version of the connector. > > > > I implemented an alternative solution using a DeserializationSchema > wrapper > > which adds de-aggregation to any other DeserializationSchema [3]. > > > > Would it be a reasonable approach including this DeserializationSchema > > wrapper (with proper tests) as part of the connector distribution? > > A user can decide to use the wrapper if they need de-aggregation. > > The wrapper works fine even if the stream is not aggregated, with some > > minimal overhead. > > > > The wrapper will be redundant when/if the main fix [1] is released. > > However, requiring a new major version, this may take a while and some > > users are blocked from upgrading to the new source by lack of > > de-aggregation support. > > > > Any opinion on this approach? > > > > Lorenzo > > > > [1] https://issues.apache.org/jira/browse/FLINK-32097 > > [2] https://github.com/apache/flink-connector-aws/pull/188 > > [3] > > > > > https://github.com/aws-samples/amazon-managed-service-for-apache-flink-examples/blob/main/java/KinesisSourceDeaggregation/flink-app/src/main/java/com/amazonaws/services/msf/deaggregation/KinesisDeaggregatingDeserializationSchemaWrapper.java > > >
