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

jobs.exit_success allow to check if a job has executed and exit successfully #33465

Merged

Conversation

meaksh
Copy link
Contributor

@meaksh meaksh commented May 24, 2016

What does this PR do?

This PR create a new method for runners.jobs which easy allow you to check if a job has been executed and it has returned successfully.

Currently salt lets you check the results of a job using:

# salt-run jobs.lookup_jid 20160520173111930120
minionsles12sp1-suma3pg.vagrant.local:
    ----------
    pid:
        15124
    retcode:
        0
    stderr:
    stdout:

If what we want is to check if a job has executed and returned successfully or not, I propose jobs.exit_success on this PR:

# salt-run jobs.exit_success 20160520173111930120
minionsles12sp1-suma3pg.vagrant.local:
    True

What do you think?

Tests written?

No

@cachedout
Copy link
Contributor

I like the idea here but it looks like a lot of this code is copied directly from lookup_jid. Can we just call to that and then parse the results in this function? I don't like to have this much duplicated code in a runner if it can be helped. Let me know what you think.

@cachedout cachedout added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label May 24, 2016
@meaksh
Copy link
Contributor Author

meaksh commented May 25, 2016

I'm agree with you @cachedout , parse the results of lookup_jid is better than duplicate code.
In our case, exit_success only uses data contained in the results of lookup_jid, so I added a refactor of exit_success in order to parse the results of lookup_jid.

What do you think now?

@cachedout
Copy link
Contributor

Much nicer! Thanks, @meaksh !

@cachedout cachedout merged commit 3bfb6bf into saltstack:2015.8 May 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants