akshat-lakhera commented on code in PR #20688:
URL: https://github.com/apache/druid/pull/20688#discussion_r4354663376
##########
processing/src/main/java/org/apache/druid/java/util/http/client/pool/ChannelResourceFactory.java:
##########
@@ -89,7 +90,7 @@ public ChannelFuture generate(final String hostname)
log.debug("Generating: %s", hostname);
URL url;
try {
- url = new URL(hostname);
+ url = URI.create(hostname).toURL();
}
catch (MalformedURLException e) {
throw new RuntimeException(e);
Review Comment:
I added the helper as you proposed, as URIs.toURL(String) in
org.apache.druid.java.util.common.URIs, in 8a410a7b11. It wraps the
IllegalArgumentException from URI.create in a MalformedURLException and keeps
the original as the cause. Every place that built a URL from a single string
now calls it, so each site only handles MalformedURLException like the old new
URL code did, and I removed the double catches in HttpPostEmitter and
FileBasedProtobufBytesDecoder. The HostAndPort and URI.resolve sites are
unchanged. I added tests for a valid URL, an invalid string and a relative
path. The forbidden-apis rule pointing at URIs.toURL can go in this PR or a
follow-up, whichever you prefer.
The failing unit test
(PartialQueryableIndexServableBundlesTest.testUseProjectionNamingUnservedProjectionFails)
also fails on master right now, it isn't related to this change.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
