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

Add Automate modeling for Embedded Ansible Events. #69

Merged

Conversation

lfu
Copy link
Member

@lfu lfu commented Mar 8, 2017

@lfu
Copy link
Member Author

lfu commented Mar 8, 2017

@miq-bot assign @gmcculloug
@miq-bot add_label providers/ansible_tower
@miq-bot add_label automate
@miq-bot add_label euwe/no

@miq-bot
Copy link
Member

miq-bot commented Mar 8, 2017

@lfu Cannot apply the following labels because they are not recognized: providers/ansible_tower, automate

@miq-bot
Copy link
Member

miq-bot commented Mar 8, 2017

@lfu Cannot apply the following label because they are not recognized: providers/ansible_tower

@miq-bot
Copy link
Member

miq-bot commented Mar 8, 2017

@lfu Cannot apply the following label because they are not recognized: automate

@miq-bot
Copy link
Member

miq-bot commented Mar 8, 2017

@lfu Cannot apply the following label because they are not recognized: providers/ansible_tower

@miq-bot
Copy link
Member

miq-bot commented Mar 8, 2017

@lfu Cannot apply the following label because they are not recognized: automate

attributes:
description:
display_name: Ansible Tower
name: ANSIBLE_TOWER
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be:

  display_name: Ansible
  name: Embedded Ansible

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Right.
Should we use Embedded Ansible to tell from external Ansible?

    display_name: Embedded Ansible
    name: EMBEDDED_ANSIBLE

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think we need to duplicate the information. Can we use this:

  display_name:
  name: Embedded_Ansible

The duplications and upper-case make it look odd
image

I would prefer to just have:
image

As for external Ansible that is distinguished by the "Ansible_Tower" name.

@lfu lfu force-pushed the embedded_ansible_events_in_ae_140758691 branch from 8a15cdd to 944a75a Compare March 8, 2017 22:58
@lfu lfu force-pushed the embedded_ansible_events_in_ae_140758691 branch 2 times, most recently from d3c2911 to 5a5b1c7 Compare March 9, 2017 14:39
@bzwei
Copy link
Contributor

bzwei commented Mar 10, 2017

I agree we don't need to do refresh on any embedded tower events.
If we must enable the refresh, two scenarios need to be considered:

  1. Refresh does not include job at all.
  2. ManageIQ playbook service creates/deletes inventory before/after execution. Running refreshes to collect a temporary inventory and delete it later makes no sense.

@lfu lfu force-pushed the embedded_ansible_events_in_ae_140758691 branch from 5a5b1c7 to f6c1870 Compare March 10, 2017 15:53
@lfu lfu force-pushed the embedded_ansible_events_in_ae_140758691 branch from f6c1870 to 7dcf25b Compare March 14, 2017 18:17
@miq-bot
Copy link
Member

miq-bot commented Mar 14, 2017

Checked commit lfu@7dcf25b with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
0 files checked, 0 offenses detected
Everything looks good. 🍪

@gmcculloug gmcculloug merged commit 716bc20 into ManageIQ:master Mar 15, 2017
@gmcculloug gmcculloug added this to the Sprint 57 Ending Mar 27, 2017 milestone Mar 15, 2017
@lfu lfu deleted the embedded_ansible_events_in_ae_140758691 branch September 29, 2018 14:33
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.

4 participants