Skip to content

Remove superficial Kafka optimization for the sake of clarity#7030

Closed
danielpetisme wants to merge 1 commit intotestcontainers:mainfrom
danielpetisme:chore/cleaning-kafka-module
Closed

Remove superficial Kafka optimization for the sake of clarity#7030
danielpetisme wants to merge 1 commit intotestcontainers:mainfrom
danielpetisme:chore/cleaning-kafka-module

Conversation

@danielpetisme
Copy link
Contributor

The Kafka module is skipping the /etc/confluent/docker/ensure checks for the optimization reasons.

The checks performed in this script are super lightweight and don't worth the code complexity it brings.

Discussions
#7014 (comment)

@eddumelendez
Copy link
Member

look like the example test and examples for kafka fail 👀

@danielpetisme
Copy link
Contributor Author

Have been looking at the failures...

Beyond just variable verification, the ensure script is doing a Zookeeper connectivity check...
https://github.com/confluentinc/kafka-images/blob/7.3.x/kafka/include/etc/confluent/docker/ensure#L23-L29

These 6 lines should be removed only when Kraft enabled && CP < 7.4
I need to find a way to update this file in a comprehensive way...

@danielpetisme
Copy link
Contributor Author

@eddumelendez Actually I think this PR is a bad idea.
The code would be less readable, if it's ok for you I propose to reject this PR.

@eddumelendez
Copy link
Member

feel free to close it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants