divijvaidya commented on code in PR #12199: URL: https://github.com/apache/kafka/pull/12199#discussion_r880303225
########## core/src/main/scala/kafka/server/ConfigAdminManager.scala: ########## @@ -200,11 +200,7 @@ class ConfigAdminManager(nodeId: Int, conf.dynamicConfig.validate(props, !configResource.name().isEmpty) } catch { case e: ApiException => throw e - //KAFKA-13609: InvalidRequestException is not really the right exception here if the - // configuration fails validation. The configuration is still well-formed, but just - // can't be applied. It should probably throw InvalidConfigurationException. However, - // we should probably only change this in a KIP since it has compatibility implications. Review Comment: I second this comment here. This is a user facing change. User's may have an application later handling logic which will break with this change. My suggestion: We should take up this breaking change with 3.3 release and document it in upgrade notes explicitly. You might need a committer (and the community) to chime in on this. Perhaps start an email thread in the dev mailing list? -- 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