kamalcph commented on PR #14116: URL: https://github.com/apache/kafka/pull/14116#issuecomment-1666869894
@clolov @divijvaidya @showuon @Hangleton @satishd 1. This PR contains huge changes but most of them are new POJO classes and specific actions. My suggestion is to review all the changes together, then we can divide this PR into multiple once we agree on the approach taken in this PR to do the integration test. 2. Tiered Storage Integration Test Framework depends upon the `core` module kafka packages, so added a new `import-control-storage.xml` which allows to import `kafka` packages . We can refactor the code (package names) once it gets finalised. 3. Increased the `ClassDataAbstractionCoupling` from 25 to 40 in `checkstyle.xml` to unblock for now. Once the changes are finalised, the `TieredStorageTestBuilder` class can be further divided to address the checkstyle error. 4. Wrote one integration `OffloadAndConsumeFromLeaderTest` test which produces records, expects the segment to be copied to remote storage and asserts that the consumer able to consume from tiered storage. Right now, `trunk` build is not stable and this test is in Failed test. We can use this framework as TDD to fix issues in the source code. 5. Java does not supports named parameter arguments so the `OffloadAndConsumeFromLeaderTest` is not readable. We can improve the readability aspects in the next PR. Please take a look when you get chance! -- 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
