Hi Sébastien,
Thanks for the KIP!
Cleaning up the configs classes is really a good idea!
BC1:
I am in favor of having only StreamsConfig as a config class to avoid
proliferation of config classes. However, I find it a bit ugly that a
Streams user is required to pass the config first to the topology and
then to KafkaStreams. Since we are talking about changing constructors,
I am wondering if it were possible to pass the configs only to KafkaStreams.
BC2:
While Sophie advocates to only pass a Map/Properties object into the
constructors, I am advocating to only passing StreamsConfig objects to
the constructors. The reason is that Map/Properties are mutable and
StreamsConfig is immutable. IMO, it would be safer to pass immutable
configs to the constructors. But maybe there is a good reason to pass
mutable configs that I do not know. Maybe Sophie can clarify.
+1 to adding all considerations discussed in the thread to the rejected
alternatives.
Best,
Bruno
On 06.03.25 02:43, Sophie Blee-Goldman wrote:
Thanks for the KIP! The proposal here sounds like the right direction to go
in for me, but I'll be interested to hear what others think.
I'm wondering if we should offer additional StreamsBuilder & Topology
constructors that accept a plain config map (or Properties) alongside the
StreamsConfig constructors. I'd advocate for providing only the
Map/Properties variety except that I'm pretty sure the StreamsConfig
options are needed for dependency injection and were requested by users of
Spring
Also, just to list out some alternatives, we could go in a different
direction completely by making TopologyConfig a true config in its own
right, and moving all the topology-specific configs from StreamsConfig to
TopologyConfig (ie removing them from StreamsConfig completely)
On the one hand, it might make more sense to keep configs separated and
group them by where they need to be applied. On the other hand I wouldn't
want to start proliferating config classes since I think this makes
discovery more difficult and increases the API surface area. So personally,
I think it's sufficient to just add the check and warning log described in
the KIP
We could also deprecate the default constructor for TopologyStreamsBuilder
to force people to pass configs in. I'm honestly fine either way, as I
don't really think it's much of a burden to ask users to pass configs into
one more place, but since the new check should help prevent users from
accidentally forgetting to pass the topology-specific configs into the
Topology/StreamsBuilder, then deprecating the default constructors seem
like na unnecessary step.
Sébastien -- My only other feedback for you on the KIP would be to list
these under the Rejected Alternatives section so we know what the range of
options are.
Thanks!
-Sophie
On Wed, Feb 26, 2025 at 11:57 PM Sebastien Viale <
sebastien.vi...@michelin.com> wrote:
Hi Everyone,
I would like to start a discussion on KIP-1138: Clean up TopologyConfig
and API for supplying configs needed by the topology<
https://cwiki.apache.org/confluence/display/KAFKA/KIP-1138%3A+Clean+up+TopologyConfig+and+API+for+supplying+configs+needed+by+the+topology
This proposal aims to simplify Kafka Streams configuration by unifying
APIs and ensuring topology-specific configs (e.g., topology.optimization,
processor.wrapper.class) are correctly applied to prevent silent
misconfigurations.
Regards,
Sébastien
