Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-3658: Fix @EmbeddedKafka for adminTimeout resolution #3664

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

artembilan
Copy link
Member

Fixes: #3658
Issue link: #3658

When @EmbeddedKafka is used with Spring context, the adminTimeout is not resolved. Apparently when adminTimeout was introduced, it was covered only by the EmbeddedKafkaCondition.

  • Extract EmbeddedKafkaBrokerFactory to encapsulate an EmbeddedKafkaBroker creation logic (including the mentioned adminTimeout)
  • Replace the logic in the EmbeddedKafkaCondition and EmbeddedKafkaContextCustomizer with that new EmbeddedKafkaBrokerFactory, essentially, introducing a single place of truth.
  • Pull adminTimeout(int) property to the EmbeddedKafkaBroker interface, making the logic in the EmbeddedKafkaBrokerFactory simpler
  • Add adminTimeout attribute verification into tests for condition, as well as Spring-based

Auto-cherry-pick to 3.2.x

…ution

Fixes: spring-projects#3658
Issue link: spring-projects#3658

When `@EmbeddedKafka` is used with Spring context, the `adminTimeout` is not resolved.
Apparently when `adminTimeout` was introduced, it was covered only by the `EmbeddedKafkaCondition`.

* Extract `EmbeddedKafkaBrokerFactory` to encapsulate an `EmbeddedKafkaBroker` creation logic
(including the mentioned `adminTimeout`)
* Replace the logic in the `EmbeddedKafkaCondition` and `EmbeddedKafkaContextCustomizer`
with that new `EmbeddedKafkaBrokerFactory`, essentially, introducing a single place of truth.
* Pull `adminTimeout(int)` property to the `EmbeddedKafkaBroker` interface,
making the logic in the `EmbeddedKafkaBrokerFactory` simpler
* Add `adminTimeout` attribute verification into tests for condition, as well as Spring-based

**Auto-cherry-pick to `3.2.x`**
@sobychacko sobychacko merged commit 0db6b81 into spring-projects:main Dec 12, 2024
3 checks passed
artembilan added a commit that referenced this pull request Dec 12, 2024
Fixes: #3658
Issue link: #3658

When `@EmbeddedKafka` is used with Spring context, the `adminTimeout` is not resolved.
Apparently when `adminTimeout` was introduced, it was covered only by the `EmbeddedKafkaCondition`.

* Extract `EmbeddedKafkaBrokerFactory` to encapsulate an `EmbeddedKafkaBroker` creation logic
(including the mentioned `adminTimeout`)
* Replace the logic in the `EmbeddedKafkaCondition` and `EmbeddedKafkaContextCustomizer`
with that new `EmbeddedKafkaBrokerFactory`, essentially, introducing a single place of truth.
* Pull `adminTimeout(int)` property to the `EmbeddedKafkaBroker` interface,
making the logic in the `EmbeddedKafkaBrokerFactory` simpler
* Add `adminTimeout` attribute verification into tests for condition, as well as Spring-based

# Conflicts:
#	spring-kafka-test/src/main/java/org/springframework/kafka/test/context/EmbeddedKafkaContextCustomizer.java
#	spring-kafka-test/src/test/java/org/springframework/kafka/test/context/EmbeddedKafkaContextCustomizerTests.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EmbeddedKafkaCustomizer ignores adminTimeout in EmbeddedKafka annotation
2 participants