I don't believe it's possible to only pass the configurations into KafkaStreams, at least not without a huge refactoring of how and when the topology is compiled. I looked into this for the processor wrapper config since obviously it's preferable to not require configs be passed in to the Topology/StreamsBuilder, and wasn't able to make it work. If it's possible at all, it would have significantly complicated the already-complex topology construction code. I don't think it's worth it.
If we really wanted to only pass configs into one place, I'd say we should just require them for the Topology and StreeamsBuilder constructor, and not pass configs into KafkaStreams at all. But that would mean a deprecation/code change affecting every single user out there, which feels like unnecessary turmoil. As for whether to require StreamsConfig vs Map/Properties, I'm fine with requiring StreamsConfig in theory, but since KafkaStreams allows users to pass in a plain config map then some users might not instantiate StreamsConfig at all, and this would force them to do so. Maybe that's not such a bad thing though...so yeah, guess I'm fine with just the StreamsConfig variant :) On Thu, Mar 13, 2025 at 3:17 PM Bill Bejeck <bbej...@apache.org> wrote: > Thanks for the KIP, this is great improvement. > > I'm +1 to all the comments and changes in the KIP thus far. > > I also like the proposal from Bruno regarding only passing configurations > to KafkaStreams. > Regarding your comment about internal configuration propagation, would > setting the visiblitly on the Topology constructor to package private or > protected solve the issue? > I'm not sure I'm addressing your concern entirely, but if we can simplify > the code paths for passing configuration, I think it's worth some tradeoffs > for the gain in simplicity. > As I'm writing this though I'm wondering if this is indeed possible when > taking the PAPI into consideration. At any rate, I think the KIP can > proceed as is, I just wanted to add my 2cents. > > Regards, > Bill > > > On Mon, Mar 10, 2025 at 11:38 AM Sebastien Viale < > sebastien.vi...@michelin.com> wrote: > > > Hi, > > > > Thank you both for your remarks. > > > > Sophie: > > > > I have added all your proposals to the rejected alternatives. > > > > BC1: > > I agree that we should not create multiple config classes, as avoiding > > that is the main goal of the KIP. > > > > It would be nice to have a single constructor (KafkaStreams) that accepts > > the configuration. However, unless I’m mistaken, this would require > > handling config propagation internally to other components like Topology. > > > > BC2: > > StreamsConfig is indeed immutable, and like Bruno, I believe this is the > > safer approach. > > Best, > > Sébastien > > > > > > De : Bruno Cadonna <cado...@apache.org> > > Date : vendredi, 7 mars 2025 à 18:55 > > À : dev@kafka.apache.org <dev@kafka.apache.org> > > Objet : [EXT] Re: [DISCUSS] KIP-1138: Clean up TopologyConfig and API for > > supplying configs needed by the topology > > Warning External sender Do not click on any links or open any attachments > > unless you trust the sender and know the content is safe. > > > > 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 > > This email was screened for spam and malicious content but exercise > > caution anyway. > > > > > > > > > > > > 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 > > < > > > 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 > > >> > > >> > > > > > >
