gharris1727 commented on code in PR #16199:
URL: https://github.com/apache/kafka/pull/16199#discussion_r1626609501


##########
storage/src/main/java/org/apache/kafka/server/log/remote/storage/RemoteLogManagerConfig.java:
##########
@@ -348,280 +346,124 @@ public final class RemoteLogManagerConfig {
                                  
REMOTE_LOG_MANAGER_FETCH_QUOTA_WINDOW_SIZE_SECONDS_DOC);
     }
 
-    private final boolean enableRemoteStorageSystem;
-    private final String remoteStorageManagerClassName;
-    private final String remoteStorageManagerClassPath;
-    private final String remoteLogMetadataManagerClassName;
-    private final String remoteLogMetadataManagerClassPath;
-    private final long remoteLogIndexFileCacheTotalSizeBytes;
-    private final int remoteLogManagerThreadPoolSize;
-    private final int remoteLogManagerCopierThreadPoolSize;
-    private final int remoteLogManagerExpirationThreadPoolSize;
-    private final long remoteLogManagerTaskIntervalMs;
-    private final long remoteLogManagerTaskRetryBackoffMs;
-    private final long remoteLogManagerTaskRetryBackoffMaxMs;
-    private final double remoteLogManagerTaskRetryJitter;
-    private final int remoteLogReaderThreads;
-    private final int remoteLogReaderMaxPendingTasks;
-    private final String remoteStorageManagerPrefix;
-    private final HashMap<String, Object> remoteStorageManagerProps;
-    private final String remoteLogMetadataManagerPrefix;
-    private final HashMap<String, Object> remoteLogMetadataManagerProps;
-    private final String remoteLogMetadataManagerListenerName;
-    private final int remoteLogMetadataCustomMetadataMaxBytes;
-    private final long remoteLogManagerCopyMaxBytesPerSecond;
-    private final int remoteLogManagerCopyNumQuotaSamples;
-    private final int remoteLogManagerCopyQuotaWindowSizeSeconds;
-    private final long remoteLogManagerFetchMaxBytesPerSecond;
-    private final int remoteLogManagerFetchNumQuotaSamples;
-    private final int remoteLogManagerFetchQuotaWindowSizeSeconds;
-
-    public RemoteLogManagerConfig(AbstractConfig config) {
-        this(config.getBoolean(REMOTE_LOG_STORAGE_SYSTEM_ENABLE_PROP),
-             config.getString(REMOTE_STORAGE_MANAGER_CLASS_NAME_PROP),
-             config.getString(REMOTE_STORAGE_MANAGER_CLASS_PATH_PROP),
-             config.getString(REMOTE_LOG_METADATA_MANAGER_CLASS_NAME_PROP),
-             config.getString(REMOTE_LOG_METADATA_MANAGER_CLASS_PATH_PROP),
-             config.getString(REMOTE_LOG_METADATA_MANAGER_LISTENER_NAME_PROP),
-             config.getLong(REMOTE_LOG_INDEX_FILE_CACHE_TOTAL_SIZE_BYTES_PROP),
-             config.getInt(REMOTE_LOG_MANAGER_THREAD_POOL_SIZE_PROP),
-             config.getInt(REMOTE_LOG_MANAGER_COPIER_THREAD_POOL_SIZE_PROP),
-             
config.getInt(REMOTE_LOG_MANAGER_EXPIRATION_THREAD_POOL_SIZE_PROP),
-             config.getLong(REMOTE_LOG_MANAGER_TASK_INTERVAL_MS_PROP),
-             config.getLong(REMOTE_LOG_MANAGER_TASK_RETRY_BACK_OFF_MS_PROP),
-             
config.getLong(REMOTE_LOG_MANAGER_TASK_RETRY_BACK_OFF_MAX_MS_PROP),
-             config.getDouble(REMOTE_LOG_MANAGER_TASK_RETRY_JITTER_PROP),
-             config.getInt(REMOTE_LOG_READER_THREADS_PROP),
-             config.getInt(REMOTE_LOG_READER_MAX_PENDING_TASKS_PROP),
-             config.getInt(REMOTE_LOG_METADATA_CUSTOM_METADATA_MAX_BYTES_PROP),
-             config.getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP),
-             config.getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP) != 
null
-                 ? 
config.originalsWithPrefix(config.getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP))
-                 : Collections.emptyMap(),
-             config.getString(REMOTE_LOG_METADATA_MANAGER_CONFIG_PREFIX_PROP),
-             config.getString(REMOTE_LOG_METADATA_MANAGER_CONFIG_PREFIX_PROP) 
!= null
-                 ? 
config.originalsWithPrefix(config.getString(REMOTE_LOG_METADATA_MANAGER_CONFIG_PREFIX_PROP))
-                 : Collections.emptyMap(),
-            config.getLong(REMOTE_LOG_MANAGER_COPY_MAX_BYTES_PER_SECOND_PROP),
-            config.getInt(REMOTE_LOG_MANAGER_COPY_QUOTA_WINDOW_NUM_PROP),
-            
config.getInt(REMOTE_LOG_MANAGER_COPY_QUOTA_WINDOW_SIZE_SECONDS_PROP),
-            config.getLong(REMOTE_LOG_MANAGER_FETCH_MAX_BYTES_PER_SECOND_PROP),
-            config.getInt(REMOTE_LOG_MANAGER_FETCH_QUOTA_WINDOW_NUM_PROP),
-            
config.getInt(REMOTE_LOG_MANAGER_FETCH_QUOTA_WINDOW_SIZE_SECONDS_PROP));
-    }
-
-    // Visible for testing
-    public RemoteLogManagerConfig(boolean enableRemoteStorageSystem,
-                                  String remoteStorageManagerClassName,
-                                  String remoteStorageManagerClassPath,
-                                  String remoteLogMetadataManagerClassName,
-                                  String remoteLogMetadataManagerClassPath,
-                                  String remoteLogMetadataManagerListenerName,
-                                  long remoteLogIndexFileCacheTotalSizeBytes,
-                                  int remoteLogManagerThreadPoolSize,
-                                  int remoteLogManagerCopierThreadPoolSize,
-                                  int remoteLogManagerExpirationThreadPoolSize,
-                                  long remoteLogManagerTaskIntervalMs,
-                                  long remoteLogManagerTaskRetryBackoffMs,
-                                  long remoteLogManagerTaskRetryBackoffMaxMs,
-                                  double remoteLogManagerTaskRetryJitter,
-                                  int remoteLogReaderThreads,
-                                  int remoteLogReaderMaxPendingTasks,
-                                  int remoteLogMetadataCustomMetadataMaxBytes,
-                                  String remoteStorageManagerPrefix,
-                                  Map<String, Object> 
remoteStorageManagerProps, /* properties having keys stripped out with 
remoteStorageManagerPrefix */
-                                  String remoteLogMetadataManagerPrefix,
-                                  Map<String, Object> 
remoteLogMetadataManagerProps, /* properties having keys stripped out with 
remoteLogMetadataManagerPrefix */
-                                  long remoteLogManagerCopyMaxBytesPerSecond,
-                                  int remoteLogManagerCopyNumQuotaSamples,
-                                  int 
remoteLogManagerCopyQuotaWindowSizeSeconds,
-                                  long remoteLogManagerFetchMaxBytesPerSecond,
-                                  int remoteLogManagerFetchNumQuotaSamples,
-                                  int 
remoteLogManagerFetchQuotaWindowSizeSeconds
-    ) {
-        this.enableRemoteStorageSystem = enableRemoteStorageSystem;
-        this.remoteStorageManagerClassName = remoteStorageManagerClassName;
-        this.remoteStorageManagerClassPath = remoteStorageManagerClassPath;
-        this.remoteLogMetadataManagerClassName = 
remoteLogMetadataManagerClassName;
-        this.remoteLogMetadataManagerClassPath = 
remoteLogMetadataManagerClassPath;
-        this.remoteLogIndexFileCacheTotalSizeBytes = 
remoteLogIndexFileCacheTotalSizeBytes;
-        this.remoteLogManagerThreadPoolSize = remoteLogManagerThreadPoolSize;
-        this.remoteLogManagerCopierThreadPoolSize = 
remoteLogManagerCopierThreadPoolSize;
-        this.remoteLogManagerExpirationThreadPoolSize = 
remoteLogManagerExpirationThreadPoolSize;
-        this.remoteLogManagerTaskIntervalMs = remoteLogManagerTaskIntervalMs;
-        this.remoteLogManagerTaskRetryBackoffMs = 
remoteLogManagerTaskRetryBackoffMs;
-        this.remoteLogManagerTaskRetryBackoffMaxMs = 
remoteLogManagerTaskRetryBackoffMaxMs;
-        this.remoteLogManagerTaskRetryJitter = remoteLogManagerTaskRetryJitter;
-        this.remoteLogReaderThreads = remoteLogReaderThreads;
-        this.remoteLogReaderMaxPendingTasks = remoteLogReaderMaxPendingTasks;
-        this.remoteStorageManagerPrefix = remoteStorageManagerPrefix;
-        this.remoteStorageManagerProps = new 
HashMap<>(remoteStorageManagerProps);
-        this.remoteLogMetadataManagerPrefix = remoteLogMetadataManagerPrefix;
-        this.remoteLogMetadataManagerProps = new 
HashMap<>(remoteLogMetadataManagerProps);
-        this.remoteLogMetadataManagerListenerName = 
remoteLogMetadataManagerListenerName;
-        this.remoteLogMetadataCustomMetadataMaxBytes = 
remoteLogMetadataCustomMetadataMaxBytes;
-        this.remoteLogManagerCopyMaxBytesPerSecond = 
remoteLogManagerCopyMaxBytesPerSecond;
-        this.remoteLogManagerCopyNumQuotaSamples = 
remoteLogManagerCopyNumQuotaSamples;
-        this.remoteLogManagerCopyQuotaWindowSizeSeconds = 
remoteLogManagerCopyQuotaWindowSizeSeconds;
-        this.remoteLogManagerFetchMaxBytesPerSecond = 
remoteLogManagerFetchMaxBytesPerSecond;
-        this.remoteLogManagerFetchNumQuotaSamples = 
remoteLogManagerFetchNumQuotaSamples;
-        this.remoteLogManagerFetchQuotaWindowSizeSeconds = 
remoteLogManagerFetchQuotaWindowSizeSeconds;
-    }
-
     public boolean enableRemoteStorageSystem() {
-        return enableRemoteStorageSystem;
+        return getBoolean(REMOTE_LOG_STORAGE_SYSTEM_ENABLE_PROP);
     }
 
     public String remoteStorageManagerClassName() {
-        return remoteStorageManagerClassName;
+        return getString(REMOTE_STORAGE_MANAGER_CLASS_NAME_PROP);
     }
 
     public String remoteStorageManagerClassPath() {
-        return remoteStorageManagerClassPath;
+        return getString(REMOTE_STORAGE_MANAGER_CLASS_PATH_PROP);
     }
 
     public String remoteLogMetadataManagerClassName() {
-        return remoteLogMetadataManagerClassName;
+        return getString(REMOTE_LOG_METADATA_MANAGER_CLASS_NAME_PROP);
     }
 
     public String remoteLogMetadataManagerClassPath() {
-        return remoteLogMetadataManagerClassPath;
+        return getString(REMOTE_LOG_METADATA_MANAGER_CLASS_PATH_PROP);
     }
 
     public long remoteLogIndexFileCacheTotalSizeBytes() {
-        return remoteLogIndexFileCacheTotalSizeBytes;
+        return getLong(REMOTE_LOG_INDEX_FILE_CACHE_TOTAL_SIZE_BYTES_PROP);
     }
 
     public int remoteLogManagerThreadPoolSize() {
-        return remoteLogManagerThreadPoolSize;
+        return getInt(REMOTE_LOG_MANAGER_THREAD_POOL_SIZE_PROP);
     }
 
     public int remoteLogManagerCopierThreadPoolSize() {
-        return remoteLogManagerCopierThreadPoolSize;
+        return getInt(REMOTE_LOG_MANAGER_COPIER_THREAD_POOL_SIZE_PROP);
     }
 
     public int remoteLogManagerExpirationThreadPoolSize() {
-        return remoteLogManagerExpirationThreadPoolSize;
+        return getInt(REMOTE_LOG_MANAGER_EXPIRATION_THREAD_POOL_SIZE_PROP);
     }
 
     public long remoteLogManagerTaskIntervalMs() {
-        return remoteLogManagerTaskIntervalMs;
+        return getLong(REMOTE_LOG_MANAGER_TASK_INTERVAL_MS_PROP);
     }
 
     public long remoteLogManagerTaskRetryBackoffMs() {
-        return remoteLogManagerTaskRetryBackoffMs;
+        return getLong(REMOTE_LOG_MANAGER_TASK_RETRY_BACK_OFF_MS_PROP);
     }
 
     public long remoteLogManagerTaskRetryBackoffMaxMs() {
-        return remoteLogManagerTaskRetryBackoffMaxMs;
+        return getLong(REMOTE_LOG_MANAGER_TASK_RETRY_BACK_OFF_MAX_MS_PROP);
     }
 
     public double remoteLogManagerTaskRetryJitter() {
-        return remoteLogManagerTaskRetryJitter;
+        return getDouble(REMOTE_LOG_MANAGER_TASK_RETRY_JITTER_PROP);
     }
 
     public int remoteLogReaderThreads() {
-        return remoteLogReaderThreads;
+        return getInt(REMOTE_LOG_READER_THREADS_PROP);
     }
 
     public int remoteLogReaderMaxPendingTasks() {
-        return remoteLogReaderMaxPendingTasks;
+        return getInt(REMOTE_LOG_READER_MAX_PENDING_TASKS_PROP);
     }
 
     public String remoteLogMetadataManagerListenerName() {
-        return remoteLogMetadataManagerListenerName;
+        return getString(REMOTE_LOG_METADATA_MANAGER_LISTENER_NAME_PROP);
     }
 
     public int remoteLogMetadataCustomMetadataMaxBytes() {
-        return remoteLogMetadataCustomMetadataMaxBytes;
+        return getInt(REMOTE_LOG_METADATA_CUSTOM_METADATA_MAX_BYTES_PROP);
     }
 
     public String remoteStorageManagerPrefix() {
-        return remoteStorageManagerPrefix;
+        return getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP);
     }
 
     public String remoteLogMetadataManagerPrefix() {
-        return remoteLogMetadataManagerPrefix;
+        return getString(REMOTE_LOG_METADATA_MANAGER_CONFIG_PREFIX_PROP);
     }
 
     public Map<String, Object> remoteStorageManagerProps() {
-        return Collections.unmodifiableMap(remoteStorageManagerProps);
+        return 
Collections.unmodifiableMap(getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP)
 != null
+                ? 
originalsWithPrefix(getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP))
+                : Collections.emptyMap());
     }
 
     public Map<String, Object> remoteLogMetadataManagerProps() {
-        return Collections.unmodifiableMap(remoteLogMetadataManagerProps);
+        return 
Collections.unmodifiableMap(getString(REMOTE_LOG_METADATA_MANAGER_CONFIG_PREFIX_PROP)
 != null
+                ? 
originalsWithPrefix(getString(REMOTE_LOG_METADATA_MANAGER_CONFIG_PREFIX_PROP))
+                : Collections.emptyMap());
     }
 
     public long remoteLogManagerCopyMaxBytesPerSecond() {
-        return remoteLogManagerCopyMaxBytesPerSecond;
+        return getLong(REMOTE_LOG_MANAGER_COPY_MAX_BYTES_PER_SECOND_PROP);
     }
 
     public int remoteLogManagerCopyNumQuotaSamples() {
-        return remoteLogManagerCopyNumQuotaSamples;
+        return getInt(REMOTE_LOG_MANAGER_COPY_QUOTA_WINDOW_NUM_PROP);
     }
 
     public int remoteLogManagerCopyQuotaWindowSizeSeconds() {
-        return remoteLogManagerCopyQuotaWindowSizeSeconds;
+        return getInt(REMOTE_LOG_MANAGER_COPY_QUOTA_WINDOW_SIZE_SECONDS_PROP);
     }
 
     public long remoteLogManagerFetchMaxBytesPerSecond() {
-        return remoteLogManagerFetchMaxBytesPerSecond;
+        return getLong(REMOTE_LOG_MANAGER_FETCH_MAX_BYTES_PER_SECOND_PROP);
     }
 
     public int remoteLogManagerFetchNumQuotaSamples() {
-        return remoteLogManagerFetchNumQuotaSamples;
+        return getInt(REMOTE_LOG_MANAGER_FETCH_QUOTA_WINDOW_NUM_PROP);
     }
 
     public int remoteLogManagerFetchQuotaWindowSizeSeconds() {
-        return remoteLogManagerFetchQuotaWindowSizeSeconds;
+        return getInt(REMOTE_LOG_MANAGER_FETCH_QUOTA_WINDOW_SIZE_SECONDS_PROP);
     }
 
