-
Notifications
You must be signed in to change notification settings - Fork 2
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
messagebus initial implementation #1
Conversation
Write README documenting the library's usage please |
|
Sure, because we have another priority |
kafka.go
Outdated
|
||
// SetTimeout set listening timeout for publish with response | ||
func (client *KafkaClient) SetTimeout(timeout int) { | ||
if timeout < 10000 { |
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.
don't you think it's better to put this into a const?
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.
sure, I will update it
thanks
topicDetails := make(map[string]*sarama.TopicDetail) | ||
topicDetails[topicName] = topicDetail | ||
request := sarama.CreateTopicsRequest{ | ||
Timeout: time.Second * 15, |
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.
also this one, better to put in a constant
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.
sure, will update it
Thanks
kafka.go
Outdated
// deleteTopic delete kafka topic | ||
func deleteTopic(broker *sarama.Broker, topicName string) error { | ||
request := sarama.DeleteTopicsRequest{ | ||
Timeout: time.Second * 15, |
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.
also this one, better to put in a constant
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.
sure, I will update it
thanks
receivedMessage := unmarshal(consumerMessage) | ||
|
||
// check the request and response message ID should be equal | ||
if messageID != receivedMessage.MessageId { |
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.
is it possible that the receivedMessage is nil?
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 think its not possible because if unmarshal function get an error, it will return empty struct
kafka.go
Outdated
|
||
// currently only support 1 message broker | ||
broker := sarama.NewBroker(brokerList[0]) | ||
_ = broker.Open(config) |
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.
is this an error that you silent? or something else?
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.
you're right, I will update it
thanks
messagebus.go
Outdated
package messagebus | ||
|
||
const ( | ||
GDPR_DeletionAccount string = "GDPR:deletionAccount" |
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.
what is this used for?
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.
this is a topic name, we registered the topic name as a public constant so the client doesn't need to manually set the topic
79ec6d2
to
2ab7e53
Compare
Features added :
please help me to review @thoriqsatriya @banukusuma @rjjatson @desniaak @purbopanambang @rafi-isakh