ableegoldman commented on a change in pull request #11705:
URL: https://github.com/apache/kafka/pull/11705#discussion_r827564609



##########
File path: streams/src/main/java/org/apache/kafka/streams/TopologyConfig.java
##########
@@ -153,6 +166,17 @@ public TopologyConfig(final String topologyName, final 
StreamsConfig globalAppCo
         } else {
             deserializationExceptionHandlerSupplier = () -> 
globalAppConfigs.getConfiguredInstance(DEFAULT_DESERIALIZATION_EXCEPTION_HANDLER_CLASS_CONFIG,
 DeserializationExceptionHandler.class);
         }
+
+        // Override the default store type config no matter it's a named 
topology or not since it should apply to all topologies
+        storeType = getString(DEFAULT_DSL_STORE_CONFIG);
+        log.info("Topology {} is overriding {} to {}", topologyName, 
DEFAULT_DSL_STORE_CONFIG, storeType);

Review comment:
       +1 -- also note that this sort of only makes sense when using the named 
topology feature, as otherwise you don't have multiple topologies and not 
really any reason to set any of these configs differently in the 
TopologyConfigs vs StreamsConfig passed in to the KafkaStreams constructor.
   
   That said, it will definitely happen, so I guess we should check for 
overrides whether the topology is named or not. Maybe you can use the 
`isTopologyOverride` method, and remove the check for `namedTopology ~= null`? 
   
   ie, you can/should use `isTopologyOverride`
   
   




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to