-
-    @Override
-    public boolean equals(Object o) {
-        if (this == o) return true;
-        if (!(o instanceof RemoteLogManagerConfig)) return false;
-        RemoteLogManagerConfig that = (RemoteLogManagerConfig) o;
-        return enableRemoteStorageSystem == that.enableRemoteStorageSystem
-                && remoteLogIndexFileCacheTotalSizeBytes == 
that.remoteLogIndexFileCacheTotalSizeBytes
-                && remoteLogManagerThreadPoolSize == 
that.remoteLogManagerThreadPoolSize
-                && remoteLogManagerCopierThreadPoolSize == 
that.remoteLogManagerCopierThreadPoolSize
-                && remoteLogManagerExpirationThreadPoolSize == 
that.remoteLogManagerExpirationThreadPoolSize
-                && remoteLogManagerTaskIntervalMs == 
that.remoteLogManagerTaskIntervalMs
-                && remoteLogManagerTaskRetryBackoffMs == 
that.remoteLogManagerTaskRetryBackoffMs
-                && remoteLogManagerTaskRetryBackoffMaxMs == 
that.remoteLogManagerTaskRetryBackoffMaxMs
-                && remoteLogManagerTaskRetryJitter == 
that.remoteLogManagerTaskRetryJitter
-                && remoteLogReaderThreads == that.remoteLogReaderThreads
-                && remoteLogReaderMaxPendingTasks == 
that.remoteLogReaderMaxPendingTasks
-                && remoteLogMetadataCustomMetadataMaxBytes == 
that.remoteLogMetadataCustomMetadataMaxBytes
-                && Objects.equals(remoteStorageManagerClassName, 
that.remoteStorageManagerClassName)
-                && Objects.equals(remoteStorageManagerClassPath, 
that.remoteStorageManagerClassPath)
-                && Objects.equals(remoteLogMetadataManagerClassName, 
that.remoteLogMetadataManagerClassName)
-                && Objects.equals(remoteLogMetadataManagerClassPath, 
that.remoteLogMetadataManagerClassPath)
-                && Objects.equals(remoteLogMetadataManagerListenerName, 
that.remoteLogMetadataManagerListenerName)
-                && Objects.equals(remoteStorageManagerProps, 
that.remoteStorageManagerProps)
-                && Objects.equals(remoteLogMetadataManagerProps, 
that.remoteLogMetadataManagerProps)
-                && Objects.equals(remoteStorageManagerPrefix, 
that.remoteStorageManagerPrefix)
-                && Objects.equals(remoteLogMetadataManagerPrefix, 
that.remoteLogMetadataManagerPrefix)
-                && remoteLogManagerCopyMaxBytesPerSecond == 
that.remoteLogManagerCopyMaxBytesPerSecond
-                && remoteLogManagerCopyNumQuotaSamples == 
that.remoteLogManagerCopyNumQuotaSamples
-                && remoteLogManagerCopyQuotaWindowSizeSeconds == 
that.remoteLogManagerCopyQuotaWindowSizeSeconds
-                && remoteLogManagerFetchMaxBytesPerSecond == 
that.remoteLogManagerFetchMaxBytesPerSecond
-                && remoteLogManagerFetchNumQuotaSamples == 
that.remoteLogManagerFetchNumQuotaSamples
-                && remoteLogManagerFetchQuotaWindowSizeSeconds == 
that.remoteLogManagerFetchQuotaWindowSizeSeconds;
+    public RemoteLogManagerConfig(Map<?, ?> props) {
+        this(props, false);
     }
 
-    @Override
-    public int hashCode() {
-        return Objects.hash(enableRemoteStorageSystem, 
remoteStorageManagerClassName, remoteStorageManagerClassPath,
-                            remoteLogMetadataManagerClassName, 
remoteLogMetadataManagerClassPath, remoteLogMetadataManagerListenerName,
-                            remoteLogMetadataCustomMetadataMaxBytes, 
remoteLogIndexFileCacheTotalSizeBytes, remoteLogManagerThreadPoolSize,
-                            remoteLogManagerCopierThreadPoolSize, 
remoteLogManagerExpirationThreadPoolSize, remoteLogManagerTaskIntervalMs,
-                            remoteLogManagerTaskRetryBackoffMs, 
remoteLogManagerTaskRetryBackoffMaxMs, remoteLogManagerTaskRetryJitter,
-                            remoteLogReaderThreads, 
remoteLogReaderMaxPendingTasks, remoteStorageManagerProps, 
remoteLogMetadataManagerProps,
-                            remoteStorageManagerPrefix, 
remoteLogMetadataManagerPrefix, remoteLogManagerCopyMaxBytesPerSecond,
-                            remoteLogManagerCopyNumQuotaSamples, 
remoteLogManagerCopyQuotaWindowSizeSeconds, 
remoteLogManagerFetchMaxBytesPerSecond,
-                            remoteLogManagerFetchNumQuotaSamples, 
remoteLogManagerFetchQuotaWindowSizeSeconds);
+    protected RemoteLogManagerConfig(Map<?, ?> props, boolean doLog) {

Review Comment:
   This constructor isn't doing anything. You can call super(ConfigDef, Map) in 
the other constructor.
   
   Also don't forget to remove CONFIG_DEF and replace it with a ConfigDef 
configDef() static method.



##########
storage/src/test/java/org/apache/kafka/server/log/remote/storage/RemoteLogManagerConfigTest.java:
##########
@@ -27,171 +25,72 @@
 
 import static 
org.apache.kafka.server.log.remote.storage.RemoteLogManagerConfig.DEFAULT_REMOTE_LOG_METADATA_MANAGER_CLASS_NAME;
 import static 
org.apache.kafka.server.log.remote.storage.RemoteLogManagerConfig.REMOTE_LOG_METADATA_MANAGER_CLASS_NAME_PROP;
+import static 
org.apache.kafka.server.log.remote.storage.RemoteLogManagerConfig.REMOTE_LOG_METADATA_MANAGER_LISTENER_NAME_PROP;
+import static 
org.apache.kafka.server.log.remote.storage.RemoteLogManagerConfig.REMOTE_LOG_STORAGE_SYSTEM_ENABLE_PROP;
+import static 
org.apache.kafka.server.log.remote.storage.RemoteLogManagerConfig.REMOTE_STORAGE_MANAGER_CLASS_PATH_PROP;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotEquals;
 
 public class RemoteLogManagerConfigTest {
 
-    private static class TestConfig extends AbstractConfig {
-        public TestConfig(Map<?, ?> originals) {
-            super(RemoteLogManagerConfig.CONFIG_DEF, originals, true);
-        }
-    }
-
     @ParameterizedTest
     @ValueSource(booleans = {true, false})
     public void testValidConfigs(boolean 
useDefaultRemoteLogMetadataManagerClass) {
         String rsmPrefix = "__custom.rsm.";
         String rlmmPrefix = "__custom.rlmm.";
         Map<String, Object> rsmProps = Collections.singletonMap("rsm.prop", 
"val");
         Map<String, Object> rlmmProps = Collections.singletonMap("rlmm.prop", 
"val");
-        RemoteLogManagerConfig expectedRemoteLogManagerConfig = 
getRemoteLogManagerConfig(useDefaultRemoteLogMetadataManagerClass,
-                rsmPrefix,
-                rlmmPrefix,
-                rsmProps,
-                rlmmProps);
 
-        Map<String, Object> props = 
extractProps(expectedRemoteLogManagerConfig);
+        Map<String, Object> props = 
getRLMProps(useDefaultRemoteLogMetadataManagerClass);
         rsmProps.forEach((k, v) -> props.put(rsmPrefix + k, v));
         rlmmProps.forEach((k, v) -> props.put(rlmmPrefix + k, v));
+
+        RemoteLogManagerConfig expectedRemoteLogManagerConfig = new 
RemoteLogManagerConfig(props);
+
         // Removing remote.log.metadata.manager.class.name so that the default 
value gets picked up.
         if (useDefaultRemoteLogMetadataManagerClass) {
             props.remove(REMOTE_LOG_METADATA_MANAGER_CLASS_NAME_PROP);
         }

Review Comment:
   This is such an incredibly silly test now that we're using AbstractConfig. 
Of course the default will be applied if the custom value isn't present in the 
map, that's what AbstractConfig is useful for!
   
   If `useDefaultRemoteLogMetadataManagerClass` is false, then this test 
reduces to `assertEquals(new RemoteLogManagerConfig(props).values(), new 
RemoteLogManagerConfig(props).values());` which is also very low-value.
   
   I think removing the parameterization, and confirming that the RLMC can be 
instantiated once is more than sufficient here.
   
   You should probably add an assertion for the rsmProps and rlmmProps, 
comparing them to the output of the non-trivial methods in the RLMC, since that 
is coverage that we are losing in this refactor.
   
   If you want, you can also add a test that constructs an RLMC with an empty 
configuration, because there are default values specified for all 
configurations.
   
   If you are really generous with your time, you can also add some bad values 
and expect ConfigExceptions. A few of the configurations have NonEmptyString 
validators, so you could verify those are present with tests.



-- 
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

Reply via email to