Modifies basic settings for PhysicalInfraManager (events, refresher) #14029
Conversation
|
Checked commit https://github.com/lenovo/manageiq/commit/cd36b7d8df6d695116bf3bafa035b10611a1fa9d with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
|
This PR looks fine to me, but I can't see the travis log for some reason to see what the error is. Any idea @rodneyhbrown7? Update, could be related to S3 being down for a while. |
|
Looks like travis is passing now. This PR should be good to go. |
|
LGTM 👍 merge? |
| @@ -621,6 +621,12 @@ | |||
| :identifier: storage_tag | |||
| - :name: unassign | |||
| :identifier: storage_tag | |||
| :ems_events: | |||
There was a problem hiding this comment.
@abellotti I'm not sure how the api.yml is used. Any comments here?
There was a problem hiding this comment.
this is attempting to expose a new top level collection on the API /api/ems_events. Is that the intent ?
Either way, I wouldn't do this here, furthermore, a provider specific subcollection might be more appropriate, /api/providers/:id/events or something like this.
There was a problem hiding this comment.
@abellotti it might help to have a quick rundown on how to add new subcollections to the MIQ API. Is there a doc or something that the Lenovo crew can look over?
There was a problem hiding this comment.
@rodneyhbrown7 on second thought, I think this change should be removed from this PR. I don't think you need to expose the events over the API for now.
If you guys decide later to expose events over the API, we should chat about the use case and how they're going to be used. For now, events are typically handled by either the Automate Engine, or by Control/Policy.
There was a problem hiding this comment.
We are looking to use the provider to expose events from the endpoint so that they can be used by Automate and Control. Is there an alternate way to expose events coming from the end point? This is probably worth a discussion.
There was a problem hiding this comment.
@rodneyhbrown7 Right, I suggest dropping this change from this PR, then the rest can be merged. And, we can talk about the right way to expose what you want via the API.
|
This can be merged if @abellotti has no problem with the |
|
@miq-bot fine/yes |
|
@miq-bot add_label fine/yes |
|
@rodneyhbrown7 unrecognized command 'fine', ignoring... Accepted commands are: add_label, assign, close_issue, move_issue, remove_label, rm_label, set_milestone |
|
@rodneyhbrown7 Cannot apply the following label because they are not recognized: fine/yes |
|
Created PR #14424. This new PR does not have the config/api.yml or the api.yml changes. |
|
Closing, Review comments implemented in #14424 |
Modify the settings for the PHysicalInfraManager. Reduces the refresh rate and adds in the refresh and event working settings for the physical providers.
@miq-bot add_label: wip, providers/physical-infrastructure