[WIP] Allowing ems event as a target #13408
Conversation
|
Not sure I like the |
There was a problem hiding this comment.
EmsEvent as a target sounds strange to me, too. I'd assume a target is something about to be refreshed - as in "I target this thing to refresh".
But then we also said, an event should make its way through Automate and not talk to the save_inventory directly.
But maybe this is not true anymore?!
For the sake of simplicity I'd start with putting a save_inventory call on the queue with the event payload. Worry about edge cases later...
| refresh_targets = targets.collect { |t| get_target("#{t}_refresh_target") unless t.blank? }.compact.uniq | ||
| return if refresh_targets.empty? | ||
| if event_payload_refresh? | ||
| EmsRefresh.queue_refresh(self, nil, sync) |
There was a problem hiding this comment.
@Ladas What does it mean to refresh an EmsEvent object? Would the provider send the same event again?
I am concerned that it may get into an infinite loop of getting EmsEvent then asking the provider to send the same event again.
There was a problem hiding this comment.
@lfu the EmsEvent can be then processed in the provider refresh code. So e.g. I am trying to batch them and refresh many events at once, or can even cause targeted refresh.
The main reason for EmsEvent is that we can use this for create/update/delete, since it already exists. While the normal target can do only update/delete
The fallback in a provider will be, that if the event is not recognized, it will do a whole ems refresh.
There was a problem hiding this comment.
@lfu I think the idea is that the EmsEvent payload has enough information in it to be able to perform the refresh without requesting that data from the provider again, it wouldn't cause the provider to send the event again.
There was a problem hiding this comment.
Also for the AWS CloudWatch event, it can be a source of a targeted refresh. But it will not ask about event in any way, the refresh code will just do a refresh according to that event.
EmsEvent implement response to :name, which is being called by refresh logging code.
Allow EmsEvent as a target
Automate code will request refresh of the target in a standard way: "/System/event_handlers/event_action_refresh?target=ems_event"
Ladas
force-pushed
the
allowing_ems_event_as_a_target
branch
from
f8fb14f to
e6ad491
Compare
|
@agrare so in the end, I am bringing in a new target type, so there is no configuration needed and we can control target in automate. You can just call it with event_action_refresh?target=ems_event Does that look less controversial to you? :-) |
|
Checked commits Ladas/manageiq@3ad93f9~...e6ad491 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
|
@Ladas still not sure about an event being the target for inventory refresh. I would have thought that the event payload would be used to build the inventory collection rather than going out to the provider for the data. |
Ladas
changed the title
|
@miq-bot add_label wip |
|
replaced by a new target type #14249 |
Allowing EmsEvent as a target, used from an automation by calling "/System/event_handlers/event_action_refresh?target=ems_event"
WIP: I need to rethink this