Allow developer to use system property to override default kafka image for embedded tests #18739
+10
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Have a use for this as well – thanks. |
kfaraz
approved these changes
Nov 12, 2025
Contributor
kfaraz
left a comment
kfaraz
left a comment
There was a problem hiding this comment.
Thanks for adding this, @capistrant !
Left a couple of non blocking suggestions.
| { | ||
| private static final String KAFKA_IMAGE = "apache/kafka:4.0.0"; | ||
| // Offering an opportunity to override can help some local devs whose system struggles to run apache/kafka. Overriding with apache/kafka-native can help. | ||
| private static final String KAFKA_IMAGE = System.getProperty("druid.kafka.test.image", "apache/kafka:4.0.0"); |
Contributor
There was a problem hiding this comment.
Nit: Should we use the name druid.testing.kafka.image to align with the Druid image property name used in DruidContainerResource?
| public class KafkaResource extends TestcontainerResource<KafkaContainer> | ||
| { | ||
| private static final String KAFKA_IMAGE = "apache/kafka:4.0.0"; | ||
| // Offering an opportunity to override can help some local devs whose system struggles to run apache/kafka. Overriding with apache/kafka-native can help. |
Contributor
There was a problem hiding this comment.
Let's convert this to a javadoc.
It would be nice to link it in the class level javadoc too.
Also maybe rephrase to something like:
Suggested change
| // Offering an opportunity to override can help some local devs whose system struggles to run apache/kafka. Overriding with apache/kafka-native can help. | |
| /** | |
| * Kafka Docker image used in embedded tests. The image name is | |
| * read from the system property {@code druid.testing.kafka.image} and | |
| * defaults to {@code apache/kafka}. Environments that cannot run that | |
| * image should set the system property to {@code apache/kafka-native}. | |
| */ |
Contributor
Author
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Some docker runtime setups can struggle to run the Apache Kafka image. Switching to something like Apache kafka-native can work (it works for my apple silicon + colima docker runtime). This change allows using test configurations locally to switch the image without changing source files. It will leave the default to the same image that KafkaResource already uses
apache/kafka:4.0.0Key changed/added classes in this PR
KafkaResourceThis PR has: