Update Default Partition Logic. #22
Conversation
rhysmccaig
commented
Aug 25, 2016
rhysmccaig
commented
- Update default partition to -1 so that use of librdkafka's default partitioner is used when one is not specified (Currently defaults to partition 0.)
- Update documentation to indicate key and partition can be specified when producing a message
Producer should default to using librdkafka's default partitioner, rather than defaulting messages to partition number 0.
|
Pretty new to the contribution thing - let me know if i should do things differently. |
| @@ -415,7 +415,7 @@ ProducerMessage::ProducerMessage(v8::Local<v8::Object> obj, Topic * topic): | |||
| m_is_empty(true) { | |||
| // We have this bad boy now | |||
|
|
|||
| m_partition = GetParameter<int64_t>(obj, "partition", 0); | |||
| m_partition = GetParameter<int64_t>(obj, "partition", -1); | |||
There was a problem hiding this comment.
Perhaps better to use the constant: http://docs.confluent.io/2.0.0/clients/librdkafka/classRdKafka_1_1Topic.html#ab69c18779df1c8e150648e640b159654
There was a problem hiding this comment.
Yeah i was just thinking that. Ill update.
|
Updated to use the RdKafka::Topic::PARTITION_UA constant as the default instead of hardcoding -1. |
|
Sorry will fix that linting error. |
Updated code formatting to comply with linting rules
| @@ -164,7 +164,11 @@ producer.on('ready', function() { | |||
| // converted to a Buffer automatically, but we're being | |||
| // explicit here for the sake of example. | |||
| message: new Buffer('Awesome message'), | |||
|
|
|||
| // for keyed messages, we also specify the key - note that this field is optional | |||
| key: 'Stormwind', | |||
|
Look's good to me. |
|
Merged. Thanks for the PR! |
|
Just as an FYI - when submitting a PR I'd generally prefer the commits get squashed. You can do this using GitHub now has this built in so it's super easy for me to do it on the fly during a merge, but it generally helps anyway :) You can do this by running |
|
Awesome - thanks for the guidance. |
