Github user justinleet commented on a diff in the pull request:
https://github.com/apache/metron/pull/600#discussion_r120434003
--- Diff:
metron-platform/metron-common/src/main/java/org/apache/metron/common/utils/KafkaUtils.java
---
@@ -68,12 +68,14 @@
return ret;
}
- public List<String> fromEndpoint(String url) throws URISyntaxException {
+ public List<String> fromEndpoint(String url){
List<String> ret = new ArrayList<>();
if(url != null) {
- URI uri = new URI(url);
- int port = uri.getPort();
- ret.add(uri.getHost() + ((port > 0)?(":" + port):""));
+ Iterable<String> splits = Splitter.on("//").split(url);
+ if(Iterables.size(splits) == 2) {
+ String hostPort = Iterables.getLast(splits);
+ ret.add(hostPort);
--- End diff --
@mattf-horton You're absolutely not being a PITA. Honestly, I tried to get
a solution to a known problem out a little too quickly because it was actually
experienced, so I'm actually pretty happy you're double checking me on this so
something half baked didn't go in.
So looking back at this a bit more fresh, this only gets called from
`getBrokersFromZookeeper`. The fact that's it's public isn't reflective of
it's use (I'd probably argue package private, so it can be unit tested, or just
better unit tested in general).
In practice, these aren't general URLs. They're specifically from
https://cwiki.apache.org/confluence/display/KAFKA/Kafka+data+structures+in+Zookeeper
e.g.
```
  "endpoints": ["PLAINTEXT://host1:9092", "SSL://host1:9093"]
```
So it's exactly protocol://host[:port] (to the best of my knowledge). So
while the URI solution is probably technically better (outside of the
underscore issue), fixing the split should be perfectly fine, if a bit kludgier.
Looking back at it, I'm pretty sure I didn't hit the right code path to
trigger it in testing. I'm going to adjust the unit tests to actually do
something more useful, so we avoid these issues in the future. I'm also going
to add a comment, so it's very explicit what the actual assumptions made by
that function are.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
