muralibasani commented on PR #13417:
URL: https://github.com/apache/kafka/pull/13417#issuecomment-1652450786
> _Description_
> This PR does the below.
@fvaleri here is the new PR https://github.com/apache/kafka/pull/14106 There
are a few dependencies on the scala version of BrokerM
muralibasani commented on PR #13417:
URL: https://github.com/apache/kafka/pull/13417#issuecomment-1647813211
> Hi @muralibasani, thanks for the additional work. I left some comments.
>
> As @mimaison, I also think that it would be better to create a dedicated
PR for metadata and stora
muralibasani commented on PR #13417:
URL: https://github.com/apache/kafka/pull/13417#issuecomment-1646626261
> Hi, the behavior is different when I pass an invalid config file:
>
> ```shell
> ### OLD
> $ bin/kafka-storage.sh info -c
~/.local/tmp/kafka/server2/config/log4j.proper
muralibasani commented on PR #13417:
URL: https://github.com/apache/kafka/pull/13417#issuecomment-1646626123
@fvaleri code fixed based on review comments.
- new LogConfig constructor added
- new method to validate zk and broker config
- new test with zk config fail
--
This is an a
muralibasani commented on PR #13417:
URL: https://github.com/apache/kafka/pull/13417#issuecomment-1646547619
> Hi, the behavior is different when I pass an invalid config file:
>
> ```shell
> ### OLD
> $ bin/kafka-storage.sh info -c
~/.local/tmp/kafka/server2/config/log4j.proper
muralibasani commented on PR #13417:
URL: https://github.com/apache/kafka/pull/13417#issuecomment-1645092579
> @muralibasani thanks, this is just a first pass.
>
> Looks like there are still some checkstyle errors to fix and a couple of
errors on KafkaClusterTestKit. Can you also avoi
muralibasani commented on PR #13417:
URL: https://github.com/apache/kafka/pull/13417#issuecomment-1643531272
> Thanks @muralibasani, let me know when you are ready for another review.
@fvaleri made the necessary changes I believe. Pls take a look. Thanks.
--
This is an automated mes
muralibasani commented on PR #13417:
URL: https://github.com/apache/kafka/pull/13417#issuecomment-1641043256
@fvaleri
Managed to remove the core dependency from tools.
If am in the right direction, will add tests, fix conflicts, and several
other minor things.
--
This is an autom
muralibasani commented on PR #13417:
URL: https://github.com/apache/kafka/pull/13417#issuecomment-1565985950
@mimaison any suggestions ?
--
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 specif
muralibasani commented on PR #13417:
URL: https://github.com/apache/kafka/pull/13417#issuecomment-1500525012
> * RaftConfig
@mimaison while continuing on this, moving constants to RaftConfig, creating
classes like MetaProperties, RawMetaProperties looks okish.
But this one Brok
muralibasani commented on PR #13417:
URL: https://github.com/apache/kafka/pull/13417#issuecomment-1480046465
> Thanks for the PR! We really want to avoid having tools depend on core. If
there are methods from core that we want to use, we can move them to
server-common.
If tools modul
muralibasani commented on PR #13417:
URL: https://github.com/apache/kafka/pull/13417#issuecomment-1477601010
@mimaison PR is ready to review.
- Had to modify build.gradle to avoid circular dependency. As formatCommand
is being used in other classes in core module.
- There is an unrela
12 matches
Mail list logo
