Add Automate modeling for External Ansible Tower Events. #68
Conversation
|
@miq-bot assign @gmcculloug |
|
@lfu Cannot apply the following labels because they are not recognized: providers/ansible_tower, automate |
| attributes: | ||
| description: | ||
| display_name: Ansible Tower | ||
| name: ANSIBLE_TOWER |
There was a problem hiding this comment.
|
@lfu Cannot apply the following label because they are not recognized: providers/ansible_tower |
|
@lfu Cannot apply the following label because they are not recognized: automate |
lfu
force-pushed
the
ansible_events_in_ae_140758707
branch
from
bbd71d4
to
aa8a886
Compare
|
@bzwei Please review. Wondering if it makes sense to issue refresh for all of the listed resources. I think there might be some object types that we do not collect, or we are not concerned with for embedded provider, that may not need to refresh. For example, groups or hosts. |
| @@ -0,0 +1,10 @@ | |||
| --- | |||
There was a problem hiding this comment.
@lfu is there a reason to choose Ansible_Tower.class rather than AnsibleTower.class?
There was a problem hiding this comment.
It has to match what is defined here.
There was a problem hiding this comment.
Wondering if the source reference should be changed to be a mixed-case value here and return AnsibleTower which would be inline with what is expected.
The original source value was VC and the events added after all followed the same uppercase pattern.
Examples:
app/models/manageiq/providers/openstack/cloud_manager/event_parser.rb#L10
app/models/manageiq/providers/google/cloud_manager/event_parser.rb#L14
app/models/manageiq/providers/storage_manager/cinder_manager/event_parser.rb#L12
@lfu Do you know if this would adversely effect anything?
Any thoughts? @Fryguy @blomquisg
There was a problem hiding this comment.
@lfu Talking to @Fryguy about this and it would be better to follow existing pattern which would mean that:
https://github.com/ManageIQ/manageiq/blob/master/app/models/manageiq/providers/embedded_ansible/automation_manager/event_parser.rb#L5
Should be changed to EMBEDDEDANSIBLE
https://github.com/ManageIQ/manageiq/blob/master/app/models/manageiq/providers/ansible_tower/automation_manager/event_parser.rb#L5
Should be changed to ANSIBLETOWER
Then we would need to update this PR and #69 to match the new source changes. (And use mixed-case in the automate class so it looks nicer like we do for OpenStack)
|
Even for external provider we don't need to refresh on receiving any job create/update/delete events. |
lfu
force-pushed
the
ansible_events_in_ae_140758707
branch
from
aa8a886
to
737935f
Compare
lfu
force-pushed
the
ansible_events_in_ae_140758707
branch
from
737935f
to
a4e415b
Compare
|
Checked commit lfu@a4e415b with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
https://www.pivotaltracker.com/story/show/140758707
cc @gmcculloug @blomquisg