-
Notifications
You must be signed in to change notification settings - Fork 6
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
adds a Kafka message consumer using the Sarama library #64
base: master
Are you sure you want to change the base?
Conversation
f988828
to
5e91cd5
Compare
5e91cd5
to
cf1ea9f
Compare
cf1ea9f
to
70d1d02
Compare
events/config.go
Outdated
KafkaUsername string `envconfig:"KAFKA_USERNAME" required:"false"` | ||
KafkaPassword string `envconfig:"KAFKA_PASSWORD" required:"false"` | ||
KafkaAlertsConfigTopics []string `envconfig:"KAFKA_ALERTS_CONFIG_TOPICS" default:"alerts"` | ||
KafkaAlertsDataTopics []string `envconfig:"KAFKA_DATA_ALERTS_TOPICS" default:"data.alerts"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like KafkaAlerts(Config|Data)Topics shouldn't be here, but rather in platform..?
I was following the pattern that was already here (see KafkaTopic).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having kafka topic in the config was a mistake because you can have multiple topics per service. We need to be able to specify topic prefix/suffix for operational reasons - e.g. moving processing to a different topic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh, apparently I knew that at some level, as I wan't using these values... I'll just go ahead and remove them.
These are a handful of data structures and behaviors that can be combined to provide flexible processing of Kafka messages. SaramaMessageConsumer is tightly focused on the consumption of Kafka messages. **This is the interface that other packages will want to implement in order to process Kafka messages.** Being a simple interface, it can easily be extended to provide additional behavior, such as retries. One such example is implemented, the NTimesRetryingConsumer. SaramaConsumerGroupHandler implements sarama.ConsumerGroupHandler. The handler is controlled by the consumer group, and is responsible for managing the lifecycle of a SaramaMessageConsumer. It can be extended if the need for more lifecycle support is required. The SaramaEventsConsumer combines a SaramaMessageConsumer and a SaramaConsumerGroupHandler providing them with a simple lifecycle API (the Run() method). This allows other systems (namely platform) to use a very thin adapter layer to control the lifecycle of the consumer. BACK-2554
70d1d02
to
e6945d3
Compare
Have the basics there, but need to: + write tests + test it + manage consumers for the other topics, whether here or somewhere else
These are a handful of data structures and behaviors that can be combined to provide flexible processing of Kafka messages.
SaramaMessageConsumer is tightly focused on the consumption of Kafka messages. This is the interface that other packages will want to implement in order to process Kafka messages. Being a simple interface, it can easily be extended to provide additional behavior, such as retries. One such example is implemented, the NTimesRetryingConsumer.
SaramaConsumerGroupHandler implements sarama.ConsumerGroupHandler. The handler is controlled by the consumer group, and is responsible for managing the lifecycle of a SaramaMessageConsumer. It can be extended if the need for more lifecycle support is required.
The SaramaEventsConsumer combines a SaramaMessageConsumer and a SaramaConsumerGroupHandler providing them with a simple lifecycle API (the Run() method). This allows other systems (namely platform) to use a very thin adapter layer to control the lifecycle of the consumer.
BACK-2554