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
