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

Log playbook stdout according to options #16333

Merged
merged 1 commit into from
Oct 30, 2017
Merged

Conversation

bzwei
Copy link
Contributor

@bzwei bzwei commented Oct 27, 2017

The playbook runner now accepts an option log_output that specifies how to write stdout of an Ansible playbook to the log. The option is one of always, on_error, and never.

@bzwei
Copy link
Contributor Author

bzwei commented Oct 27, 2017

@miq-bot add_label enhancement
@miq-bot assign @gmcculloug
@mkanoor @syncrou @tinaafitz please review

@@ -97,6 +99,11 @@ def ansible_job
ManageIQ::Providers::EmbeddedAnsible::AutomationManager::Job.find(options[:tower_job_id])
end

def set_status(message, status = "ok")
Copy link
Contributor

Choose a reason for hiding this comment

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

@bzwei - Where does this come into play for this PR

Copy link
Contributor

@syncrou syncrou Oct 27, 2017

Choose a reason for hiding this comment

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

Nevermind - Missed the call to super

Copy link
Contributor

@syncrou syncrou left a comment

Choose a reason for hiding this comment

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

Some rework on the naming for log_stdout

@@ -173,4 +180,13 @@ def delete_job_template
_log.log_backtrace(err)
false
end

def log_stdout(succeeded)
Copy link
Contributor

@syncrou syncrou Oct 27, 2017

Choose a reason for hiding this comment

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

It's hard to know what is required as an argument to this method without knowing its internal workings. Should this be called log_stdout? with the argument having a default? In the current format you could pass a 1 or yes as an argument and it would still work. Passing no would also be treated as a true Is that expected behavior?

@miq-bot
Copy link
Member

miq-bot commented Oct 30, 2017

Checked commit bzwei@ae02659 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

Copy link
Contributor

@syncrou syncrou left a comment

Choose a reason for hiding this comment

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

I'm ok with the changes. Knowing that log_stdout is private removes my concern.

@gmcculloug gmcculloug merged commit be1f434 into ManageIQ:master Oct 30, 2017
@gmcculloug gmcculloug added this to the Sprint 72 Ending Oct 30, 2017 milestone Oct 30, 2017
@bzwei bzwei deleted the log_stdout branch October 30, 2017 19:30
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