satishd commented on a change in pull request #11110:
URL: https://github.com/apache/kafka/pull/11110#discussion_r696454031



##########
File path: 
storage/src/main/java/org/apache/kafka/server/log/remote/storage/RemoteLogManagerConfig.java
##########
@@ -253,9 +253,9 @@ public RemoteLogManagerConfig(AbstractConfig config) {
              config.getInt(REMOTE_LOG_READER_THREADS_PROP),
              config.getInt(REMOTE_LOG_READER_MAX_PENDING_TASKS_PROP),
              config.getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP),
-             
config.originalsWithPrefix(config.getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP)),
+             
config.originalsWithPrefix(config.getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP)
 == null ? "" : config.getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP)),

Review comment:
       @ccding It should be non empty based on the validator that is set for 
this config. If an empty string is passed then all the Kafka config properties 
will be passed, which is wrong. 
   There should be null validation instead of passing an empty string as 
mentioned below.  Same for the `REMOTE_LOG_METADATA_MANAGER_CONFIG_PREFIX_PROP` 
check. 
   
   ```
   config.getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP) != null 
                        ? 
config.originalsWithPrefix(config.getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP))
 
                        : Collections.emptyMap()
   ```




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