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

Fix Embedded Ansible Plays in the Job UI (v2/rebased with master) #19247

Merged

Conversation

NickLaMuro
Copy link
Member

@NickLaMuro NickLaMuro commented Sep 3, 2019

Rebased version of #19236 with the original text below

This is the second half of the extraction from #19215 , using the modifications from master...NickLaMuro:embedded_ansible_fix_missing_ui_elements_with_ansible_runner_v2 , followed by some cleanup.

https://bugzilla.redhat.com/show_bug.cgi?id=1741883

@bdunne Please review.

Links

Adds a method for collecting the plays for a given playbook run,
itemizing them and determining the elapsed time for each.  Very similar
to the code found in the `manageiq-providers-ansible_tower` code:

https://github.com/ManageIQ/manageiq-providers-ansible_tower/blob/88aa135c/app/models/manageiq/providers/ansible_tower/shared/automation_manager/job.rb#L117-L144

The difference here is that the code that fetches the play events is
using an wrapper model class around the event data.  By contrast, the
code here is working with raw hashes instead, though most of the keys
do end up aligning with the method names from the tower code since the
wrapper models are `HashModel` objects.

https://bugzilla.redhat.com/show_bug.cgi?id=1741883
@NickLaMuro
Copy link
Member Author

@miq-bot add_label bug, embedded ansible, ivanchuk/yes
@miq-bot assign @bdunne

Copy link
Member

@kbrock kbrock left a comment

Choose a reason for hiding this comment

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

just the 1 change

NickLaMuro and others added 2 commits September 3, 2019 14:14
- Minor refactor to make the finish_time setting a little clearer from a
  readbility perspective
- Add a spec to demonstrate that non-play events are skipped
- Modify the times in the spec to be much farther apart and use
  Timecop.freeze to avoid potential time-based sproadic test failures.
- Some rubocops

https://bugzilla.redhat.com/show_bug.cgi?id=1741883
@NickLaMuro NickLaMuro force-pushed the embedded_ansible_fix_plays_output_v2 branch from 31bc4d3 to 3d6bcf8 Compare September 3, 2019 19:14
@miq-bot
Copy link
Member

miq-bot commented Sep 3, 2019

Checked commits NickLaMuro/manageiq@7702e4e~...3d6bcf8 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 1 offense detected

app/models/manageiq/providers/embedded_ansible/automation_manager/job.rb

@agrare agrare assigned agrare and unassigned bdunne Sep 3, 2019
@agrare agrare merged commit 01a516a into ManageIQ:master Sep 3, 2019
@agrare agrare added this to the Sprint 120 Ending Sep 16, 2019 milestone Sep 3, 2019
simaishi pushed a commit that referenced this pull request Sep 4, 2019
…output_v2

Fix Embedded Ansible Plays in the Job UI (v2/rebased with master)

(cherry picked from commit 01a516a)

https://bugzilla.redhat.com/show_bug.cgi?id=1741883
@simaishi
Copy link
Contributor

simaishi commented Sep 4, 2019

Ivanchuk backport details:

$ git log -1
commit fd8047e783deefc416f45ed204f38c5bea9b1fcd
Author: Adam Grare <agrare@redhat.com>
Date:   Tue Sep 3 16:36:31 2019 -0400

    Merge pull request #19247 from NickLaMuro/embedded_ansible_fix_plays_output_v2
    
    Fix Embedded Ansible Plays in the Job UI (v2/rebased with master)
    
    (cherry picked from commit 01a516a7f10106bbf62f304fbb50d8995dda98fa)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1741883

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.

7 participants