search

logstash-plugins / logstash-input-s3

Apache License 2.0
57 stars 150 forks source link

Commit to add is_gzipped into the mix for making optional checks on compressed files without gz extensions. #181

Open apatnaik14 opened 5 years ago

apatnaik14 commented 5 years ago

Hi Team,

In regards to the issue previously raised under

https://github.com/logstash-plugins/logstash-input-s3/issues/180

I have created a pull request to merge the changes of adding support of gz files even though the file extension may not be .gz. Please review and advise is the above can be merged to master.

Regards, Arpan

Thanks for contributing to Logstash! If you haven't already signed our CLA, here's a handy link: https://www.elastic.co/contributor-agreement/

elasticcla commented 5 years ago

Hi @apatnaik14, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in your Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile?

apatnaik14 commented 5 years ago

CLA signed.

I have updated the email details for verification purposes. Not sure why the build is failing. The error does not correlate to the change I made.

Please advise.

apatnaik14 commented 5 years ago

@yaauie I apologize for the multiple updates, but I was hoping to check if there was anything pending on the above pull request and if the above feature is something that can be merged into master.

yaauie commented 5 years ago

@apatnaik14 while this is assigned to me, it fell to the back burner while we finalised the 7.3.0 release and I'm just picking it up now.

I can see the value in allowing users of this plugin to force gzip decoding regardless of filenames, and appreciate your contribution.

I have created an alternate implementation with #185 that adds decode_gzip => force. I avoided using a boolean value because a default value of false for is_gzipped would imply that gzip-decoding would be disabled entirely which is not the case in implementation.

apatnaik14 commented 5 years ago

@yaauie Thank you so much for the update! I was hoping to check if there was a release plan for the above change or maybe a tentative date on when the change will be available in the master build?