[GitHub] [kafka] hachikuji commented on pull request #9967: KAFKA-12236; New meta.properties logic for KIP-500
hachikuji commented on pull request #9967: URL: https://github.com/apache/kafka/pull/9967#issuecomment-770307353 One test failure fixed by https://github.com/apache/kafka/pull/10006. Merging to trunk. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] hachikuji commented on pull request #9967: KAFKA-12236; New meta.properties logic for KIP-500
hachikuji commented on pull request #9967: URL: https://github.com/apache/kafka/pull/9967#issuecomment-770279434 @ijuma Yep, got a little lazy verifying the case class conversion. Hopefully fixed now. We were in fact relying on the `equals`, but I modified the test case. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] hachikuji commented on pull request #9967: KAFKA-12236; New meta.properties logic for KIP-500
hachikuji commented on pull request #9967: URL: https://github.com/apache/kafka/pull/9967#issuecomment-768742967 @chia7712 @ijuma Thanks for the comments thus far. This is ready for another look when you have time. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] hachikuji commented on pull request #9967: KAFKA-12236; New meta.properties logic for KIP-500
hachikuji commented on pull request #9967: URL: https://github.com/apache/kafka/pull/9967#issuecomment-768034713 @ijuma To answer your question, newly generated ClusterIds do indeed use the Base64 encoding. I think it probably needs to be smart enough to accept both versions though. Let me add a test case for this. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] hachikuji commented on pull request #9967: KAFKA-12236; New meta.properties logic for KIP-500
hachikuji commented on pull request #9967: URL: https://github.com/apache/kafka/pull/9967#issuecomment-768034167 Another thing we need to do here is replace `controller.id` and `broker.id` with a single `node.id`. I will get to this tomorrow. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] hachikuji commented on pull request #9967: KAFKA-12236; New meta.properties logic for KIP-500
hachikuji commented on pull request #9967: URL: https://github.com/apache/kafka/pull/9967#issuecomment-768033295 > Pardon me, what did you mean "in each log directory"? I read this PR and it seems to me the new meta.properties must exists in "only one" log directory (the head of directories by default) I guess this is chastisement for the shortage of tests verifying this behavior. I have added some and caught a bug in the process. Perhaps the bug was the source of the confusion. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
