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

Fix plugin load order. Loading built-in plugins should be last #1410

Merged
merged 1 commit into from
Jan 16, 2017

Conversation

repeatedly
Copy link
Member

Registry now loads built-in plugins first, but it should be last.
Fluentd should prefer 3rd party plugins to overwrite behaviour and keep to use same plugin, e.g. parser plugin.

@repeatedly repeatedly added the bug Something isn't working label Jan 10, 2017
@repeatedly
Copy link
Member Author

@tagomoris Check this patch.

@repeatedly
Copy link
Member Author

Windows environment tests are toooooo unstable.
I re-run AppVeyor test over 20+ times but tests are still failed...

@repeatedly
Copy link
Member Author

@tagomoris ping?

Copy link
Member

@tagomoris tagomoris left a comment

Choose a reason for hiding this comment

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

When some fluentd versions are installed on a ruby runtime, this code looks to find lib of older Fluentd versions rather than running versions. Doesn't it?

@repeatedly
Copy link
Member Author

Which point? fluentd gem is removed from gem search > https://github.com/fluent/fluentd/pull/1410/files#diff-da556f72b0de83b0e659240bf9d879bbR92

@tagomoris
Copy link
Member

Oh, I missed it.
Looks good to me - Of course, it's better to have some tests.

@repeatedly repeatedly merged commit 48526e4 into master Jan 16, 2017
@repeatedly repeatedly deleted the fix-plugin-load-order branch January 16, 2017 02:21
@repeatedly
Copy link
Member Author

yeah, adding test is better but it needs extra gem installation.
It is hard in unit test. This is TODO of fluentd's integration tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants