Hi all,
KIP-839 has been created to progress on the builder way
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=211884640
Kr,
F.
Le ven. 13 mai 2022 à 19:15, Colin McCabe <cmcc...@apache.org> a écrit :
> I agree. I would really like to see a builder interface here. If you
> changed the KIP to add this, you'd have to do the vote again, but I think
> it would be well worth it.
>
> best,
> Colin
>
>
> On Thu, May 12, 2022, at 00:13, Bruno Cadonna wrote:
> > Hi Francois,
> >
> > Modifying this KIP or starting a new one is your call.
> >
> > I guess the idea of the builder might cause some more discussions. So if
> > you want to get a working version released as soon as possible, I would
> > opt to get the current KIP implemented and create a new one for the
> > builder. If time is not an issue I would modify the current KIP.
> >
> > But as I said: It's your call.
> >
> > Best,
> > Bruno
> >
> > On 11.05.22 11:01, François Rosière wrote:
> >> To be clear, there is no problem for me to update the current KIP with
> the
> >> builder approach.
> >> It's not a lot of work in terms of code.
> >> So, up to you. Let me know and I will do the necessary to go in one or
> the
> >> other direction...
> >> Thanks again for the feedbacks.
> >>
> >> Le mer. 11 mai 2022 à 10:52, François Rosière <
> francois.rosi...@gmail.com>
> >> a écrit :
> >>
> >>> Hello,
> >>>
> >>> Builder is clearly the way to go for future releases of Kafka.
> >>>
> >>> If we align streams, we would have 3 builders
> >>>
> >>> new ConsumerBuilder<String, MyPojo>()
> >>> .withKeyDeserializer(<KEY_DESERIALIZER>)
> >>> .withValueDeserializer(<VALUE_DESERIALIZER>)
> >>> .withInterceptors(<LIST_OF_INTERCEPTORS>)
> >>> .withMetricsReporter(<METRICS_REPORTER>)
> >>> .build(<MAP_OR_PROPERTIES_OR_CONFIG>);
> >>>
> >>> new ProducerBuilder<String, MyPojo>()
> >>> .withKeySerializer(<KEY_SERIALIZER>)
> >>> .withValueSerializer(<VALUE_SERIALIZER>)
> >>> .withInterceptors(<LIST_OF_INTERCEPTORS>)
> >>> .withPartitioner(<PARTITIONER>)
> >>> .withMetricsReporter(<METRICS_REPORTER>)
> >>> .build(<MAP_OR_PROPERTIES_OR_CONFIG>);
> >>>
> >>> new KafkaStreamsBuilder(<TOPOLOGY>)
> >>> .withProducerInterceptors(<LIST_OF_PRODUER_INTERCEPTORS>)
> >>> .withConsumerInterceptors(<LIST_OF_CONSUMER_INTERCEPTORS>)
> >>> .withTime(<TIME>)
> >>> .withKafkaClientSupplier(<KAFKA_CLIENT_SUPPLIER>)
> >>> .withMetricsReporter(<METRICS_REPORTER>)
> >>> .build(<MAP_OR_PROPERTIES_OR_CONFIG>);
> >>>
> >>> The builder property would always override the configuration
> "instances".
> >>> There is maybe other methods to add to the builders...
> >>> The map, properties or config could be given to the constructor of the
> >>> builder instead of the build method...
> >>> At the end, we may only keep one single constructor in the Producer,
> >>> Consumer and KafkaStreams obects.
> >>>
> >>> @Chris, @Bruno, thank you for your replies and proposals. Do you want I
> >>> create another KIP explaining the builder approach or do you prefer to
> do
> >>> it?
> >>>
> >>> Kr,
> >>>
> >>> F.
> >>>
> >>>
> >>> Le mer. 11 mai 2022 à 09:46, Bruno Cadonna <cado...@apache.org> a
> écrit :
> >>>
> >>>> Hi Francois and Chris,
> >>>>
> >>>> I find the idea of the builder interesting.
> >>>>
> >>>> I think we should go ahead with the current KIP as it is to allow
> >>>> Francois to fix his issue soon. If one of you or both want to push
> >>>> forward the builder idea, feel free to create a new KIP and discuss it
> >>>> with the community.
> >>>>
> >>>> Regarding Francois' questions:
> >>>>
> >>>> 3. If the existing constructors should be removed, they need to be
> >>>> marked as deprecated and removed in one of the next major releases.
> >>>>
> >>>> 5. Yes, I think Streams should be aligned.
> >>>>
> >>>> Both questions should be discussed in the context of a new KIP about
> the
> >>>> builder idea.
> >>>>
> >>>> Best,
> >>>> Bruno
> >>>>
> >>>> On 11.05.22 04:24, Chris Egerton wrote:
> >>>>> Hi Francois,
> >>>>>
> >>>>> Thanks for your thoughts. I think it's worth noting that in regards
> to
> >>>> item
> >>>>> 2, it's possible to explicitly declare the type parameters for a
> builder
> >>>>> without capturing it in a variable; for example:
> >>>>>
> >>>>> KafkaProducer<String, Integer> p = new Builder<String, Integer>(...)
> >>>>> .withKeySerializer(new StringSerializer())
> >>>>> .withValueSerializer(new IntegerSerializer())
> >>>>> .build();
> >>>>>
> >>>>> That aside, given the three binding votes already cast on the vote
> >>>> thread,
> >>>>> it's probably too late to be worth changing direction at this point.
> >>>> Thanks
> >>>>> for entertaining the proposal, and congratulations on your KIP!
> >>>>>
> >>>>> Cheers,
> >>>>>
> >>>>> Chris
> >>>>>
> >>>>> On Tue, May 10, 2022 at 5:33 PM François Rosière <
> >>>> francois.rosi...@gmail.com>
> >>>>> wrote:
> >>>>>
> >>>>>> Hello Chris,
> >>>>>>
> >>>>>> Thanks for the feedback. Builders is definitely the pattern to apply
> >>>> when
> >>>>>> an object needs to be built using different arguments/combinations.
> >>>>>>
> >>>>>> Here are my thoughts about the proposal:
> >>>>>>
> >>>>>> 1. The builder should only expose meaningful methods for the users
> >>>> such as
> >>>>>> the interceptors, the serializer/deserializer, partitioner, etc. A
> >>>> method
> >>>>>> like the configured instances is internal and should not be exposed
> if
> >>>> we
> >>>>>> don't want to expose the config itself. Using this internal method
> is
> >>>> the
> >>>>>> only way to solve the issue if the config is exposed.
> >>>>>>
> >>>>>> 2. As the key and value types are not given, a variable will need
> to be
> >>>>>> created for the builder before being used. Otherwise, there is no
> way
> >>>> to
> >>>>>> infer the type correctly. Breaks a bit the inline usage with DSL
> style.
> >>>>>>
> >>>>>> 3. What about existing constructors, they would need to stay to keep
> >>>>>> compatibility with existing o could they be removed in benefit of
> the
> >>>>>> builder?
> >>>>>>
> >>>>>> 4. Having an access to the config also gives a way to also fine tune
> >>>> other
> >>>>>> aspects such as the logging related to the config. Log unused, skip
> >>>> some
> >>>>>> properties, etc.
> >>>>>>
> >>>>>> 5. What about streams? Shouldn't it be aligned?
> >>>>>>
> >>>>>> So, to summarise, the KIP was a best effort solution to support
> already
> >>>>>> configured instances related to both the producer and the consumer.
> >>>>>> The builder will work, it's just a matter of deciding the best
> >>>> approach...
> >>>>>> for me, both solutions are fine, I just need a way to inject already
> >>>>>> configured dependencies into the producers and consumers.
> >>>>>>
> >>>>>> If we conclude, I will drop a PR on Github.
> >>>>>>
> >>>>>> Kr,
> >>>>>>
> >>>>>> F.
> >>>>>>
> >>>>>> Le mar. 10 mai 2022 à 15:01, Chris Egerton <fearthecel...@gmail.com>
> a
> >>>>>> écrit :
> >>>>>>
> >>>>>>> Hi Francois,
> >>>>>>>
> >>>>>>> Thanks for the KIP! I sympathize with the issue you're facing and
> with
> >>>>>>> John's reluctance to let perfect be the enemy of good, and if KIP
> >>>> freeze
> >>>>>>> were tomorrow, I think this would be good enough. Given that we
> still
> >>>>>> have
> >>>>>>> some time to work with, I'd like to propose an alternative approach
> >>>> and
> >>>>>> see
> >>>>>>> what your thoughts are.
> >>>>>>>
> >>>>>>> There are a few issues with the current client APIs that are
> closely
> >>>>>>> related to the KIP:
> >>>>>>> 1. Too many constructors (there are currently four each for
> >>>> KafkaProducer
> >>>>>>> and KafkaConsumer, yet they all do basically the same thing)
> >>>>>>> 2. Lack of type safety with interceptors (you have no way to
> enforce
> >>>> at
> >>>>>>> compile time that your ProducerInterceptor<String, Integer> is used
> >>>> with
> >>>>>> a
> >>>>>>> Serializer<String> and Serializer<Integer>, for example)
> >>>>>>> 3. Inflexibility and inconsistency with instantiation of pluggable
> >>>>>>> interfaces (you can bring your own (de)serializers, but everything
> >>>> else
> >>>>>>> gets instantiated and configured for you at producer/consumer
> >>>>>> construction
> >>>>>>> time)
> >>>>>>>
> >>>>>>> The KIP as it exists now will only address item 3, and will
> exacerbate
> >>>>>> item
> >>>>>>> 1.
> >>>>>>>
> >>>>>>> In addition, there are a few new issues introduced by the KIP as it
> >>>>>> exists
> >>>>>>> now:
> >>>>>>> 1. Tighter coupling between the ProducerConfig/ConsumerConfig
> classes
> >>>> and
> >>>>>>> the KafkaProducer/KafkaConsumer classes. Any change we make in the
> >>>> future
> >>>>>>> that breaks either of these config classes in unexpected ways (but
> >>>> which
> >>>>>>> does not break the KafkaProducer/KafkaConsumer constructors that do
> >>>> not
> >>>>>>> accept these classes as parameters) will now have a much higher
> >>>> chance to
> >>>>>>> also break a user's entire producer/consumer application.
> >>>>>>> 2. Complexity for users like yourself who would like to override
> >>>> behavior
> >>>>>>> in a ProducerConfig/ConsumerConfig in order to inject
> pre-instantiated
> >>>>>>> dependencies. The example in the KIP overrides
> >>>>>>> AbstractConfig::getConfiguredInstances [1] in order to achieve
> this.
> >>>> But
> >>>>>>> there are two other overloaded variants of getConfiguredInstances,
> and
> >>>>>> two
> >>>>>>> AbstractConfig::getConfiguredInstance methods that also exist. We'd
> >>>>>> either
> >>>>>>> need to establish a dependency graph between these methods (e.g.,
> some
> >>>>>>> methods are guaranteed to invoke another overloaded variant) as
> part
> >>>> of
> >>>>>> the
> >>>>>>> public API for the AbstractConfig, or users would need to override
> >>>> every
> >>>>>>> single one of these methods in order to ensure that their code
> won't
> >>>>>> break
> >>>>>>> at runtime after bumping their Kafka version.
> >>>>>>>
> >>>>>>> I think introducing the builder pattern for KafkaProducer and
> >>>>>> KafkaConsumer
> >>>>>>> would alleviate all of these issues. As a rough draft of what the
> API
> >>>>>> might
> >>>>>>> look like for KafkaProducer:
> >>>>>>>
> >>>>>>> public class Builder<K, V> {
> >>>>>>> private final Map<String, Object> props;
> >>>>>>> private Serializer<K> keySerializer;
> >>>>>>> private Serializer<V> valueSerializer;
> >>>>>>> private List<ProducerInterceptor<K, V>> interceptors;
> >>>>>>> private Map<String, Object> configuredInstances;
> >>>>>>> private Map<String, List<Object>> configuredInstanceLists;
> >>>>>>>
> >>>>>>> public Builder(Map<String, Object> props) {
> >>>>>>> this.props = props;
> >>>>>>> this.interceptors = new ArrayList<>();
> >>>>>>> this.configuredInstances = new HashMap<>();
> >>>>>>> this.configuredInstanceLists = new HashMap<>();
> >>>>>>> }
> >>>>>>>
> >>>>>>> // Use this serializer, if non-null
> >>>>>>> // Will take precedence over any serializer class specified
> in
> >>>> the
> >>>>>>> properties for this producer
> >>>>>>> public Builder withKeySerializer(Serializer<K> serializer) {
> >>>>>>> this.keySerializer = serializer;
> >>>>>>> return this;
> >>>>>>> }
> >>>>>>>
> >>>>>>> public Builder withValueSerializer(Serializer<V> serializer)
> {
> >>>>>>> this.valueSerializer = serializer;
> >>>>>>> return this;
> >>>>>>> }
> >>>>>>>
> >>>>>>> // Use these interceptors (has no effect if null)
> >>>>>>> // Each must already be configured
> >>>>>>> // Will be combined with any interceptor classes also
> specified
> >>>> in
> >>>>>> the
> >>>>>>> properties for this producer
> >>>>>>> public Builder withInterceptors(List<ProducerInterceptor<K,
> V>>
> >>>>>>> interceptors) {
> >>>>>>> if (interceptors != null) {
> >>>>>>> this.interceptors.addAll(interceptors);
> >>>>>>> }
> >>>>>>> return this;
> >>>>>>> }
> >>>>>>>
> >>>>>>> // Use this plugin instance, if non-null
> >>>>>>> // Must already be configured
> >>>>>>> // Will take precedence over any plugin class specified for
> the
> >>>> same
> >>>>>>> property in the properties for this producer (wording here needs
> work
> >>>> but
> >>>>>>> you get the idea)
> >>>>>>> public Builder withConfiguredInstance(String property, Object
> >>>>>> instance)
> >>>>>>> {
> >>>>>>> this.configuredInstances.put(property, instance);
> >>>>>>> return this;
> >>>>>>> }
> >>>>>>>
> >>>>>>> // Use these plugin instances (has no effect if null)
> >>>>>>> // Each must already be configured
> >>>>>>> // Will be combined with any plugin classes also specified
> for
> >>>> the
> >>>>>> same
> >>>>>>> property in the properties for this consumer
> >>>>>>> public Builder withConfiguredInstances(String property,
> >>>> List<Object>
> >>>>>>> instances) {
> >>>>>>> this.configuredInstanceLists.put(property, instances);
> >>>>>>> return this;
> >>>>>>> }
> >>>>>>>
> >>>>>>> public KafkaProducer<K, V> build() { ... }
> >>>>>>> }
> >>>>>>>
> >>>>>>> Thoughts?
> >>>>>>>
> >>>>>>> [1] -
> >>>>>>>
> >>>>>>>
> >>>>>>
> >>>>
> https://kafka.apache.org/31/javadoc/org/apache/kafka/common/config/AbstractConfig.html#getConfiguredInstances(java.lang.String,java.lang.Class,java.util.Map)
> >>>>>>>
> >>>>>>> Cheers,
> >>>>>>>
> >>>>>>> Chris
> >>>>>>>
> >>>>>>> On Mon, May 9, 2022 at 4:55 PM Bruno Cadonna <cado...@apache.org>
> >>>> wrote:
> >>>>>>>
> >>>>>>>> Hi Francois,
> >>>>>>>>
> >>>>>>>> I think you can go ahead and call for votes.
> >>>>>>>>
> >>>>>>>> Could you please also clean up a little bit the KIP since it has
> >>>> still
> >>>>>>>> parts that refer to its first version? For example,
> "Compatibility,
> >>>>>>>> Deprecation, and Migration Plan" still mentions only two
> >>>> constructors.
> >>>>>>>> IMO you can also remove section "Public Interfaces" since it does
> not
> >>>>>>>> contain much information.
> >>>>>>>>
> >>>>>>>> Best,
> >>>>>>>> Bruno
> >>>>>>>>
> >>>>>>>> On 09.05.22 17:45, Bruno Cadonna wrote:
> >>>>>>>>> Hi Francois,
> >>>>>>>>>
> >>>>>>>>> You can open a PR and people can review it, but it must not be
> >>>> merged
> >>>>>>>>> until the KIP is approved.
> >>>>>>>>>
> >>>>>>>>> Best,
> >>>>>>>>> Bruno
> >>>>>>>>>
> >>>>>>>>> On 09.05.22 16:07, François Rosière wrote:
> >>>>>>>>>> Can a PR be dropped on Github or do we still need some approval
> >>>>>> first?
> >>>>>>>>>>
> >>>>>>>>>> Le dim. 8 mai 2022 à 06:08, John Roesler <vvcep...@apache.org>
> a
> >>>>>>> écrit
> >>>>>>>> :
> >>>>>>>>>>
> >>>>>>>>>>> Thanks, François!
> >>>>>>>>>>>
> >>>>>>>>>>> Those changes look good to me.
> >>>>>>>>>>>
> >>>>>>>>>>> Thanks,
> >>>>>>>>>>> -John
> >>>>>>>>>>>
> >>>>>>>>>>> On Fri, May 6, 2022, at 13:51, François Rosière wrote:
> >>>>>>>>>>>> The KIP has been updated to reflect the last discussion
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=211882578#KIP832:Allowcreatingaproducer/consumerusingaproducer/consumerconfig-ProposedChanges
> >>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> Le ven. 6 mai 2022 à 20:44, François Rosière
> >>>>>>>>>>>> <francois.rosi...@gmail.com>
> >>>>>>>>>>> a
> >>>>>>>>>>>> écrit :
> >>>>>>>>>>>>
> >>>>>>>>>>>>> Hello,
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> No problem to also add a constructor taking the
> StreamsConfig in
> >>>>>>> the
> >>>>>>>>>>>>> TopologyTestDriver.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Summary about the changes to apply:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> - Create 2 new constructors in KafkaProducer
> >>>>>>>>>>>>> - Create a new constructor in KafkaConsumer and
> increase de
> >>>>>>>>>>> visibility
> >>>>>>>>>>>>> of an existing one
> >>>>>>>>>>>>> - Create a new constructor in TopologyTestDriver
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Kr,
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> F.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Le ven. 6 mai 2022 à 16:57, John Roesler <
> vvcep...@apache.org>
> >>>> a
> >>>>>>>> écrit
> >>>>>>>>>>> :
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>> Thanks for the KIP, François!
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> I'm generally in favor of your KIP, since you're
> >>>>>>>>>>>>>> proposing to follow the existing pattern of the
> >>>>>>>>>>>>>> constructors for both Producer and Consumer,
> >>>>>>>>>>>>>> but with the config object instead of Properties
> >>>>>>>>>>>>>> or Map configs. Also, because we already have
> >>>>>>>>>>>>>> this pattern in Streams, and we are just
> >>>>>>>>>>>>>> extending it to Producer and Consumer.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Following on the KIP-378 discussion, I do still think
> >>>>>>>>>>>>>> this is somewhat of an abuse of the Config objects,
> >>>>>>>>>>>>>> and it would be better to have a formal dependency
> >>>>>>>>>>>>>> injection interface, but I also don't want to let perfect
> >>>>>>>>>>>>>> be the enemy of good. Since it looks like this approach
> >>>>>>>>>>>>>> works, and there is also some precedent for it already,
> >>>>>>>>>>>>>> I'd be inclined to approve it.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Since KIP-378 didn't make it over the finish line, and it
> >>>>>>>>>>>>>> seems like a small expansion to your proposal, do you
> >>>>>>>>>>>>>> mind also adding the StreamsConfig to the
> >>>>>>>>>>>>>> TopologyTestDriver constructors? That way, we can go
> >>>>>>>>>>>>>> ahead and resolve both KIPs at once.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Thank you,
> >>>>>>>>>>>>>> -John
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> On Fri, May 6, 2022, at 06:06, François Rosière wrote:
> >>>>>>>>>>>>>>> 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
> >>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>>
> >>
>
