Adds possibility to read '.gz' files when using local firehose#2394
Adds possibility to read '.gz' files when using local firehose#2394se7entyse7en wants to merge 2 commits intoapache:masterfrom
Conversation
There was a problem hiding this comment.
suggest com.metamx.common.CompressionUtils#isGz
There was a problem hiding this comment.
Also proposing
InputStream inputStream = new FileInputStream(f);
if(CompressionUtils.isGz(f.getName()))
{
inputStream = CompressionUtils.gzipInputStream(inputStream)
}
|
Hi @se7entyse7en, thanks for the PR! Can you add a unit test that confirms this works? |
se7entyse7en
force-pushed
the
feature-local-firehose-gzip
branch
from
5784ded to
1b12ceb
Compare
| rawInputStream.close(); | ||
| } catch (IOException ioe) | ||
| { | ||
| Throwables.propagate(ioe); |
There was a problem hiding this comment.
suggest e.addSuppressed(ioe); throw Throwables.propagate(e);
|
@se7entyse7en it is possible that a certain change may have performance improvements in some areas. But to retain the prior behavior you should return Or submit proper benchmarking showing the impact on heap and performance. |
|
👍 |
| firehoseParser | ||
| ); | ||
| } | ||
| private static final EmittingLogger log = new EmittingLogger(LocalFirehoseFactory.class); |
There was a problem hiding this comment.
it appears you changed spaces to tabs. druid uses 2 spaces for indentation. pls use appropriate formatter as mentioned in https://github.com/druid-io/druid/blob/master/CONTRIBUTING.md
|
@se7entyse7en can we finish this up? |
|
Similar functionality was added in another PR (I don't have the number handy) and then improved in #5586 to handle more formats. |
…#8085 in apache druid)
5 participants
No description provided.