kamalcph commented on code in PR #14161: URL: https://github.com/apache/kafka/pull/14161#discussion_r1300319555
########## storage/src/main/java/org/apache/kafka/storage/internals/log/LogConfig.java: ########## @@ -500,22 +500,29 @@ public static void validateBrokerLogConfigValues(Map<?, ?> props, * The default values should be extracted from the KafkaConfig. * @param props The properties to be validated */ - private static void validateTopicLogConfigValues(Map<?, ?> props, - boolean isRemoteLogStorageSystemEnabled) { + public static void validateTopicLogConfigValues(Map<?, ?> props, + boolean isRemoteLogStorageSystemEnabled, + boolean isReceivingConfigFromStore) { validateValues(props); boolean isRemoteLogStorageEnabled = (Boolean) props.get(TopicConfig.REMOTE_LOG_STORAGE_ENABLE_CONFIG); if (isRemoteLogStorageEnabled) { - validateRemoteStorageOnlyIfSystemEnabled(isRemoteLogStorageSystemEnabled); - validateNoRemoteStorageForCompactedTopic(props); - validateRemoteStorageRetentionSize(props); - validateRemoteStorageRetentionTime(props); + validateRemoteStorageOnlyIfSystemEnabled(isRemoteLogStorageSystemEnabled, isReceivingConfigFromStore); + if (!isReceivingConfigFromStore) { + validateNoRemoteStorageForCompactedTopic(props); + validateRemoteStorageRetentionSize(props); + validateRemoteStorageRetentionTime(props); + } } } - private static void validateRemoteStorageOnlyIfSystemEnabled(boolean isRemoteLogStorageSystemEnabled) { + private static void validateRemoteStorageOnlyIfSystemEnabled(boolean isRemoteLogStorageSystemEnabled, boolean isReceivingConfigFromStore) { if (!isRemoteLogStorageSystemEnabled) { - throw new ConfigException("Tiered Storage functionality is disabled in the broker. " + - "Topic cannot be configured with remote log storage."); + if (isReceivingConfigFromStore) { Review Comment: The intention is not to supply the `boolean` to differentiate the cases. IMO, the existing error message should be sufficient: > Tiered Storage functionality is disabled in the broker. Topics cannot be configured with remote log storage. Let's wait for others opinion. cc @showuon @divijvaidya -- 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