Skip to content

RATIS-2716. The string based ip address should be normalized in SetPriorityCommand - #1610

Merged
szetszwo merged 3 commits into
apache:masterfrom
amaliujia:fix_3
Sep 25, 2026
Merged

szetszwo merged 3 commits into
apache:masterfrom
amaliujia:fix_3

Conversation

@amaliujia

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

The string based ip address should be normalized in SetPriorityCommand. For example, localhost:5 won't be able to match with 127.0.0.1.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/RATIS-2716

How was this patch tested?

Unit test

@amaliujia
amaliujia requested a review from szetszwo September 24, 2026 05:10

@szetszwo szetszwo 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.

@amaliujia , thanks for working on this! Please see the comments inlined.

for (String optionValue : optionValues) {
String[] str = optionValue.split("[|]");
if (str.length < 2) {
throw new IllegalArgumentException("The format of the parameter is wrong");

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.

Let's add the format and the actual value in the message:

        throw new IllegalArgumentException("Invalid option <PEER_HOST:PEER_PORT|PRIORITY>: " + optionValue);

addressPriorityMap = parseAddressPriorityMap(
cl.getOptionValues(PEER_WITH_NEW_PRIORITY_OPTION_NAME));
} catch (IllegalArgumentException e) {
println("The format of the parameter is wrong");

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.

Let's print out the exception:

      e.printStackTrace(getPrintStream());

@szetszwo szetszwo 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.

+1 the change looks good.

@szetszwo
szetszwo merged commit de725fa into apache:master Sep 25, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants