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.

Sign up for GitHub

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

Jump to bottom

Add support for using run_role_async #18108

Merged
merged 1 commit into from
Oct 31, 2018
Merged

Add support for using run_role_async #18108

merged 1 commit into from
Oct 31, 2018

Conversation

borod108
Copy link

@borod108 borod108 commented Oct 18, 2018 •
edited
Loading

Add the option to use the run_role_async using AnsibleWorkflow

Required for implementing: https://bugzilla.redhat.com/show_bug.cgi?id=1644605

@borod108
Copy link
Author

@miq-bot add_label wip

@miq-bot miq-bot added the wip label Oct 18, 2018
@agrare agrare self-assigned this Oct 18, 2018
agrare
agrare reviewed Oct 18, 2018
View reviewed changes
app/models/manageiq/providers/ansible_runner_workflow.rb Outdated
alias finish process_finished
alias abort_job process_abort
alias cancel process_cancel
alias error process_error

def fail_unimplamented
raise "this is an abstract class, use a subclass that implaments a 'start' method"
end
Copy link
Member

Choose a reason for hiding this comment

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

This is slick 👍

agrare
agrare reviewed Oct 18, 2018
View reviewed changes
app/models/manageiq/providers/ansible_role_workflow.rb Outdated

def load_transitions
super.tap do |transactions|
transactions.merge(
Copy link
Member

Choose a reason for hiding this comment

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

should this be merge! ?

Copy link
Member

Choose a reason for hiding this comment

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

Oh nevermind I think this is right

Copy link
Author

Choose a reason for hiding this comment

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

You were right! Fixed.

Copy link
Author

Choose a reason for hiding this comment

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

You were right! Fixed.

Copy link
Author

Choose a reason for hiding this comment

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

You are right! Thank you!

@borod108 borod108 force-pushed the rfe/add_role_async_runner_to_wf branch 4 times, most recently from a4b7d0a to 07ed02e Compare October 22, 2018 09:36
agrare
agrare reviewed Oct 22, 2018
View reviewed changes
app/models/manageiq/providers/ansible_playbook_workflow.rb

def load_transitions
super.tap do |transactions|
transactions.merge!(
Copy link
Member

Choose a reason for hiding this comment

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

You can probably just do super.merge(...) (untested)

@borod108 borod108 force-pushed the rfe/add_role_async_runner_to_wf branch 2 times, most recently from 142f0f3 to 14f164a Compare October 25, 2018 12:09
Add support for using run_role_async
7af794d 
Add the option to use the run_role_async using AnsibleWorkflow
@borod108 borod108 force-pushed the rfe/add_role_async_runner_to_wf branch from 14f164a to 7af794d Compare October 29, 2018 08:08
@miq-bot
Copy link
Member

miq-bot commented Oct 29, 2018

Checked commit borod108@7af794d with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
5 files checked, 1 offense detected

app/models/manageiq/providers/ansible_runner_workflow.rb

  • ⚠️ - Line 10, Col 19 - Lint/UnusedMethodArgument - Unused method argument - options. If it's necessary, use _ or _options as an argument name to indicate that it won't be used. You can also write as job_options(*) if you want the method to accept any arguments but don't care about them.

@borod108 borod108 changed the title [wip] Add support for using run_role_async Add support for using run_role_async Oct 29, 2018
@borod108
Copy link
Author

@miq-bot remove_label wip

@borod108
Copy link
Author

@agrare what do you think?

@miq-bot miq-bot removed the wip label Oct 29, 2018
@borod108 borod108 mentioned this pull request Oct 29, 2018
Closed
agrare
agrare approved these changes Oct 31, 2018
View reviewed changes
Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

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

LGTM

@agrare agrare merged commit 724d1e7 into ManageIQ:master Oct 31, 2018
@agrare agrare added this to the Sprint 98 Ending Nov 5, 2018 milestone Oct 31, 2018
@borod108
Copy link
Author

borod108 commented Dec 3, 2018

@miq-bot add-label hammer/yes

simaishi pushed a commit that referenced this pull request Dec 3, 2018
@agrare @simaishi
Merge pull request #18108 from borod108/rfe/add_role_async_runner_to_wf
d3b4c65 
Add support for using run_role_async

(cherry picked from commit 724d1e7)
@simaishi
Copy link
Contributor

simaishi commented Dec 3, 2018

Hammer backport details:

$ git log -1
commit d3b4c65a09645b340032715a703fcca76754ed63
Author: Adam Grare <agrare@redhat.com>
Date:   Wed Oct 31 13:50:40 2018 -0400

    Merge pull request #18108 from borod108/rfe/add_role_async_runner_to_wf
    
    Add support for using run_role_async
    
    (cherry picked from commit 724d1e759addea12f34fa923ffd5a94861845536)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agrare agrare agrare approved these changes

Assignees

@agrare agrare

Labels
enhancement hammer/backported providers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@borod108 @miq-bot @simaishi @agrare