[GitHub] [kafka] divijvaidya commented on pull request #14176: KAFKA-15295: Add config validation when remote storage is enabled on a topic
divijvaidya commented on PR #14176: URL: https://github.com/apache/kafka/pull/14176#issuecomment-1679415679 Unrelated test failures: ``` [Build / JDK 17 and Scala 2.13 / kafka.server.DynamicBrokerReconfigurationTest.testThreadPoolResize()](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-14176/15/testReport/junit/kafka.server/DynamicBrokerReconfigurationTest/Build___JDK_17_and_Scala_2_13___testThreadPoolResize__/) [Build / JDK 17 and Scala 2.13 / org.apache.kafka.controller.QuorumControllerTest.testBalancePartitionLeaders()](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-14176/15/testReport/junit/org.apache.kafka.controller/QuorumControllerTest/Build___JDK_17_and_Scala_2_13___testBalancePartitionLeaders__/) [Build / JDK 11 and Scala 2.13 / kafka.api.TransactionsTest.testBumpTransactionalEpoch(String).quorum=kraft](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-14176/15/testReport/junit/kafka.api/TransactionsTest/Build___JDK_11_and_Scala_2_13___testBumpTransactionalEpoch_String__quorum_kraft/) [Build / JDK 11 and Scala 2.13 / kafka.server.DynamicBrokerReconfigurationTest.testThreadPoolResize()](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-14176/15/testReport/junit/kafka.server/DynamicBrokerReconfigurationTest/Build___JDK_11_and_Scala_2_13___testThreadPoolResize__/) [Build / JDK 11 and Scala 2.13 / org.apache.kafka.trogdor.coordinator.CoordinatorTest.testTaskRequestWithOldStartMsGetsUpdated()](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-14176/15/testReport/junit/org.apache.kafka.trogdor.coordinator/CoordinatorTest/Build___JDK_11_and_Scala_2_13___testTaskRequestWithOldStartMsGetsUpdated__/) [Build / JDK 20 and Scala 2.13 / kafka.server.DynamicBrokerReconfigurationTest.testThreadPoolResize()](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-14176/15/testReport/junit/kafka.server/DynamicBrokerReconfigurationTest/Build___JDK_20_and_Scala_2_13___testThreadPoolResize__/) [Build / JDK 20 and Scala 2.13 / org.apache.kafka.controller.QuorumControllerTest.testBalancePartitionLeaders()](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-14176/15/testReport/junit/org.apache.kafka.controller/QuorumControllerTest/Build___JDK_20_and_Scala_2_13___testBalancePartitionLeaders__/) ``` -- 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
[GitHub] [kafka] divijvaidya commented on pull request #14176: KAFKA-15295: Add config validation when remote storage is enabled on a topic
divijvaidya commented on PR #14176: URL: https://github.com/apache/kafka/pull/14176#issuecomment-1679031173 @kamalcph tests are still failing [RemoteTopicCrudTest.testCreateRemoteTopicWithValidRetentionTime(String).quorum=kraft](https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-14176/14/testReport/junit/kafka.admin/RemoteTopicCrudTest/Build___JDK_8_and_Scala_2_12___testCreateRemoteTopicWithValidRetentionTime_String__quorum_kraft/) Please check. -- 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
[GitHub] [kafka] divijvaidya commented on pull request #14176: KAFKA-15295: Add config validation when remote storage is enabled on a topic
divijvaidya commented on PR #14176: URL: https://github.com/apache/kafka/pull/14176#issuecomment-1678685831 @kamalcph Tests from this PR are failing in CI such as `RemoteTopicCrudTest.testEnableRemoteLogOnExistingTopicTest`, see: https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-14176/13/testReport/junit/kafka.admin/RemoteTopicCrudTest/Build___JDK_8_and_Scala_2_12___testEnableRemoteLogOnExistingTopicTest_String__quorum_kraft/ -- 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
[GitHub] [kafka] divijvaidya commented on pull request #14176: KAFKA-15295: Add config validation when remote storage is enabled on a topic
divijvaidya commented on PR #14176: URL: https://github.com/apache/kafka/pull/14176#issuecomment-1676930952 accidentally approved this PR earlier, please ignore that. I am still waiting for another revision after the latest round of comments. -- 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
[GitHub] [kafka] divijvaidya commented on pull request #14176: KAFKA-15295: Add config validation when remote storage is enabled on a topic
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: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
