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

Don't start every MetricsCollectorWorker #19683

Merged

Conversation

agrare
Copy link
Member

@agrare agrare commented Jan 3, 2020

Fix the #workers method for any worker that includes the PerEmsTypeWorkerMixin (aka just the MetricsCollectorWorkers).

This was setting an instance variable @workers but this wasn't actually being called so the ivar was never set up. This led to every metrics collector worker for every EMS type to be started if the role was enabled.

#19494

@Fryguy
Copy link
Member

Fryguy commented Jan 3, 2020

Does this effectively revert #19494 ? It's hard to tell what the difference is besides the specs

@Fryguy
Copy link
Member

Fryguy commented Jan 3, 2020

Also, can you fix the rubocops?

@agrare
Copy link
Member Author

agrare commented Jan 3, 2020

Does this effectively revert #19494 ? It's hard to tell what the difference is besides the specs

Not exactly, I think what @carbonin was trying to do with #19494 was not override the method because #19451 added some logic to the base method which wasn't picked up by these workers because they overrode the whole method. I tried to keep that in spirit by calling out to super after the check for EMSs

Also, can you fix the rubocops?

Sure thing

Fix the `#workers` method for any worker that includes the
PerEmsTypeWorkerMixin (aka just the MetricsCollectorWorkers).

This was setting an instance variable `@workers` but this wasn't
actually being called so the ivar was never set up.  This led to every
metrics collector worker for every EMS type to be started if the role
was enabled.
@agrare agrare force-pushed the fix_starting_metrics_collector_workers branch from d23840f to 5cb0e6b Compare January 3, 2020 19:02
@miq-bot
Copy link
Member

miq-bot commented Jan 3, 2020

Checked commit agrare@5cb0e6b with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🍪

@Fryguy Fryguy merged commit de8437c into ManageIQ:master Jan 3, 2020
@Fryguy Fryguy added this to the Sprint 127 Ending Jan 6, 2020 milestone Jan 3, 2020
@Fryguy Fryguy self-assigned this Jan 3, 2020
@agrare agrare deleted the fix_starting_metrics_collector_workers branch January 3, 2020 20:08
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.

3 participants