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

Fixes #29867 - Show job status for each host on JobInvocation detail #32

Merged
merged 2 commits into from Jun 25, 2020
Merged

Fixes #29867 - Show job status for each host on JobInvocation detail #32

merged 2 commits into from Jun 25, 2020

Conversation

stejskalleos
Copy link
Contributor

@stejskalleos stejskalleos commented May 19, 2020 •
edited

Requires: theforeman/foreman_remote_execution#494

Before

hammer job-invocation info --id 881
ID:                  881
Description:         Run sleep 160
Status:              succeeded
Success:             11
Failed:              0
Pending:             0
Total:               11
Start:               2020-05-18 12:43:11 UTC
Randomized ordering: false
Job Category:        Commands
Hosts:               
 - zzzz100.example.com
 - zzzz101.example.com
 - zzzz102.example.com
 - zzzz103.example.com

Now

hammer job-invocation info --id 881 --host-status
ID:                  881
Description:         Run sleep 160
Status:              succeeded
Success:             11
Failed:              0
Pending:             0
Total:               11
Start:               2020-05-18 12:43:11 UTC
Randomized ordering: false
Job Category:        Commands
Hosts:               
 - zzzz100.example.com, Job status: success
 - zzzz101.example.com, Job status: success
 - zzzz102.example.com, Job status: success
 - zzzz103.example.com, Job status: success

@stejskalleos stejskalleos changed the title [WIP] Fixes #29867 - Show job status for each host on JobInvocation detail Fixes #29867 - Show job status for each host on JobInvocation detail May 19, 2020
@stejskalleos stejskalleos mentioned this pull request May 19, 2020
Merged
@adamruzicka
Copy link
Contributor

@ofedoren hi, could you please take a look?

ofedoren
ofedoren reviewed Jun 23, 2020
View reviewed changes
Copy link
Member

@ofedoren ofedoren left a comment

Choose a reason for hiding this comment

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

@stejskalleos, seems OK to be merged, but I've got one suggestion, which can be considered as an advice for the future, not necessary addressed in this PR :D

If everything stays the way it is already: I think the --host-status flag can be renamed to something more specific. Maybe I'm wrong, but I'm finding a bit confusing --host-status flag showing job status for each host...

lib/hammer_cli_foreman_remote_execution/job_invocation.rb Outdated Show resolved Hide resolved
lib/hammer_cli_foreman_remote_execution/job_invocation.rb Show resolved Hide resolved
@stejskalleos stejskalleos mentioned this pull request Jun 25, 2020
Merged
@stejskalleos
Copy link
Contributor Author

Updated & rebased,
description and type changed in REX plugin: theforeman/foreman_remote_execution#510

ofedoren
ofedoren approved these changes Jun 25, 2020
View reviewed changes
Copy link
Member

@ofedoren ofedoren left a comment

Choose a reason for hiding this comment

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

With theforeman/foreman_remote_execution#510 works great after API docs recaching :) so, 👍

@adamruzicka adamruzicka merged commit 39e8895 into theforeman:master Jun 25, 2020
@adamruzicka
Copy link
Contributor

Thank you @stejskalleos & @ofedoren !

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

@ofedoren ofedoren ofedoren approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@stejskalleos @adamruzicka @ofedoren