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 error log from runnerfactory as error is returned by API #5085

Merged
merged 1 commit into from
Sep 5, 2017

Conversation

vjsamuel
Copy link
Contributor

@vjsamuel vjsamuel commented Sep 3, 2017

The same error that is being logged is also being returned by the API. it should be the caller's choice to decide if its a debug/info/error level log.

@elasticmachine
Copy link
Collaborator

Can one of the admins verify this patch?

@exekias
Copy link
Contributor

exekias commented Sep 4, 2017

ok to test

@@ -28,7 +27,6 @@ func NewRunnerFactory(outlet channel.Factory, registrar *registrar.Registrar, be
func (r *RunnerFactory) Create(c *common.Config) (cfgfile.Runner, error) {
p, err := New(c, r.outlet, r.beatDone, r.registrar.GetStates())
if err != nil {
logp.Err("Error creating prospector: %s", err)
Copy link
Member

Choose a reason for hiding this comment

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

LGTM. The only problem with the error logged by the runner factory is probably that it's more generic and does not talk about the prospector itself?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Merged it as is as I kind of like producing errors in as few places and just pass them on. We can still improve it in case it becomes an issue.

@exekias
Copy link
Contributor

exekias commented Sep 4, 2017

jenkins retest this please

@ruflin ruflin merged commit 48238e1 into elastic:master Sep 5, 2017
@vjsamuel vjsamuel deleted the remove_runnerfactory_error branch July 25, 2018 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants