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

Collect more attributes for Ansible Tower job #14076

Merged
merged 1 commit into from
Feb 28, 2017

Conversation

bzwei
Copy link
Contributor

@bzwei bzwei commented Feb 24, 2017

New attributes include start_time, finish_time, verbosity, hosts, and configuration_script_base_id

https://www.pivotaltracker.com/story/show/140529219

@gmcculloug
Copy link
Member

@bdunne Please review

Copy link
Member

@bdunne bdunne left a comment

Choose a reason for hiding this comment

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

Overall, looks good


if parameters.empty?
self.parameters =
raw_job.extra_vars_hash.collect do |para_key, para_val|
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be on the previous line to prevent the awkward indentation.

@@ -119,4 +120,11 @@ def inventory_name(action)
def use_default_inventory?(hosts)
hosts.blank? || hosts == 'localhost'
end

# update job attributes only available to playbook provisioning
def update_job_for_playbook(action, job, hosts)
Copy link
Member

Choose a reason for hiding this comment

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

Does this need a hosts = nil or are you expecting someone to pass nil as the last arg?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a private method used internal only, but hosts might be nil if it is never set anywhere.

@gmcculloug
Copy link
Member

@bzwei Please review test failures.

@miq-bot
Copy link
Member

miq-bot commented Feb 28, 2017

Checked commit bzwei@e807f8e with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
6 files checked, 0 offenses detected
Everything looks good. 🏆

@gmcculloug gmcculloug merged commit 1261a46 into ManageIQ:master Feb 28, 2017
@gmcculloug gmcculloug added this to the Sprint 56 Ending Mar 13, 2017 milestone Feb 28, 2017
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.

5 participants