ableegoldman commented on code in PR #14659:
URL: https://github.com/apache/kafka/pull/14659#discussion_r1379396433
##########
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KeyValueStoreMaterializer.java:
##########
@@ -27,26 +31,44 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
/**
* Materializes a key-value store as either a {@link
TimestampedKeyValueStoreBuilder} or a
* {@link VersionedKeyValueStoreBuilder} depending on whether the store is
versioned or not.
*/
-public class KeyValueStoreMaterializer<K, V> {
+public class KeyValueStoreMaterializer<K, V> implements StoreFactory {
private static final Logger LOG =
LoggerFactory.getLogger(KeyValueStoreMaterializer.class);
private final MaterializedInternal<K, V, KeyValueStore<Bytes, byte[]>>
materialized;
+ private final Set<String> connectedProcessorNames = new HashSet<>();
+
+ private Materialized.StoreType defaultStoreType =
Materialized.StoreType.ROCKS_DB;
Review Comment:
nit: I don't love that we define the default store type in this random
hard-to-find location. If we ever wanted to change it going forward I think
this would be easily missed (or at the least, a pain to debug/locate).
Can we at least define a variable for the default defaultStoreType
(so...defaultDefaultStoreType? 😬 ) and put that in a sensible location? I guess
maybe where the storeType itself is defined?
##########
streams/src/main/java/org/apache/kafka/streams/kstream/internals/KeyValueStoreMaterializer.java:
##########
@@ -27,26 +31,44 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
/**
* Materializes a key-value store as either a {@link
TimestampedKeyValueStoreBuilder} or a
* {@link VersionedKeyValueStoreBuilder} depending on whether the store is
versioned or not.
*/
-public class KeyValueStoreMaterializer<K, V> {
+public class KeyValueStoreMaterializer<K, V> implements StoreFactory {
private static final Logger LOG =
LoggerFactory.getLogger(KeyValueStoreMaterializer.class);
private final MaterializedInternal<K, V, KeyValueStore<Bytes, byte[]>>
materialized;
+ private final Set<String> connectedProcessorNames = new HashSet<>();
+
+ private Materialized.StoreType defaultStoreType =
Materialized.StoreType.ROCKS_DB;
Review Comment:
nit: I don't love that we define the default store type in this random
hard-to-find location. If we ever wanted to change it going forward I think
this would be easily missed (or at the least, a pain to debug/locate).
Can we at least define a variable for the default defaultStoreType
(so...defaultDefaultStoreType? 😬 ) and put that in a sensible location? I guess
maybe where the storeType itself is defined?
--
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
