Add null check for `logger` #1332

Merged
merged 5 commits into from Dec 6, 2016

Projects

None yet

2 participants

@okkez
Contributor
okkez commented Nov 28, 2016

If plugin uses config_param :name, :string, deprecated: "This is deprecated" then we cannot use the plugin with plugins using v0.12
compatible layer.

@okkez
Contributor
okkez commented Nov 28, 2016 edited

I've added deprecated option to fluent-plugin-grok-parser, but I could not complete it.
Because in_tcp in Fluentd v0.14.9 does not use v0.14 API.

Maybe root cause:

See also

@tagomoris
Member

LGTM.
Can you add any tests about it?

@okkez
Contributor
okkez commented Nov 28, 2016

I've added test and pushed changes.
@tagomoris could you review again?

test/config/test_configurable.rb
+ assert_nil(obj.log)
+ end
+
+ test 'nothing raised if obsoleted parameter is configured' do
@tagomoris
tagomoris Nov 28, 2016 Member

This test case shows that "nothing raised", but test code does "assert_raise". Which one is correct?

@okkez
okkez Nov 28, 2016 Contributor

Wow, I've fixed 🙇

@tagomoris
Member

@okkez could you rebase this branch on master and push again? CI jobs are failing with many reasons which are already solved in other pull-requests.

okkez added some commits Nov 28, 2016
@okkez @okkez okkez Add null check for `logger`
If plugin uses `config_param :name, :string, deprecated: "This is
deprecated"` then we cannot use the plugin with plugins using v0.12
compatible layer.
a580500
@okkez @okkez okkez Add assertion for error log 4cf96bd
@okkez @okkez okkez Add test for logger is nil 12f92c1
@okkez @okkez okkez Fix test description
ae7db9c
@okkez
Contributor
okkez commented Nov 30, 2016

Rebased and pushed.

@okkez okkez Fix assertion
afb251f
@tagomoris
Member

LGTM. Thank you!

@tagomoris tagomoris merged commit 4b1dc5c into fluent:master Dec 6, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment