Skip to content

KAFKA-19592: testGenerateAssignmentWithBootstrapServer uses wrong JSON format#20336

Merged
FrankYang0529 merged 7 commits into
apache:trunkfrom
TaiJuWu:KAFKA-19592
Aug 11, 2025
Merged

KAFKA-19592: testGenerateAssignmentWithBootstrapServer uses wrong JSON format#20336
FrankYang0529 merged 7 commits into
apache:trunkfrom
TaiJuWu:KAFKA-19592

Conversation

@TaiJuWu

@TaiJuWu TaiJuWu commented Aug 11, 2025

Copy link
Copy Markdown
Collaborator

This PR do following:

  1. Use correct json format to test.
  2. make PartitionReassignmentState and VerifyAssignmentResult.java
    become record

Reviewers: TengYao Chi frankvicky@apache.org, Ken Huang
s7133700@gmail.com, PoAn Yang payang@apache.org

@github-actions github-actions Bot added triage PRs from the community tools small Small PRs labels Aug 11, 2025

@FrankYang0529 FrankYang0529 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like PartitionReassignmentState and VerifyAssignmentResult can be records. Could you update it? Thanks.

@github-actions github-actions Bot removed the small Small PRs label Aug 11, 2025

@frankvicky frankvicky left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch.
I have a minor comment for you to consider.

Comment on lines +156 to +161
String topicsToMoveJson = "{\"topics\":\n\t[{\"topic\": \"foo\"}],\n\"version\":1\n}";
var assignment = generateAssignment(admin, topicsToMoveJson, "1,2,3", false);
Map<TopicPartition, List<Integer>> proposedAssignments = assignment.getKey();
String assignmentJson = String.format("{\"version\":1,\"partitions\":" +
"[{\"topic\":\"foo\",\"partition\":0,\"replicas\":%s,\"log_dirs\":[\"any\",\"any\",\"any\"]}" +
"]}", proposedAssignments.get(foo0));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not easy to read. Can we use the text block feature from Java 17?

For example:

            String assignmentJson = String.format("""
                {"version":1,"partitions":\
                [{"topic":"foo","partition":0,"replicas":%s,"log_dirs":["any","any","any"]}\
                ]}""", proposedAssignments.get(foo0));

@FrankYang0529 FrankYang0529 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the patch.

@FrankYang0529 FrankYang0529 merged commit 18045c6 into apache:trunk Aug 11, 2025
24 checks passed
@TaiJuWu TaiJuWu deleted the KAFKA-19592 branch August 11, 2025 12:24
@github-actions github-actions Bot removed the triage PRs from the community label Aug 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants