KAFKA-14595 Move ReassignPartitionsCommand to java#13247
Merged
jolshan merged 106 commits intoapache:trunkfrom Nov 1, 2023
Merged
KAFKA-14595 Move ReassignPartitionsCommand to java#13247jolshan merged 106 commits intoapache:trunkfrom
jolshan merged 106 commits intoapache:trunkfrom
Conversation
nizhikov
changed the title
Contributor
Author
|
This PR is a first part of moving |
Member
|
Is |
Contributor
Author
When move But, I propose to move with small steps and split moving in several changes. It seems new java classes can be keep in |
nizhikov
force-pushed
the
KAFKA-14595
branch
2 times, most recently
from
3aa7274 to
77a363b
Compare
nizhikov
force-pushed
the
KAFKA-14595
branch
from
77a363b to
ffe57e5
Compare
nizhikov
changed the title
Member
|
If you want to do it in small steps, one way is that you introduce the new classes, but you do not update the command to use them. That way you can put them in the right destination from the start. In any case, I'll leave it to @mimaison to say how he'd prefer it. |
Contributor
Author
|
Hello @ijuma
It's more about simplifing review then my personal preferences :) For now, I introduced java classes from |
Contributor
Author
|
@mimaison Please, share your feedback. Are you ready to review and merge current changes (only case classes and options moved to java code) or should I continue work and rewrite the whole command in Java? |
Member
|
I think we should directly move the classes to the tools module instead of temporarily keeping them in core. Since it's one of the large tools, it's ok to first create the new classes in tools and rewrite the tool in a follow up PR as Ismael suggested. |
Contributor
Author
Are you suggesting to perform all moving in one PR? |
Member
We can create the new Java classes in tools and not touch the existing Scala code. Then in a second time, we can rewrite the tool in Java using the new classes and delete all the old Scala code. |
Contributor
Author
Thanks for feedback. OK. I will move new classes to tools and revert scala code changes made in this PR. |
Contributor
Author
|
@mimaison New classes moved to tools. Please, share your opinion - should I continue work on this task and move task logic to java? |
|
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) |
fvaleri
reviewed
Jul 8, 2023
Contributor
Author
|
CI looks OK |
Member
|
@jolshan Before merging you probably want to run the ReassignPartitionsTest and ThrottlingTest system tests to verify the command still works as expected. |
Contributor
Author
|
@mimaison Thanks for the hint. I will run system tests, also. |
Contributor
Author
|
|
Contributor
Author
|
|
Contributor
Author
|
Note, this PR and #14588 are mutual exclusive. |
Contributor
Author
jolshan
approved these changes
Oct 24, 2023
Member
jolshan
left a comment
jolshan
left a comment
There was a problem hiding this comment.
Looks good. I will give Mickael a day or so for any further comments.
Contributor
Author
|
@jolshan Looks like we good to go. Are you ready to merge this PR? |
Contributor
Author
|
Hello @mimaison |
Contributor
Author
|
Thanks all for the help, review and merge! |
k-wall
pushed a commit
to k-wall/kafka
that referenced
this pull request
Nov 21, 2023
This PR is part of apache#13247 It contains changes to rewrite single test in java. Intention is reduce changes in parent PR. Reviewers: Luke Chen <showuon@gmail.com>, Taras Ledkov <tledkov@apache.org>
k-wall
pushed a commit
to k-wall/kafka
that referenced
this pull request
Nov 21, 2023
…he#14456) This PR is part of apache#13247 It contains ReassignPartitionsIntegrationTest rewritten in java. Goal of PR is reduce changes size in main PR. Reviewers: Taras Ledkov <tledkov@apache.org>, Justine Olshan <jolshan@confluent.io>
yyu1993
pushed a commit
to yyu1993/kafka
that referenced
this pull request
Feb 15, 2024
This PR contains changes required to move PartitionReassignmentState class to java code. Reviewers: Mickael Maison <mickael.maison@gmail.com>, Justine Olshan <jolshan@confluent.io>, Federico Valeri <fedevaleri@gmail.com>, Taras Ledkov Taras Ledkov <tledkov@apache.org>, Kamal Chandraprakash<kamal.chandraprakash@gmail.com>,
AnatolyPopov
pushed a commit
to aiven/kafka
that referenced
this pull request
Feb 16, 2024
This PR is part of apache#13247 It contains changes to rewrite single test in java. Intention is reduce changes in parent PR. Reviewers: Luke Chen <showuon@gmail.com>, Taras Ledkov <tledkov@apache.org>
AnatolyPopov
pushed a commit
to aiven/kafka
that referenced
this pull request
Feb 16, 2024
…he#14456) This PR is part of apache#13247 It contains ReassignPartitionsIntegrationTest rewritten in java. Goal of PR is reduce changes size in main PR. Reviewers: Taras Ledkov <tledkov@apache.org>, Justine Olshan <jolshan@confluent.io>
AnatolyPopov
pushed a commit
to aiven/kafka
that referenced
this pull request
Feb 16, 2024
This PR contains changes required to move PartitionReassignmentState class to java code. Reviewers: Mickael Maison <mickael.maison@gmail.com>, Justine Olshan <jolshan@confluent.io>, Federico Valeri <fedevaleri@gmail.com>, Taras Ledkov Taras Ledkov <tledkov@apache.org>, Kamal Chandraprakash<kamal.chandraprakash@gmail.com>,
Cerchie
pushed a commit
to Cerchie/kafka
that referenced
this pull request
Feb 22, 2024
This PR is part of apache#13247 It contains changes to rewrite single test in java. Intention is reduce changes in parent PR. Reviewers: Luke Chen <showuon@gmail.com>, Taras Ledkov <tledkov@apache.org>
menghe999
reviewed
Aug 6, 2024
| * | ||
| * @return A result that is useful for testing. | ||
| */ | ||
| static VerifyAssignmentResult verifyAssignment(Admin adminClient, |
There was a problem hiding this comment.
Hi @nizhikov, I found many methods are static methods (which means default permission), our application uses the function of partition assignment(not script), and we are using the partition assignment in our app by scala API. but now we can't access these static methods.
verifyAssignment(org.apache.kafka.clients.admin.Admin, java.lang.String, java.lang.Boolean)' is not public in 'org.apache.kafka.tools.reassign.ReassignPartitionsCommand'. Cannot be accessed from outside package
Contributor
Author
There was a problem hiding this comment.
Yes. It's an expected side effect.
Methods of this class are not public API and can be changed without any restrictions.
I advice you to modify your code.
|
chia7712
pushed a commit
that referenced
this pull request
Jun 24, 2025
see https://github.com/apache/kafka/blob/9570c67b8c4ed1a4c3888511adad58d9b3a8bc0f/core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala#L1208 During the rewrite for [KAFKA-14595](#13247), the relevant condition was omitted. Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
chia7712
pushed a commit
that referenced
this pull request
Jun 24, 2025
see https://github.com/apache/kafka/blob/9570c67b8c4ed1a4c3888511adad58d9b3a8bc0f/core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala#L1208 During the rewrite for [KAFKA-14595](#13247), the relevant condition was omitted. Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
chia7712
pushed a commit
that referenced
this pull request
Jun 24, 2025
see https://github.com/apache/kafka/blob/9570c67b8c4ed1a4c3888511adad58d9b3a8bc0f/core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala#L1208 During the rewrite for [KAFKA-14595](#13247), the relevant condition was omitted. Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
jiafu1115
pushed a commit
to jiafu1115/kafka
that referenced
this pull request
Jul 2, 2025
see https://github.com/apache/kafka/blob/9570c67b8c4ed1a4c3888511adad58d9b3a8bc0f/core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala#L1208 During the rewrite for [KAFKA-14595](apache#13247), the relevant condition was omitted. Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
jiafu1115
pushed a commit
to jiafu1115/kafka
that referenced
this pull request
Jul 3, 2025
see https://github.com/apache/kafka/blob/9570c67b8c4ed1a4c3888511adad58d9b3a8bc0f/core/src/main/scala/kafka/admin/ReassignPartitionsCommand.scala#L1208 During the rewrite for [KAFKA-14595](apache#13247), the relevant condition was omitted. Reviewers: Chia-Ping Tsai <chia7712@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR contains changes required to move
PartitionReassignmentStateclass to java code.Committer Checklist (excluded from commit message)