Flatten worker settings #684
Conversation
Fryguy
requested review from
agrare,
andyvesel,
aufi and
mansam
as code owners
| :ems_metrics_collector_worker: | ||
| :ems_metrics_gnocchi_granularity: 300 | ||
| :ems_metrics_openstack_default_service: "auto" |
There was a problem hiding this comment.
@agrare As discussed we should open a separate issue to not "infect" the base setting, but instead use the ems.ems_openstack section for these.
There was a problem hiding this comment.
wondering if these should become settings per provider. in general, wondering whether global settings for provider workers makes sense.
There was a problem hiding this comment.
Ideally, yes, these should be per-provider-instance settings, though unfortunately I don't think we don't have a mechanism for that at the moment (without making it part of the "create provider" form)
| :event_catcher: | ||
| :event_catcher_openstack_service: "auto" |
Fryguy
force-pushed
the
flatten_worker_settings
branch
from
9ed95b1 to
0164615
Compare
|
Checked commit Fryguy@0164615 with ruby 2.6.3, rubocop 0.82.0, haml-lint 0.35.0, and yamllint |
| @@ -420,7 +420,7 @@ def parse_datetime(datetime) | |||
| end | |||
|
|
|||
| def metric_service_from_settings | |||
| Settings[:workers][:worker_base][:queue_worker_base][:ems_metrics_collector_worker][:ems_metrics_openstack_default_service] | |||
| Settings.workers.ems_metrics_collector_worker.ems_metrics_openstack_default_service | |||
There was a problem hiding this comment.
@agrare I don't understand how this could have blown up previously. Like why is this rescued with StandardError?
There was a problem hiding this comment.
🤷 looks like it was added here ManageIQ/manageiq#13918
Looks like he was worried about the settings not being there?
There was a problem hiding this comment.
Strangely, the code doesn't care if the key is there but the value is nil, even though the log message talks about a nil value 🤷♂️
I can tweak this in a follow up to make more sense.
|
This pull request has been automatically marked as stale because it has not been updated for at least 3 months. If these changes are still valid, please remove the Thank you for all your contributions! More information about the ManageIQ triage process can be found in the triage process documentation. |
|
This pull request has been automatically closed because it has not been updated for at least 3 months. Feel free to reopen this pull request if these changes are still valid. Thank you for all your contributions! More information about the ManageIQ triage process can be found in the triage process documentation. |
Depends on ManageIQ/manageiq#20975