You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/09/21 03:09:33 UTC

[GitHub] [kafka] ccding opened a new pull request #11348: MINOR: fix CreateTopic to return the same as DescribeTopic

ccding opened a new pull request #11348:
URL: https://github.com/apache/kafka/pull/11348


   DescribeTopic returns
   
   https://github.com/ccding/kafka/blob/79788ca042330faea7dc736f1f6ceb75b3f4d1d9/core/src/main/scala/kafka/server/ConfigHelper.scala#L45
   ```
   config.originals.asScala.filter(_._2 != null) ++ config.nonInternalValues.asScala
   ```
   so we should do the same for CreateTopic
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] ccding commented on a change in pull request #11348: MINOR: fix CreateTopic to return the same as DescribeTopic

Posted by GitBox <gi...@apache.org>.
ccding commented on a change in pull request #11348:
URL: https://github.com/apache/kafka/pull/11348#discussion_r713470616



##########
File path: core/src/main/scala/kafka/server/ZkAdminManager.scala
##########
@@ -118,7 +118,7 @@ class ZkAdminManager(val config: KafkaConfig,
     metadataAndConfigs.get(topicName).foreach { result =>
       val logConfig = LogConfig.fromProps(LogConfig.extractLogConfigMap(config), configs)
       val createEntry = configHelper.createTopicConfigEntry(logConfig, configs, includeSynonyms = false, includeDocumentation = false)(_, _)
-      val topicConfigs = logConfig.nonInternalValues.asScala.map { case (k, v) =>
+      val topicConfigs = (logConfig.originals.asScala.filter(_._2 != null) ++ logConfig.nonInternalValues.asScala).map { case (k, v) =>

Review comment:
       fixed




-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] junrao commented on pull request #11348: MINOR: fix CreateTopic to return the same as DescribeTopic

Posted by GitBox <gi...@apache.org>.
junrao commented on pull request #11348:
URL: https://github.com/apache/kafka/pull/11348#issuecomment-924462411


   cherry-picked the PR to 3.0 branch too.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] ccding commented on pull request #11348: MINOR: fix CreateTopic to return the same as DescribeTopic

Posted by GitBox <gi...@apache.org>.
ccding commented on pull request #11348:
URL: https://github.com/apache/kafka/pull/11348#issuecomment-924028973


   failed tests are passing on my local run


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] junrao commented on a change in pull request #11348: MINOR: fix CreateTopic to return the same as DescribeTopic

Posted by GitBox <gi...@apache.org>.
junrao commented on a change in pull request #11348:
URL: https://github.com/apache/kafka/pull/11348#discussion_r713413729



##########
File path: core/src/main/scala/kafka/server/ZkAdminManager.scala
##########
@@ -118,7 +118,7 @@ class ZkAdminManager(val config: KafkaConfig,
     metadataAndConfigs.get(topicName).foreach { result =>
       val logConfig = LogConfig.fromProps(LogConfig.extractLogConfigMap(config), configs)
       val createEntry = configHelper.createTopicConfigEntry(logConfig, configs, includeSynonyms = false, includeDocumentation = false)(_, _)
-      val topicConfigs = logConfig.nonInternalValues.asScala.map { case (k, v) =>
+      val topicConfigs = (logConfig.originals.asScala.filter(_._2 != null) ++ logConfig.nonInternalValues.asScala).map { case (k, v) =>

Review comment:
       Could we move ConfigHelper.describeConfigs.allConfigs to ConfigHelper and reuse it here?




-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] junrao merged pull request #11348: MINOR: fix CreateTopic to return the same as DescribeTopic

Posted by GitBox <gi...@apache.org>.
junrao merged pull request #11348:
URL: https://github.com/apache/kafka/pull/11348


   


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org