Update Sarama to version 1.30.0 #715
Conversation
|
This change breaks existing Kafka version parsing. For example, "0.10.2" is no longer an acceptable version string, it has to be 0.10.2.0 |
| version, ok := kafkaVersions[kafkaVersion] | ||
| if !ok { | ||
| version, err := sarama.ParseKafkaVersion(kafkaVersion) | ||
| if err != nil { |
There was a problem hiding this comment.
Would it be helpful to include the error in the message so folks know what is wrong?
|
It's something we could fix in Sarama if you think it would be worthwhile? Did you catch this because you were currently using one of the version numbers that omitted a trailing zero? I believe Kafka moved to semver since 1.0.0 so it's only the 0.x.y.z series that would need this coverage. |
|
Yeah, I have a number of clusters that are running older versions. Unfortunate, but such is life when you don't control everything. Been running burrow for years now with the old version (missing the trailing .0), so this was just a bit surprising. I'm fine with the change, if you would prefer not to fix this (sarama approach looks reasonable). Just wanted to raise it as "breaking" change that is probably worth mentioning to users. Or a hybrid approach could be Ok too - try the sarama version lookup, if that fails, fall back to the old version mapping for pre-1.0.0 versions. |
Update Sarama and use built-in ParseKafkaVersion, now that it includes all versions.
@dnwe would you mind taking a quick 👀 at this PR?