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

Use separate search prefix for gems and additional plugin path #1133

Merged
merged 1 commit into from
Aug 3, 2016
Merged

Use separate search prefix for gems and additional plugin path #1133

merged 1 commit into from
Aug 3, 2016

Conversation

HatsuneMiku3939
Copy link
Contributor

Allow to search plugin from multiple prefix directory.

Related with issue #1131 (v0.14 doesn't load plugins from directory specified by "-p" option)

@repeatedly repeatedly added bug Something isn't working v0.14 labels Aug 1, 2016
@tagomoris
Copy link
Member

With this change, Fluentd seems to load all in_*.rb files from anywhere possible. Is it correct and intentional?
Fluentd should load plugins from directory specified by -p DIR (primal), or gems which have paths like fluent/plugin/in_*.rb.

@HatsuneMiku3939
Copy link
Contributor Author

No. It's not intentional. Thanks for mention it.

It seems to we need two rule of plugin search prefix.

  • for specified directory (not have paths like fluent/plugin/in_*.rb)
  • for gems

@HatsuneMiku3939
Copy link
Contributor Author

This is working as we expect. but not looks good to me.
Do you have an idea to improve this?

@tagomoris
Copy link
Member

Current your implementation is breaking code compatibility of Registry class. It might be used by any 3rd party plugins. It can't be merged.

@tagomoris
Copy link
Member

My idea is to add optional keyword argument (like file_prefix or some better name) to be used only for @paths, earlier than @search_prefix.

@HatsuneMiku3939
Copy link
Contributor Author

@tagomoris
Sorry for breaking change. actually, I didn't realize Registry is opened to 3rd party plugins. Thanks for mention it.

How about this one? HatsuneMiku3939@d57576a

@tagomoris
Copy link
Member

@HatsuneMiku3939 it looks good to me!

@HatsuneMiku3939
Copy link
Contributor Author

@tagomoris
Thanks!

@HatsuneMiku3939 HatsuneMiku3939 changed the title Allow to search plugin from multiple prefix directory Use separate search prefix for gems and additional plugin path Aug 2, 2016
@repeatedly
Copy link
Member

LGTM

@tagomoris tagomoris merged commit f5ae9f3 into fluent:master Aug 3, 2016
@tagomoris tagomoris mentioned this pull request Aug 3, 2016
Closed
@HatsuneMiku3939 HatsuneMiku3939 deleted the issue-1131 branch August 3, 2016 05:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@tagomoris tagomoris

Labels
bug Something isn't working v0.14
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@HatsuneMiku3939 @tagomoris @repeatedly