kamalcph commented on PR #14176: URL: https://github.com/apache/kafka/pull/14176#issuecomment-1672925848
> 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. @divijvaidya @showuon I have added the RemoteTopicCreationTest for topic creation in both Kraft and ZK mode. In Kraft mode, the `KafkaConfigs` are not propagated somewhere the remote storage configs are dropped. Not sure, what I am missing from the test. Could you please take a look and help me unblock? Thanks! -- 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
