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

Remove prospector validation as not needed anymore #5537

Merged
merged 1 commit into from
Nov 15, 2017

Conversation

ruflin
Copy link
Contributor

@ruflin ruflin commented Nov 7, 2017

Since the prospector are now registered through init, the validation is not needed anymore as in case a prospector type does not exist, and error is returned anyway. The harvester types are still needed because several harvesters use the log harvester.

@ruflin ruflin requested a review from kvch November 7, 2017 23:01
@kvch
Copy link
Contributor

kvch commented Nov 8, 2017

Right now the following error is presented if the name of the prospector is invalid:

2017/11/08 19:33:26.012726 beat.go:632: CRIT Exiting: Error in initing prospector: Error retrieving factory for prospector 'cica'
Exiting: Error in initing prospector: Error retrieving factory for prospector 'cica'

I wonder if a clearer error message should be shown. For example: "Unknown prospector type".

@ruflin
Copy link
Contributor Author

ruflin commented Nov 9, 2017

@kvch I pushed c25a8dd with a better error message.

@ruflin ruflin force-pushed the remove-prospector-validation branch 2 times, most recently from 8e8bd4d to 2416716 Compare November 14, 2017 04:24
Since the prospector are now registered through init, the validation is not needed anymore as in case a prospector type does not exist, and error is returned anyway. The harvester types are still needed because several harvesters use the log harvester.
@ruflin ruflin force-pushed the remove-prospector-validation branch from 2416716 to 3063c57 Compare November 15, 2017 02:57
Copy link
Contributor

@exekias exekias left a comment

Choose a reason for hiding this comment

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

LGTM

@kvch kvch merged commit 08c92c7 into elastic:master Nov 15, 2017
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.

3 participants