feat : upgrade to spring boot 3.4 and polish tc config #606
Conversation
|
Warning Rate limit exceeded@rajadilipkolli has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 22 minutes and 57 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes in this pull request introduce a new Changes
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
kafka-avro/spring-boot-kafka-avro-producer/src/test/java/com/example/springbootkafkaavro/SpringBootKafkaAvroProducerApplicationIntTests.java (1)
Line range hint
35-43: Consider optimizing test execution and adding cleanupWhile the test implementation is solid, here are some suggestions for improvement:
- The 30-second timeout might be excessive for local testing. Consider making it configurable or reducing it.
- Consider adding test cleanup to prevent message accumulation between test runs.
@Test void contextLoads(CapturedOutput output) throws Exception { this.mockMvc .perform(post("/person/publish").param("name", "junit").param("age", "33")) .andExpect(status().isOk()); - await().atMost(30, SECONDS) + await().atMost(10, SECONDS) // Reduced timeout .untilAsserted( () -> assertThat(output.getOut()).contains("Person received : junit : 33")); } + +@AfterEach +void cleanup() { + // Add cleanup logic for Kafka messages if needed +}kafka-avro/spring-boot-kafka-avro-producer/src/test/java/com/example/springbootkafkaavro/TestSpringBootKafkaAvroProducerApplication.java (1)
6-6: Consider renaming the class to reflect its new roleSince
TestSpringBootKafkaAvroProducerApplicationis now serving as a standard application class rather than a test configuration class, consider renaming it to something likeSpringBootKafkaAvroProducerApplicationTestto better indicate its purpose and avoid potential confusion.kafka-avro/spring-boot-kafka-avro-producer/src/test/java/com/example/springbootkafkaavro/containers/KafkaContainersConfig.java (2)
30-31: Let Spring manage the Kafka container lifecycleExplicitly calling
confluentKafkaContainer.start();within the bean definition may interfere with Spring's lifecycle management. Spring Boot can automatically start and stop Testcontainers when they are defined as beans. Consider removing thestart()call to allow Spring to handle the container lifecycle.
50-51: Let Spring manage the Schema Registry container lifecycleSimilarly, calling
schemaRegistry.start();inside the bean definition bypasses Spring's container management. Remove the explicitstart()call to let Spring Boot manage the Schema Registry container, ensuring proper startup and shutdown aligned with the application context.kafka-avro/spring-boot-kafka-avro-producer/pom.xml (1)
152-152: Update Google Java Format to version1.25.0The Google Java Format version in the
spotless-maven-pluginconfiguration has been updated. Ensure all code is reformatted according to the new formatter rules to maintain code style consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
kafka-avro/spring-boot-kafka-avro-consumer/generated-avro/com/example/springbootkafkaavro/model/Person.java(0 hunks)kafka-avro/spring-boot-kafka-avro-producer/pom.xml(3 hunks)kafka-avro/spring-boot-kafka-avro-producer/src/test/java/com/example/springbootkafkaavro/SpringBootKafkaAvroProducerApplicationIntTests.java(2 hunks)kafka-avro/spring-boot-kafka-avro-producer/src/test/java/com/example/springbootkafkaavro/TestSpringBootKafkaAvroProducerApplication.java(1 hunks)kafka-avro/spring-boot-kafka-avro-producer/src/test/java/com/example/springbootkafkaavro/containers/KafkaContainersConfig.java(1 hunks)kafka-avro/spring-boot-kafka-avro-producer/src/test/java/com/example/springbootkafkaavro/containers/KafkaRaftWithExtraListenersContainer.java(0 hunks)
💤 Files with no reviewable changes (2)
- kafka-avro/spring-boot-kafka-avro-producer/src/test/java/com/example/springbootkafkaavro/containers/KafkaRaftWithExtraListenersContainer.java
- kafka-avro/spring-boot-kafka-avro-consumer/generated-avro/com/example/springbootkafkaavro/model/Person.java
🔇 Additional comments (7)
kafka-avro/spring-boot-kafka-avro-producer/src/test/java/com/example/springbootkafkaavro/SpringBootKafkaAvroProducerApplicationIntTests.java (2)
9-9: LGTM: Clean import of new container configuration
The import of KafkaContainersConfig aligns with the architectural change to centralize container management.
27-27: Verify container initialization in KafkaContainersConfig
The switch to KafkaContainersConfig looks good, but let's verify that it properly initializes both Kafka and Schema Registry containers.
✅ Verification successful
Let me generate another verification script to check for Kafka container initialization since we only see the Schema Registry part.
Container configuration verified successfully
The KafkaContainersConfig properly initializes both required containers:
- Kafka container with proper network configuration and aliases
- Schema Registry container with correct bootstrap configuration and network setup
- Both containers share the same network for communication
- Dynamic property registration is configured for schema registry URL
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify KafkaContainersConfig implementation
# Check if the config class properly initializes both containers
# Look for container initialization
ast-grep --pattern 'class KafkaContainersConfig {
$$$
@Bean
$$$Container $$$() {
$$$
}
$$$
}'
# Verify Schema Registry configuration
rg -A 5 "schemaRegistry" --type java
Length of output: 5352
Script:
#!/bin/bash
# Check for Kafka container initialization
rg -A 10 "KafkaContainer" --type java
# Also check for network setup which is crucial for container communication
rg -A 5 "Network" --type java
Length of output: 54462
kafka-avro/spring-boot-kafka-avro-producer/src/test/java/com/example/springbootkafkaavro/TestSpringBootKafkaAvroProducerApplication.java (1)
3-10: LGTM
The integration of KafkaContainersConfig into the application startup process using SpringApplication.from(...).with(...).run(args); is appropriate and aligns with Spring Boot best practices.
kafka-avro/spring-boot-kafka-avro-producer/src/test/java/com/example/springbootkafkaavro/containers/KafkaContainersConfig.java (1)
54-71: LGTM
The DynamicPropertyRegistrar correctly configures the Schema Registry URLs for both the Kafka producer and consumer. This dynamic approach ensures that the application picks up the correct endpoints based on the Testcontainers' runtime configuration.
kafka-avro/spring-boot-kafka-avro-producer/pom.xml (3)
9-9: Update Spring Boot parent version
The parent version has been updated to 3.4.0. Ensure that all project dependencies and plugins are compatible with this newer version to prevent any runtime issues.
21-21: Verify the downgrade of confluent.version from 7.8.0 to 7.7.2
The Confluent platform version is being downgraded from 7.8.0 to 7.7.2. Verify that this change is intentional and that the older version meets all the application's requirements. Downgrading may reintroduce previously resolved bugs or remove features present in the newer version.
22-22: Update springdoc-openapi version
The springdoc-openapi.version is updated to 2.7.0. Confirm that this version is compatible with Spring Boot 3.4.0 and that there are no breaking changes affecting the API documentation.
| .withListener("tc-kafka:19092") // Internal alias and port | ||
| .withNetwork(network) // Shared network for communication | ||
| .withNetworkAliases("tc-kafka") // Alias to match Schema Registry | ||
| .withReuse(true); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Review the use of .withReuse(true) in testing context
Using .withReuse(true) on the confluentKafkaContainer keeps the container running between test executions. This can lead to shared state across tests and might cause flaky tests or unexpected behaviors. Unless there's a specific need for container reuse, consider removing .withReuse(true) to ensure a clean environment for each test run.
Summary by CodeRabbit
Release Notes
New Features
Personclass for managing user data, including fields for ID, name, and age, with serialization support.KafkaContainersConfigfor automated Kafka and Schema Registry container setup during integration testing.Bug Fixes
Refactor
TestSpringBootKafkaAvroProducerApplicationto a standard application class, removing test-specific configurations.KafkaRaftWithExtraListenersContainerclass to streamline container management.Documentation
pom.xmlto reflect new dependency versions and removed obsolete dependencies.