[GitHub] [kafka] cmccabe commented on a change in pull request #11067: MINOR: log broker configs in KRaft mode
cmccabe commented on a change in pull request #11067: URL: https://github.com/apache/kafka/pull/11067#discussion_r672512638 ## File path: core/src/main/scala/kafka/server/BrokerServer.scala ## @@ -389,6 +389,9 @@ class BrokerServer( // a potentially lengthy recovery-from-unclean-shutdown operation here, if required. metadataListener.startPublishing(metadataPublisher).get() + // Log static broker configurations. + new KafkaConfig(config.originals(), true) Review comment: > 1. Where are we logging this for the zk case? It's logged from DynamicBrokerConfig.initialize (see above stack trace) > 2. Can we expose a method in KafkaConfig to write the configs to a logger or something? Seems a bit cleaner than this approach. Can we do this after 3.0? Due to the inheritance-based design here, there isn't a simple way to expose this without changing the base class. The comment does explain what the line does... ## File path: core/src/main/scala/kafka/server/BrokerServer.scala ## @@ -389,6 +389,9 @@ class BrokerServer( // a potentially lengthy recovery-from-unclean-shutdown operation here, if required. metadataListener.startPublishing(metadataPublisher).get() + // Log static broker configurations. + new KafkaConfig(config.originals(), true) Review comment: > 1. Where are we logging this for the zk case? It's logged from DynamicBrokerConfig.initialize (see above stack trace) > 2. Can we expose a method in KafkaConfig to write the configs to a logger or something? Seems a bit cleaner than this approach. Can we do this after 3.0? Due to the inheritance-based design here, there isn't a simple way to expose this without changing the base class. The comment does explain what the line does... ## File path: core/src/main/scala/kafka/server/BrokerServer.scala ## @@ -389,6 +389,9 @@ class BrokerServer( // a potentially lengthy recovery-from-unclean-shutdown operation here, if required. metadataListener.startPublishing(metadataPublisher).get() + // Log static broker configurations. + new KafkaConfig(config.originals(), true) Review comment: > 1. Where are we logging this for the zk case? In the ZK case, it's logged from DynamicBrokerConfig.initialize (see above stack trace) > 2. Can we expose a method in KafkaConfig to write the configs to a logger or something? Seems a bit cleaner than this approach. Can we do this after 3.0? Due to the inheritance-based design here, there isn't a simple way to expose this without changing the base class. The comment does explain what the line does... ## File path: core/src/main/scala/kafka/server/BrokerServer.scala ## @@ -389,6 +389,9 @@ class BrokerServer( // a potentially lengthy recovery-from-unclean-shutdown operation here, if required. metadataListener.startPublishing(metadataPublisher).get() + // Log static broker configurations. + new KafkaConfig(config.originals(), true) Review comment: Filed https://issues.apache.org/jira/browse/KAFKA-13105 -- 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
[GitHub] [kafka] cmccabe commented on a change in pull request #11067: MINOR: log broker configs in KRaft mode
cmccabe commented on a change in pull request #11067: URL: https://github.com/apache/kafka/pull/11067#discussion_r672512638 ## File path: core/src/main/scala/kafka/server/BrokerServer.scala ## @@ -389,6 +389,9 @@ class BrokerServer( // a potentially lengthy recovery-from-unclean-shutdown operation here, if required. metadataListener.startPublishing(metadataPublisher).get() + // Log static broker configurations. + new KafkaConfig(config.originals(), true) Review comment: > 1. Where are we logging this for the zk case? It's logged from DynamicBrokerConfig.initialize (see above stack trace) > 2. Can we expose a method in KafkaConfig to write the configs to a logger or something? Seems a bit cleaner than this approach. Can we do this after 3.0? Due to the inheritance-based design here, there isn't a simple way to expose this without changing the base class. The comment does explain what the line does... ## File path: core/src/main/scala/kafka/server/BrokerServer.scala ## @@ -389,6 +389,9 @@ class BrokerServer( // a potentially lengthy recovery-from-unclean-shutdown operation here, if required. metadataListener.startPublishing(metadataPublisher).get() + // Log static broker configurations. + new KafkaConfig(config.originals(), true) Review comment: > 1. Where are we logging this for the zk case? It's logged from DynamicBrokerConfig.initialize (see above stack trace) > 2. Can we expose a method in KafkaConfig to write the configs to a logger or something? Seems a bit cleaner than this approach. Can we do this after 3.0? Due to the inheritance-based design here, there isn't a simple way to expose this without changing the base class. The comment does explain what the line does... ## File path: core/src/main/scala/kafka/server/BrokerServer.scala ## @@ -389,6 +389,9 @@ class BrokerServer( // a potentially lengthy recovery-from-unclean-shutdown operation here, if required. metadataListener.startPublishing(metadataPublisher).get() + // Log static broker configurations. + new KafkaConfig(config.originals(), true) Review comment: > 1. Where are we logging this for the zk case? In the ZK case, it's logged from DynamicBrokerConfig.initialize (see above stack trace) > 2. Can we expose a method in KafkaConfig to write the configs to a logger or something? Seems a bit cleaner than this approach. Can we do this after 3.0? Due to the inheritance-based design here, there isn't a simple way to expose this without changing the base class. The comment does explain what the line does... ## File path: core/src/main/scala/kafka/server/BrokerServer.scala ## @@ -389,6 +389,9 @@ class BrokerServer( // a potentially lengthy recovery-from-unclean-shutdown operation here, if required. metadataListener.startPublishing(metadataPublisher).get() + // Log static broker configurations. + new KafkaConfig(config.originals(), true) Review comment: Filed https://issues.apache.org/jira/browse/KAFKA-13105 -- 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
[GitHub] [kafka] cmccabe commented on a change in pull request #11067: MINOR: log broker configs in KRaft mode
cmccabe commented on a change in pull request #11067: URL: https://github.com/apache/kafka/pull/11067#discussion_r672574835 ## File path: core/src/main/scala/kafka/server/BrokerServer.scala ## @@ -389,6 +389,9 @@ class BrokerServer( // a potentially lengthy recovery-from-unclean-shutdown operation here, if required. metadataListener.startPublishing(metadataPublisher).get() + // Log static broker configurations. + new KafkaConfig(config.originals(), true) Review comment: Filed https://issues.apache.org/jira/browse/KAFKA-13105 -- 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
[GitHub] [kafka] cmccabe commented on a change in pull request #11067: MINOR: log broker configs in KRaft mode
cmccabe commented on a change in pull request #11067: URL: https://github.com/apache/kafka/pull/11067#discussion_r672512638 ## File path: core/src/main/scala/kafka/server/BrokerServer.scala ## @@ -389,6 +389,9 @@ class BrokerServer( // a potentially lengthy recovery-from-unclean-shutdown operation here, if required. metadataListener.startPublishing(metadataPublisher).get() + // Log static broker configurations. + new KafkaConfig(config.originals(), true) Review comment: > 1. Where are we logging this for the zk case? In the ZK case, it's logged from DynamicBrokerConfig.initialize (see above stack trace) > 2. Can we expose a method in KafkaConfig to write the configs to a logger or something? Seems a bit cleaner than this approach. Can we do this after 3.0? Due to the inheritance-based design here, there isn't a simple way to expose this without changing the base class. The comment does explain what the line does... -- 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
[GitHub] [kafka] cmccabe commented on a change in pull request #11067: MINOR: log broker configs in KRaft mode
cmccabe commented on a change in pull request #11067: URL: https://github.com/apache/kafka/pull/11067#discussion_r672512638 ## File path: core/src/main/scala/kafka/server/BrokerServer.scala ## @@ -389,6 +389,9 @@ class BrokerServer( // a potentially lengthy recovery-from-unclean-shutdown operation here, if required. metadataListener.startPublishing(metadataPublisher).get() + // Log static broker configurations. + new KafkaConfig(config.originals(), true) Review comment: > 1. Where are we logging this for the zk case? It's logged from DynamicBrokerConfig.initialize (see above stack trace) > 2. Can we expose a method in KafkaConfig to write the configs to a logger or something? Seems a bit cleaner than this approach. Can we do this after 3.0? Due to the inheritance-based design here, there isn't a simple way to expose this without changing the base class. The comment does explain what the line does... ## File path: core/src/main/scala/kafka/server/BrokerServer.scala ## @@ -389,6 +389,9 @@ class BrokerServer( // a potentially lengthy recovery-from-unclean-shutdown operation here, if required. metadataListener.startPublishing(metadataPublisher).get() + // Log static broker configurations. + new KafkaConfig(config.originals(), true) Review comment: > 1. Where are we logging this for the zk case? It's logged from DynamicBrokerConfig.initialize (see above stack trace) > 2. Can we expose a method in KafkaConfig to write the configs to a logger or something? Seems a bit cleaner than this approach. Can we do this after 3.0? Due to the inheritance-based design here, there isn't a simple way to expose this without changing the base class. The comment does explain what the line does... -- 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