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

Propagate exit status if given a child program to execute #163

Merged
merged 3 commits into from
Aug 18, 2020

Conversation

chrisburr
Copy link
Contributor

This more of a feature suggestion but it was simple enough for me to implement it as send a PR instead of opening an issue. I'm not sure if there are edge cases I haven't thought about it.

When passing an executable with arguments to would be useful to have the prmon process exit with the same error as the child, i.e.

Current behaviour:

$ prmon --json-summary test.json -- bash -c 'sleep 5 && exit 43'
Child process 894316 had non-zero return value: 43
$ echo $?
0

Suggested behaviour:

$ prmon --json-summary test.json -- bash -c 'sleep 5 && exit 43'
Child process 894316 had non-zero return value: 43
$ echo $?
43

Any thoughts? If backwards compatibility is important it could be hidden behind a command line flag (--exit-with-child-code).

@graeme-a-stewart
Copy link
Member

Hi @chrisburr - thanks a lot for this. I don't think I can see a use case for keeping the old 'always success' exit code in this case, so I think we don't really need the backwards compatibility flag.

Let me add a little note to the docs and add a unit test as well (our coverage is not great, but let's not make it worse for simple things!).

@graeme-a-stewart
Copy link
Member

@chrisburr I sent a PR to your branch with documentation and a test for this one

graeme-a-stewart and others added 2 commits August 18, 2020 20:25
Add test for feature where child exit code is
propagated back through prmon.
@graeme-a-stewart graeme-a-stewart merged commit 0359007 into HSF:master Aug 18, 2020
@chrisburr chrisburr deleted the exit-with-code branch August 22, 2020 19:22
@chrisburr
Copy link
Contributor Author

@graeme-a-stewart Thanks for the patch to clean this up (I was on holiday at the time).

Can you make a new release so I can update prmon on conda-forge?

@graeme-a-stewart
Copy link
Member

Yes, will do - there's one other small thing I need to fix, but it will be done soon...

@graeme-a-stewart graeme-a-stewart mentioned this pull request Sep 7, 2020
@graeme-a-stewart graeme-a-stewart added this to the v2.1 milestone Sep 8, 2020
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.

2 participants