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

Feature data source kafka topic list + Fix acceptance test #217

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

dttung2905
Copy link
Contributor

Merry Christmas and Happy New Year everyone,

Error: /20 17:24:21 [ERROR] Error connecting to kafka kafka: client has run out of available brokers to talk to (Is your cluster reachable?)

We use tag latest for our docker image (confluentinc/cp-kafka and confluentinc/cp-zookeeper). In the later version kafka.security.auth.SimpleAclAuthorizer was replaced by kafka.security.authorizer.AclAuthorizer which will make kafka throw error and ultimately make all 3 kafka broker goes down

@dttung2905
Copy link
Contributor Author

Hi @Mongey,
Happy new year! Could you help me take a look at this MR ?

@Mongey
Copy link
Owner

Mongey commented Jan 5, 2022

Thanks for starting on this @dttung2905
I think we should call the data source kafka_topics rather than kafka_topic_list and we should change it to return a more complex structure, instead of just a list of strings, e.g. it should return a list of structs similar to the kafka_topic datasource (if possible)

			"name": {
				Type:        schema.TypeString,
				Required:    true,
				Description: "The name of the topic.",
			},
			"partitions": {
				Type:        schema.TypeInt,
				Computed:    true,
				Description: "Number of partitions.",
			},
			"replication_factor": {
				Type:        schema.TypeInt,
				Computed:    true,
				Description: "Number of replicas.",
			},
			"config": {
				Type:        schema.TypeMap,
				Computed:    true,
				Description: "A map of string k/v attributes.",
				Elem:        schema.TypeString,
			},

@dttung2905
Copy link
Contributor Author

Thanks @Mongey for the suggestion. Let me make relevant changes to the MR and update you here

@dttung2905
Copy link
Contributor Author

Hi @Mongey, I have change the returned attributes like you suggested above. Do you mind reviewing it again ?

@dttung2905
Copy link
Contributor Author

Hi @Mongey , just want to revisit this issue whether there is anything I can improve on?

@dttung2905
Copy link
Contributor Author

@Mongey Just to to circle back on this issue for your feedback

@gintek
Copy link

gintek commented Dec 5, 2022

Hi folks, is there any chance to merge this change in near future ?

@dttung2905
Copy link
Contributor Author

Hi @gintek I would love to. Do you know who else could review and merge the MR ?

@dttung2905 dttung2905 force-pushed the feature-data-source-kafka-topic-list branch from e08e5b2 to a6b3925 Compare April 22, 2023 07:21
@dttung2905
Copy link
Contributor Author

Hi @Mongey,
I have rebased my PR . There is a CI failure that is due to invalid your invalid SSH key (link). Could you help to take a look at it ?
Also there is an unrelated CI test failure. I'm not sure whether we need to do something about it as it has been there for a while iinw

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.

3 participants