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

added configurable Kafka retention time #244

Merged
merged 3 commits into from
Nov 20, 2024

Conversation

umkhan123
Copy link

added configurable
kafkaTpRetnTimeMs, "kafka-topic-retention-time-ms", "900000", "Kafka topic retention time in ms, default is 900000 ms i.e 15 minutes")

Copy link
Owner

@sbezverk sbezverk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add value validation logic to be in a sane range. Also introduce a kafka configuration struct, if in future other parameters need to be specified, parameters list of NewKafkaPublisher will not grow indefinitely.

@umkhan123
Copy link
Author

added the validation for retention time and also Kafka config struct.

Copy link
Owner

@sbezverk sbezverk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there, few minor changes and good to go..

pkg/kafka/kafka-publisher.go Outdated Show resolved Hide resolved
pkg/kafka/kafka-publisher.go Outdated Show resolved Hide resolved
pkg/kafka/kafka-publisher.go Outdated Show resolved Hide resolved
pkg/kafka/kafka-publisher.go Outdated Show resolved Hide resolved
pkg/kafka/kafka-publisher.go Show resolved Hide resolved
@umkhan123
Copy link
Author

@sbezverk can you please review

@sbezverk sbezverk merged commit 216d035 into sbezverk:master Nov 20, 2024
2 checks passed
@sbezverk
Copy link
Owner

thank you

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.

2 participants