divijvaidya commented on PR #14176: URL: https://github.com/apache/kafka/pull/14176#issuecomment-1672922357
Regarding my previous comment: > where does kraft create topic and alter topic config invoke the validation because we seem to only be changing the controller config validation. It does it in ControllerConfigurationValidator so we should be good with just changing it as you have done in this PR. > Let me get back tomorrow with a better suited test suite We need to add tests at: 1. ControllerConfigurationValidatorTest -> asserts that the validation logic added in controller validator is correct 2. PlaintestAdminIntegrationTest (see: testInvalidIncrementalAlterConfigs) -> asserts that correct exception is propagated to client side for both zk and kraft. We should have two tests here, one for createTopic with invalid config and another for alterConfig for existing topic. In a separate ticket later, we will add tests that validate TS related configuration hierarchy and changes for cluster, broker and topic level. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
