KAFKA-14588 ConfigCommand rewritten to java

[nizhikov]

PR contains ConfigCommand rewritten in java. Due to heavy dependencies on core classes command can't be moved in tools for now. But it can be rewritten in java staying in core module and moved when all dependencies lands in corresponding modules outside of core.

Committer Checklist (excluded from commit message)

• [ ] Verify design and implementation
• [ ] Verify test coverage and CI build status
• [ ] Verify documentation (including upgrade notes)

[nizhikov]

Hello @mimaison @jolshan

It seems like I done with rewriting ConfigCommand code to java. It can't be moved to tools right now, because of dependencies on KafkaConfig and other parts of core, but it passes all tests.

Can you, please, help me with moving dependencies of ConfigCommand to the correct modules.

PR's is here - #15075 #15387

[chia7712]

I feel this migration can be addressed after 4.0 since we can remove zk support. By deleting zk code, DynamicBrokerConfig and KafkaConfig will not be used by ConfigCommand.

[nizhikov]

Hello @chia7712

Thanks for the feedback.

I think we should move step by step. So I prepared two small PR's that move part of the config to the corresponding java classes

PR's is here - https://github.com/apache/kafka/pull/15075 https://github.com/apache/kafka/pull/15387

I think PR's can be merged right now. Please, take a look.

[nizhikov]

Hello @chia7712 @mimaison @jolshan

Right now, we can't move ConfigCommand to the tools because of heavy dependencies on core classes. But, I prepared a PR that rewrites command from scala to java.

I suggest to proceed and merge java version of command in core module. When all dependencies of ConfigCommand will land in corresponding modules we can just mode java code to tools.

WDYT?

[chia7712]

When all dependencies of ConfigCommand will land in corresponding modules we can just mode java code to tools.

That seems be a good solution for now. +1

[nizhikov]

@chia7712 Thanks for the feedback. I wll prepare step by step PR's for tests in the next couple of days.

[mimaison]

I suggest to proceed and merge java version of command in core module.

This can be helpful while re-implementing these classes but I'm not sure we want to merge that to trunk.

The goal is to break down the core module. So if we can't move ConfigCommand to tools now because of some issues, we should focus on resolving these issues first. Solving them may force us to do changes in ConfigCommand. It seems just rewriting the tool in Java while not properly analyzing the dependencies and changes required to move it to tools is ignoring these issues.

[nizhikov]

Hello @mimaison

Thanks for the feedback.

The goal is to break down the core module

Yes, I understand this.

we should focus on resolving these issues first

Yes. But, issues are implemented by the other contributors. I'm here to help with any of the dependency or other part of migration. Do you have some tickets I can take and implement?

Solving them may force us to do changes in ConfigCommand.

It seems better to do it in the java version of command.

In my opinion, It will be better to improve and support java version of code rather then postpone migration until all dependencies will be migrated.

What do you think?

[chia7712]

It seems to me rewriting tools-related tests code has a good reason: we are increase the test coverage for those tests (for example: #15840). Those new test-purposed java code are easy to moved in the future.

Also, the concern about "production" code makes sense to me. Hence, we can stop the rewriting if testing code are done.

[nizhikov]

@chia7712 @mimaison

AFAIK we are going to rewrite all the kafka code to java eventually. We want to split core to the several modules, also.

It seems like this PR conforms both of the goals :). And we will merge it sooner or later.

Not sure I understand the reasons to postpone migration if code are ready and passes all the tests. After migration to java and resolving all the dependencies moving ConfigCommand to tools will be trivial task.

Anyway, It's up to you.

If we postpone rewriting ConfigCommand itself, please, tell me, if we going to proceed with the #15848 and merge remaining ConfigCommand tests?

I will try to find next ticket to move dependencies from core to corresponding modules so if you have one in mind don't hesitate to ping me.

[chia7712]

If we postpone rewriting ConfigCommand itself, please, tell me, if we going to proceed with the https://github.com/apache/kafka/pull/15848 and merge remaining ConfigCommand tests?

I prefer to complete tests migration for following reasons.

1. migrating tests code have less impaction to production code
2. we are increasing the test coverage for those command tools. By migration we can avoid adding more and more scala code

At any rate, @nizhikov you have complete many great migration for kafka. We can postpone rewriting for a while (especially, 3.8.0 is going to be released). Meanwhile, it would be great to take a look at flaky if @nizhikov you have free time. That will be another great contribution to Kafka as those flaky [0] can imply something wrong in our code base. WDYT? I'm working with other contributors to fix those flaky, and it would be nice to have you in this party :)

[0] https://issues.apache.org/jira/issues/?jql=project%20%3D%20KAFKA%20AND%20resolution%20%3D%20Unresolved%20AND%20text%20~%20%22flaky%22%20ORDER%20BY%20priority%20DESC%2C%20updated%20DESC

[nizhikov]

@chia7712 Got it.

Waiting for your feedback regarding #15848

[chia7712]

Waiting for your feedback regarding https://github.com/apache/kafka/pull/15848

will take a look today :)

[nizhikov]

We can postpone rewriting for a while (especially, 3.8.0 is going to be released)

@chia7712 @mimaison

Can you, please, clarify. Are we stop this PR until 3.8 or until all dependencies will be moved out of core?

[chia7712]

Are we stop this PR until 3.8 or until all dependencies will be moved out of core?

short answer: Yes!

Personally, it would be great to address the final purpose - all in java. The production code has too many Scala dependencies which obstructs us from rewriting. I prefer to rewrite the tool-related test code due to "lower" dependencies - 1) we can test tool through command-line arguments. 2) we have new test infra

@nizhikov thanks for all your migration, and the "rewrite" party is not over - for example, https://issues.apache.org/jira/browse/KAFKA-16845 - please feel free to join this after-party :)

[github-actions[bot]]

This PR is being marked as stale since it has not had any activity in 90 days. If you would like to keep this PR alive, please ask a committer for review. If the PR has merge conflicts, please update it with the latest from trunk (or appropriate release branch)

If this PR is no longer valid or desired, please feel free to close it. If no activity occurs in the next 30 days, it will be automatically closed.

[github-actions[bot]]

This PR is being marked as stale since it has not had any activity in 90 days. If you would like to keep this PR alive, please leave a comment asking for a review. If the PR has merge conflicts, update it with the latest from the base branch.

If you are having difficulty finding a reviewer, please reach out on the [mailing list](https://kafka.apache.org/contact).

If this PR is no longer valid or desired, please feel free to close it. If no activity occurs in the next 30 days, it will be automatically closed.

[github-actions[bot]]

This PR has been closed since it has not had any activity in 120 days. If you feel like this was a mistake, or you would like to continue working on it, please feel free to re-open the PR and ask for a review.