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

Check modules and prospectors settings when reload is off #5053

Merged
merged 3 commits into from
Sep 6, 2017

Conversation

exekias
Copy link
Contributor

@exekias exekias commented Aug 29, 2017

This change ensures that we check configs when loading them through
filebeat.config.modules, filebeat.config.prospectors or metricbeat.config.modules.

It will error and exit if settings are wrong and reload is disabled

Part of #4810

This change ensures that we check configs when loading them through
`filebeat.config.modules`, `filebeat.config.prospectors` or `metricbeat.config.modules`.

It will error and exit if settings are wrong and **reload is disabled**
@exekias exekias added bug Filebeat Filebeat in progress Pull request is currently in progress. Metricbeat Metricbeat review needs_backport PR is waiting to be backported to other branches. labels Aug 29, 2017
@kvch
Copy link
Contributor

kvch commented Aug 29, 2017

Please note that it conflicts with my PR #5026, so one of us will have to rebase their branch after merging. :)

done: make(chan struct{}),
}
}

// Check configs are valid (only if reload is disabled)
func (rl *Reloader) Check(runnerFactory RunnerFactory) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

It is quite similar to the existing config reload code in Run function. I would extract anything that could be extracted into smaller functions. So those could be reused it in both Check and Run. Thus, code duplication could be eliminated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll do some refactoring, thanks!

@kvch
Copy link
Contributor

kvch commented Aug 29, 2017

I have one concerns regarding the code. I have already accepted your PR, so decide if you would address it or not.

@exekias exekias added v6.0.0-rc1 and removed in progress Pull request is currently in progress. labels Sep 5, 2017
@exekias
Copy link
Contributor Author

exekias commented Sep 5, 2017

This should be ready for a second look

@kvch
Copy link
Contributor

kvch commented Sep 5, 2017

After it turns green, I would merge it.

@kvch kvch merged commit 4c17368 into elastic:master Sep 6, 2017
@exekias exekias removed the needs_backport PR is waiting to be backported to other branches. label Sep 6, 2017
exekias added a commit to exekias/beats that referenced this pull request Sep 6, 2017
* Check modules and prospectors settings when reload is off

This change ensures that we check configs when loading them through
`filebeat.config.modules`, `filebeat.config.prospectors` or `metricbeat.config.modules`.

It will error and exit if settings are wrong and **reload is disabled**

* Extract common operations from check and start

(cherry picked from commit 4c17368)
tsg pushed a commit that referenced this pull request Sep 7, 2017
)

* Check modules and prospectors settings when reload is off

This change ensures that we check configs when loading them through
`filebeat.config.modules`, `filebeat.config.prospectors` or `metricbeat.config.modules`.

It will error and exit if settings are wrong and **reload is disabled**

* Extract common operations from check and start

(cherry picked from commit 4c17368)
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
…) (elastic#5107)

* Check modules and prospectors settings when reload is off

This change ensures that we check configs when loading them through
`filebeat.config.modules`, `filebeat.config.prospectors` or `metricbeat.config.modules`.

It will error and exit if settings are wrong and **reload is disabled**

* Extract common operations from check and start

(cherry picked from commit ac3d0dd)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants