Re: [PR] KAFKA-14585: Move StorageTool to tools [kafka]

2024-04-27 Thread via GitHub
fvaleri commented on code in PR #14847: URL: https://github.com/apache/kafka/pull/14847#discussion_r1580757743 ## metadata/src/main/java/org/apache/kafka/metadata/properties/MetaProperties.java: ## @@ -47,7 +47,7 @@ public final class MetaProperties { /** * The

Re: [PR] KAFKA-14585: Move StorageTool to tools [kafka]

2024-04-26 Thread via GitHub
fvaleri commented on code in PR #14847: URL: https://github.com/apache/kafka/pull/14847#discussion_r1580759331 ## tools/src/main/java/org/apache/kafka/tools/StorageTool.java: ## @@ -0,0 +1,555 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

Re: [PR] KAFKA-14585: Move StorageTool to tools [kafka]

2024-04-26 Thread via GitHub
fvaleri commented on code in PR #14847: URL: https://github.com/apache/kafka/pull/14847#discussion_r1580757743 ## metadata/src/main/java/org/apache/kafka/metadata/properties/MetaProperties.java: ## @@ -47,7 +47,7 @@ public final class MetaProperties { /** * The

Re: [PR] KAFKA-14585: Move StorageTool to tools [kafka]

2024-04-26 Thread via GitHub
fvaleri commented on code in PR #14847: URL: https://github.com/apache/kafka/pull/14847#discussion_r1580741403 ## tools/src/test/java/org/apache/kafka/tools/StorageToolTest.java: ## @@ -0,0 +1,517 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + *

Re: [PR] KAFKA-14585: Move StorageTool to tools [kafka]

2024-04-25 Thread via GitHub
mimaison commented on code in PR #14847: URL: https://github.com/apache/kafka/pull/14847#discussion_r1579785296 ## metadata/src/main/java/org/apache/kafka/metadata/properties/MetaProperties.java: ## @@ -47,7 +47,7 @@ public final class MetaProperties { /** * The

Re: [PR] KAFKA-14585: Move StorageTool to tools [kafka]

2024-04-21 Thread via GitHub
fvaleri commented on PR #14847: URL: https://github.com/apache/kafka/pull/14847#issuecomment-2068086501 @showuon @mimaison I think this is now ready for review. I think now changes are well isolated. There is no code refactoring or Kafka configuration changes, so comparison with the

Re: [PR] KAFKA-14585: Move StorageTool to tools [kafka]

2024-01-16 Thread via GitHub
nizhikov commented on PR #14847: URL: https://github.com/apache/kafka/pull/14847#issuecomment-1893726661 Can we extract changes regarding `LogConfig`, `RĐ°ftConfig` into separate PR to simplify review? -- This is an automated message from the Apache Git Service. To respond to the message,

Re: [PR] KAFKA-14585: Move StorageTool to tools [kafka]

2024-01-16 Thread via GitHub
showuon commented on PR #14847: URL: https://github.com/apache/kafka/pull/14847#issuecomment-1893320028 @fvaleri , I think you could extract the refactor part of code into another PR, and ping me there when ready for review. Thanks. -- This is an automated message from the Apache Git

Re: [PR] KAFKA-14585: Move StorageTool to tools [kafka]

2023-12-31 Thread via GitHub
fvaleri commented on PR #14847: URL: https://github.com/apache/kafka/pull/14847#issuecomment-1872986366 Rebased. Waiting for review. Thanks. @showuon @mimaison @cmccabe -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub

Re: [PR] KAFKA-14585: Move StorageTool to tools [kafka]

2023-12-13 Thread via GitHub
showuon commented on PR #14847: URL: https://github.com/apache/kafka/pull/14847#issuecomment-1853758861 OK, let's wait until https://issues.apache.org/jira/browse/KAFKA-15853 is merged, or we've got response from Colin. -- This is an automated message from the Apache Git Service. To

Re: [PR] KAFKA-14585: Move StorageTool to tools [kafka]

2023-12-06 Thread via GitHub
fvaleri commented on PR #14847: URL: https://github.com/apache/kafka/pull/14847#issuecomment-1842545770 @showuon it is called when you instantiate the KafkaConfig object, so it's the constructor:

Re: [PR] KAFKA-14585: Move StorageTool to tools [kafka]

2023-12-05 Thread via GitHub
showuon commented on PR #14847: URL: https://github.com/apache/kafka/pull/14847#issuecomment-1842216032 > The tool calls KafkaConfig.validateValues which runs the full set of configuration validations. @fvaleri , sorry, I didn't see where we invoke `KafkaConfig.validateValues` in

Re: [PR] KAFKA-14585: Move StorageTool to tools [kafka]

2023-11-29 Thread via GitHub
fvaleri commented on PR #14847: URL: https://github.com/apache/kafka/pull/14847#issuecomment-1831441198 > The tool calls KafkaConfig.validateValues which runs the full set of configuration validations. Here we only have the ones that makes sense with the tool commands, the rest will be

Re: [PR] KAFKA-14585: Move StorageTool to tools [kafka]

2023-11-27 Thread via GitHub
fvaleri commented on PR #14847: URL: https://github.com/apache/kafka/pull/14847#issuecomment-1828371630 @mimaison @showuon this is big one, but changes are fairly isolated. It works, but there are some open points which I mention in the PR description. Let me know what you think. --

[PR] KAFKA-14585: Move StorageTool to tools [kafka]

2023-11-27 Thread via GitHub
fvaleri opened a new pull request, #14847: URL: https://github.com/apache/kafka/pull/14847 This change move the StorageTool from core to tools module. We have wrapper script, so there is no need to create a redirect object in core (see KIP-906). There is no impact on system tests.