revert : upgrade to spring boot 2.4.0 #888
Conversation
As kafka-streams are not working
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThe pull request introduces several modifications to the 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: 0
🧹 Outside diff range and nitpick comments (1)
order-service/src/test/java/com/example/orderservice/TestOrderServiceApplication.java (1)
19-22: Document the reason for using system propertyConsider adding a comment explaining why system property is preferred over
withAdditionalProfiles()for profile activation. This helps maintain context for the specific approach chosen during the Spring Boot version revert.+ // Using system property to ensure profile is set before Spring context initialization + // This approach was chosen as part of reverting Spring Boot 2.4.0 to maintain compatibility System.setProperty("spring.profiles.active", AppConstants.PROFILE_LOCAL); SpringApplication.from(OrderServiceApplication::main) .with(ContainersConfig.class) .run(args);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
order-service/pom.xmlis excluded by!**/*.xml
📒 Files selected for processing (3)
order-service/src/test/java/com/example/orderservice/TestOrderServiceApplication.java(1 hunks)order-service/src/test/java/com/example/orderservice/common/ContainersConfig.java(2 hunks)order-service/src/test/java/com/example/orderservice/repositories/OrderRepositoryTest.java(1 hunks)
🔇 Additional comments (4)
order-service/src/test/java/com/example/orderservice/TestOrderServiceApplication.java (1)
19-19: Verify profile activation timing with test containers
The change to use system properties for profile activation might affect the initialization order of test containers. Ensure that the profile is set early enough for proper container configuration.
order-service/src/test/java/com/example/orderservice/common/ContainersConfig.java (2)
24-25:
Review Kafka configuration changes carefully
The changes to the Kafka container configuration are significant:
- Switching to Confluent's Kafka image: While generally more stable, ensure this version (7.7.1) is compatible with your Kafka streams implementation.
- Adding Kraft mode: This changes Kafka's consensus mechanism, which could impact stream processing behavior.
Since this PR aims to fix Kafka streams issues, these changes need careful consideration.
Consider the following alternatives:
- return new KafkaContainer(DockerImageName.parse("confluentinc/cp-kafka").withTag("7.7.1"))
- .withKraft()
+ return new KafkaContainer(DockerImageName.parse("confluentinc/cp-kafka").withTag("6.2.1"))#!/bin/bash
# Search for Kafka streams usage and version in the codebase
rg "kafka-streams" --type xml --type gradle
rg "KStream|StreamsBuilder" --type java -l14-14: Verify testcontainers version compatibility
Since this PR is reverting Spring Boot version due to Kafka streams issues, we should ensure the testcontainers import path is compatible with the target version.
✅ Verification successful
The previous results show inconsistent import paths across services. Let's verify the testcontainers version and Spring Boot version to determine the correct import path for the target version we're reverting to.
Import path change is correct and consistent
The import change from org.testcontainers.kafka.KafkaContainer to org.testcontainers.containers.KafkaContainer is correct. The evidence shows:
- The project is using Spring Boot 3.3.5 and has consistent testcontainers dependencies across services
- Most services (order-service, inventory-service) are already using the new
org.testcontainers.containers.KafkaContainerimport path - Only catalog-service and payment-service are using the older import path, suggesting they need similar updates
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for testcontainers version in build files
find . -type f -name "pom.xml" -o -name "build.gradle" | xargs grep -l "testcontainers"
# Check the import usage across the codebase
rg "org.testcontainers.(kafka|containers).KafkaContainer" --type java
Length of output: 1169
Script:
#!/bin/bash
# Check testcontainers version in pom.xml files
find . -name "pom.xml" -exec grep -l "testcontainers" {} \; -exec grep -A 5 "testcontainers" {} \;
# Check Spring Boot version in order-service
grep -A 3 "<parent>" order-service/pom.xml
Length of output: 7510
order-service/src/test/java/com/example/orderservice/repositories/OrderRepositoryTest.java (1)
32-32: LGTM! The database configuration change aligns with Spring Boot version revert.
The explicit configuration @AutoConfigureTestDatabase(replace = NONE) is appropriate when reverting from Spring Boot 2.4.0, as it ensures consistent test database behavior with earlier versions, particularly important when using TestContainers.
Let's verify the Spring Boot version in the project:
As kafka-streams are not working