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.

Sign up for GitHub

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

Jump to bottom

Migrate filter_stdout plugin to v0.14 API #1058

Merged
merged 18 commits into from
Aug 1, 2016
Merged

Migrate filter_stdout plugin to v0.14 API #1058

merged 18 commits into from
Aug 1, 2016

Conversation

okkez
Copy link
Contributor

@okkez okkez commented Jun 17, 2016

No description provided.

tagomoris
tagomoris reviewed Jun 23, 2016
View reviewed changes
lib/fluent/plugin/filter_stdout.rb
@@ -31,7 +30,7 @@ class StdoutFilter < Filter
def configure(conf)
super

@formatter = Plugin.new_formatter(@format)
@formatter = Fluent::Plugin.new_formatter(@format)
Copy link
Member

Choose a reason for hiding this comment

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

This plugin should use :formatter plugin helper to instantiate/configure Formatter instance.
But currently, there are no way to keep compatibility of configurations between styles of v0.12 (flat parameters of filter and formatter) and v0.14 (<format> subsection).
I'll add that method and document. Please wait for a while.

@okkez okkez mentioned this pull request Jul 4, 2016
Merged
@tagomoris tagomoris added the pending To be done in the future label Jul 6, 2016
@tagomoris
Copy link
Member

This change depends on #1079

@okkez okkez force-pushed the migrate-v0.14-api-filter_stdout branch from a2102be to c1d6d1c Compare July 29, 2016 03:52
@okkez
Copy link
Contributor Author

okkez commented Jul 29, 2016

Use helpers compat_parameters and inject.
@tagomoris Could you review again?

@tagomoris tagomoris removed the pending To be done in the future label Jul 29, 2016
tagomoris
tagomoris reviewed Jul 29, 2016
View reviewed changes
lib/fluent/plugin/filter_stdout.rb
def configure(conf)
conf['format'] ||= DEFAULT_FORMAT_TYPE
Copy link
Member

Choose a reason for hiding this comment

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

Don't do this operations.
Please use default_type keyword argument of formatter_create method instead.

@tagomoris
Copy link
Member

Using format_config and inject_config is not so bad, but plugin test drivers for v0.14 plugins can receive Element instances for #configure method.
I think it's easier and smarter to build Element object than concatenating configuration strings.

@okkez
Copy link
Contributor Author

okkez commented Jul 29, 2016

OK, I will rewrite tests.

@tagomoris
Copy link
Member

Great. Thank you for contribution!

@tagomoris tagomoris merged commit ab92993 into fluent:master Aug 1, 2016
@okkez okkez deleted the migrate-v0.14-api-filter_stdout branch August 1, 2016 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
v0.14
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@okkez @tagomoris