[
https://issues.apache.org/jira/browse/METRON-976?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16039354#comment-16039354
]
ASF GitHub Bot commented on METRON-976:
---------------------------------------
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.
> KafkaUtils doesn't handle SASL_PLAINTEXT
> ----------------------------------------
>
> Key: METRON-976
> URL: https://issues.apache.org/jira/browse/METRON-976
> Project: Metron
> Issue Type: Bug
> Reporter: Justin Leet
> Assignee: Justin Leet
>
> Java's URI class throws an error because '_' isn't allowed to be in a URI.
> We'll need to move to a manually split approach.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
