gianm commented on issue #8576: fix http firehose factory leaky connection in 
constructor
URL: https://github.com/apache/incubator-druid/pull/8576#issuecomment-534782159
 
 
   > Hmm, URL doesn't seem to care whether or not it is valid to open the 
connection, see [existing serde 
test](https://github.com/apache/incubator-druid/blob/master/server/src/test/java/org/apache/druid/segment/realtime/firehose/HttpFirehoseFactoryTest.java#L40).
   > 
   > `java.net.URI` is final, so can't easily mock to ensure that 
`toURL().openConnection()` isn't called either, and would be sort of a lame 
test without the context of this issue since the constructor will no longer be 
opening a connection. Any other ideas?
   
   Ah, it seems like the connection actually gets made on the next line (when 
we look for the header), and errors are suppressed. This makes it tough to 
write a non-brittle, non-silly test. Yeah, okay, I guess it's fine to skip the 
test in this case.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to