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 ssh port to inventory #560

Merged

Conversation

dangel101
Copy link
Member

@dangel101
Copy link
Member Author

/ost

Copy link
Member

@mwperina mwperina left a comment

Choose a reason for hiding this comment

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

+1

@mwperina
Copy link
Member

/ost

Copy link
Member

@ahadas ahadas left a comment

Choose a reason for hiding this comment

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

besides the comment inside on keep checking for null value of hostname, this looks great, thanks @dangel101

@dangel101
Copy link
Member Author

/ost

@@ -190,14 +190,14 @@ public AnsibleReturnValue runCommand(AnsibleCommandConfig commandConfig, int tim
}

private AuditLogable createAuditLogable(AnsibleCommandConfig command, String taskName) {
if (command.hosts() == null) {
if (command.host() == null) {
Copy link
Member

Choose a reason for hiding this comment

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

We can remove the check if we add a check to AnsibleExecutor.runCommand

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@dangel101
Copy link
Member Author

/ost

@smelamud
Copy link
Member

smelamud commented Aug 3, 2022

Please add a description to the commit message.

Host validity is now verified before building the ansible-runner command,
thus any operation on 'host' property is done on a non-null object
@dangel101 dangel101 force-pushed the configure_ssh_port_on_ansible_command_config branch from 4325c5c to 863fbd3 Compare August 4, 2022 07:48
Copy link
Member

@mwperina mwperina left a comment

Choose a reason for hiding this comment

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

+1

@mwperina mwperina merged commit f7561f9 into oVirt:master Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants