rodesai commented on code in PR #17929: URL: https://github.com/apache/kafka/pull/17929#discussion_r1857289699
########## streams/src/main/java/org/apache/kafka/streams/processor/internals/StoreFactory.java: ########## @@ -75,4 +76,79 @@ default void configure(final StreamsConfig config) { boolean isCompatibleWith(StoreFactory storeFactory); + class FactoryWrappingStoreBuilder<T extends StateStore> implements StoreBuilder<T> { Review Comment: This gave me the spins - previously the factories would wrap builders and suppliers so we could have a unified configuration interface. But now we're wrapping a factory in a builder, which is confusing and makes the code harder to reason about because when I have a builder instance it could actually be a factory underneath (not sure if that causes any problems, but it was nice that there was a non-circular hierarchy of wrapping before). Would it be better to instead revert the dsl operators to what they previously used to do (create builders) and create processor instances that return the unwrapped builders, and then wrap them in a factory from `addStateStore`? So, for example `KStreamAggregate` would store a `KeyValueStoreMaterializer` which would return a StoreBuilder like it used to, and then that gets wrapped by `addStateStore`? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org