KAFKA-2300: Error in controller log when broker tries to rejoin cluster

[fpj]

No description provided.

[asfbot]

kafka-trunk-git-pr #59 FAILURE
Looks like there's a problem with this pull request

[fpj]

The test failures do not seem to be related to this patch.

[asfbot]

kafka-trunk-git-pr #60 SUCCESS
This pull request looks good

[fpj]

Thanks, Ismael. I have made the change you suggested and also fixed some style issues as per your recommendation.

[asfbot]

kafka-trunk-git-pr #68 SUCCESS
This pull request looks good

[guozhangwang]

@fpj Is this patch ready for review now?

[fpj]

it is ready for review, yes. @guozhangwang

[guozhangwang]

I am not familiar with String interpolation in Scala, but just double check if @ijuma 's comment of using $ instead of % is still valid?

[ijuma]

Yes, it is. One needs to use $ and s" on strings that are interpolated. So, this and many other examples have two issues: % is used and the s" is in the string that doesn't need it but not in the one that does. It does seem like @fpj was quite conservative in the line break. I'd probably write many of these as just a single line.

[fpj]

I prefer line breaks over long lines, is that alright?

[guozhangwang]

Myself sometimes over-use line breakers and sometimes do not use at all and write very long lines, I guess depending on what I ate on that day :P, or more likely if I a used larger monitor on that day. I think we do not have a strict rule about when to use line breaks.

Anyways, all others good to me, @fpj do you want to change the % and s" before I merge it?

[fpj]

hi @guozhangwang @ijuma

Thanks for reviewing this patch. I have updated it according to your comments. The two main things I have done are to wrap all calls to sendRequestToBrokers with a try/catch block (previously I was only doing it in the one that caused the bug described in the jira) and to fix the strings as you pointed out. Let me know if this ok.

[asfbot]

kafka-trunk-git-pr #131 SUCCESS
This pull request looks good