ctubbsii commented on code in PR #4185:
URL: https://github.com/apache/accumulo/pull/4185#discussion_r1464099531
##########
core/src/main/java/org/apache/accumulo/core/util/AddressUtil.java:
##########
@@ -44,7 +43,7 @@ public static HostAndPort parseAddress(String address,
boolean ignoreMissingPort
}
public static HostAndPort parseAddress(String address, int defaultPort) {
- return parseAddress(address, true).withDefaultPort(defaultPort);
+ return HostAndPort.fromString(address).withDefaultPort(defaultPort);
Review Comment:
This changes behavior of this parse method to no longer replace `+` with `:`
before calling `HostAndPort.fromString()`. That could be a problem, depending
on where the address came from.
For example, let's say there's a WAL with a filename like
`/path/to/tserver+port/UUID`. If we extract the tserver part,
`tserverPart=tserver+port`, then call `AddressUtil.parseAddress(tserverPart,
defaultPort)`, then we will get a resulting address that looks like:
`tserver+port:defaultPort`, which is incorrect. This is a hypothetical, but
possible outcome of this change. I haven't scoured the code to see what might
be impacted, but it seems likely that it might be an error somewhere, since the
point of AddressUtil is to help us handle these kinds of things that
HostAndPort wouldn't necessarily do automatically.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
