Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support data compression in buffer plugins #1172

Merged
merged 21 commits into from
Sep 6, 2016

Conversation

ganmacs
Copy link
Member

@ganmacs ganmacs commented Aug 19, 2016

TODO

  • Add compress true in buffer plugin configuration.
  • Compress a chunk by to_compressed_msgpack_stream in EventStream
  • Decompress a chunk in write_to
  • Add some test

Not for this PR tasks

  • Support data compression in forward protocol

Close #1169

@ganmacs ganmacs force-pushed the support-data-compression-in-buffer branch 2 times, most recently from 3504bec to 04daf34 Compare August 22, 2016 05:14
@ganmacs ganmacs changed the title [WIP] support data compression in buffer plugins Support data compression in buffer plugins Aug 22, 2016
@ganmacs ganmacs changed the title Support data compression in buffer plugins [WIP] Support data compression in buffer plugins Aug 22, 2016
@ganmacs ganmacs force-pushed the support-data-compression-in-buffer branch from 04daf34 to 22c86e8 Compare August 24, 2016 06:01
@ganmacs ganmacs force-pushed the support-data-compression-in-buffer branch 8 times, most recently from 5208911 to 89c2a2c Compare August 26, 2016 05:09
@ganmacs ganmacs changed the title [WIP] Support data compression in buffer plugins Support data compression in buffer plugins Aug 26, 2016
@@ -149,11 +152,31 @@ def open(&block)
raise NotImplementedError, "Implement this method in child class"
end

def write_to(io)
def write_to(io, options = {})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use **kwargs instead of tailing option = {}.

@tagomoris
Copy link
Member

I added review comments for implementations.
@ganmacs Could you update your code? I'll back to here after that for code review and test review.

@ganmacs ganmacs force-pushed the support-data-compression-in-buffer branch from 66f8c18 to 72dd08c Compare August 29, 2016 06:55
@ganmacs
Copy link
Member Author

ganmacs commented Aug 29, 2016

@tagomoris
I updated.

if @chunk.is_a?(IO)
# reset io(@chunk) to read
@chunk.seek(0, IO::SEEK_SET)
decompress('', io: @chunk) # not to call IO#read
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifying blank string for unused argument looks bad API.
Is there a way to use default value for that parameter? Or any other solution? (At least, nil is better for unused parameter value.)

@ganmacs
Copy link
Member Author

ganmacs commented Aug 30, 2016

@tagomoris
I updated.

@ganmacs ganmacs force-pushed the support-data-compression-in-buffer branch from 061b2de to 71970b6 Compare August 30, 2016 05:46
@ganmacs ganmacs force-pushed the support-data-compression-in-buffer branch 3 times, most recently from 3226342 to ac376e1 Compare September 2, 2016 10:09
@ganmacs
Copy link
Member Author

ganmacs commented Sep 2, 2016

@tagomoris
I fixed a bug that occurs when output plugin has #format method
I added a test test/plugin/test_output_as_buffered.rb

@ganmacs ganmacs force-pushed the support-data-compression-in-buffer branch from ac376e1 to 0ed911a Compare September 5, 2016 03:18
@tagomoris
Copy link
Member

Okay, LGTM!

@tagomoris tagomoris merged commit 68d27dd into fluent:master Sep 6, 2016
@ganmacs ganmacs deleted the support-data-compression-in-buffer branch June 8, 2019 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants