To stay consistent with existing code, we should simply add 2 constructors. One with ser/deser and one without. So that, users have the choice to use one or the other. I updated the KIP accordingly.
Le ven. 6 mai 2022 à 12:55, François Rosière <francois.rosi...@gmail.com> a écrit : > On the other hand, the KafkaConsumer constructor with a config + > serializer and deserializer already exists but is not public. > It would also complexify a bit the caller to not have the > serializer/deserializer exposed at constructor level. > > Once the KIP would have been implemented, for streams, instead of having a > custom config (already possible), I may simply define a custom > KafkaClientSupplier reusing the custom configs of both the producer and the > consumer. > This supplier currently creates producers and consumers using the > constructors with a map of config + serializer/deserializer. > > So, it seems it's easier to have the constructor with 3 parameters. But in > any case, it will work if the config can be accessed... > > Le ven. 6 mai 2022 à 12:14, François Rosière <francois.rosi...@gmail.com> > a écrit : > >> Hello, >> >> We may create a constructor with a single parameter which is the config >> but then, I would need to give the serializer/deserializer by also >> overriding the config. >> Like I would do for the interceptors. >> So, no real opinion on that, both solutions are ok for me. >> Maybe easier to take the approach of the single parameter. >> >> Hope it respond to the question. >> >> Kr, >> >> F. >> >> Le ven. 6 mai 2022 à 11:59, Bruno Cadonna <cado...@apache.org> a écrit : >> >>> Hi Francois, >>> >>> Thank you for updating the KIP! >>> >>> Now the motivation of the KIP is much clearer. >>> >>> I would still be interested in: >>> >>> >> 2. Why do you only want to change/add the constructors that take the >>> >> properties objects and de/serializers and you do not also want to >>> >> add/change the constructors that take only the properties? >>> >>> >>> Best, >>> Bruno >>> >>> On 05.05.22 23:15, François Rosière wrote: >>> > Hello Bruno, >>> > >>> > The KIP as been updated. Feel free to give more feedbacks and I will >>> > complete accordingly. >>> > >>> > Kr, >>> > >>> > F. >>> > >>> > Le jeu. 5 mai 2022 à 22:22, Bruno Cadonna <cado...@apache.org> a >>> écrit : >>> > >>> >> Hi Francois, >>> >> >>> >> Thanks for the KIP! >>> >> >>> >> Here my first feedback: >>> >> >>> >> 1. Could you please extend the motivation section, so that it is clear >>> >> for a non-Spring dev why the change is needed? Usually, a motivation >>> >> section benefits a lot from an actual example. >>> >> Extending the motivation section would also make the KIP more >>> >> self-contained which is important IMO since this is kind of a log of >>> the >>> >> major changes to Kafka. Descriptions of major changes should not >>> >> completely depend on external links (which may become dead in future). >>> >> Referencing external resources to point to more details or give >>> context >>> >> is useful, though. >>> >> >>> >> 2. Why do you only want to change/add the constructors that take the >>> >> properties objects and de/serializers and you do not also want to >>> >> add/change the constructors that take only the properties? >>> >> >>> >> 3. I found the following stalled KIP whose motivation is really >>> similar >>> >> to yours: >>> >> >>> >> >>> >> >>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-378%3A+Enable+Dependency+Injection+for+Kafka+Streams+handlers >>> >> >>> >> That KIP is also the reason why Kafka Streams still has the >>> constructors >>> >> with the StreamsConfig parameter. Maybe you want to mention this KIP >>> in >>> >> yours or even incorporate the remaining topology test driver API >>> changes >>> >> in your KIP. >>> >> Some related links: >>> >> - https://github.com/apache/kafka/pull/5344#issuecomment-413350338 >>> >> - https://github.com/apache/kafka/pull/10484 >>> >> - https://issues.apache.org/jira/browse/KAFKA-6386 >>> >> >>> >> Best, >>> >> Bruno >>> >> >>> >> >>> >> On 04.05.22 22:26, François Rosière wrote: >>> >>> Hi all, >>> >>> >>> >>> KIP-832 has been created to allow implementing Spring managed >>> >> interceptors >>> >>> for Producers and Consumers. >>> >>> >>> >>> At the moment, interceptors are given as configuration classes to the >>> >>> producer and consumer configurations. So, the idea here would be to >>> >> create >>> >>> 2 new constructors to allow using a Producer and Consumer >>> configuration >>> >>> instead of properties or a key value map of configurations entries. >>> >>> Interceptors could then be given as instances by overriding a config >>> >> method. >>> >>> More details can be found in the Spring issue. >>> >>> https://github.com/spring-projects/spring-kafka/issues/2244 >>> >>> >>> >>> Any feedback, proposal, vote for this KIP would be more than welcome. >>> >>> >>> >>> Kind regards, >>> >>> >>> >>> Francois R. >>> >>> >>> >>> Le lun. 2 mai 2022 à 21:05, François Rosière < >>> francois.rosi...@gmail.com> >>> >> a >>> >>> écrit : >>> >>> >>> >>>> Kip link: >>> >>>> >>> >> >>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=211882578 >>> >>>> >>> >>>> >>> >>> >>> >> >>> > >>> >>
