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

Prints error messages in terminal if job submission fails #6

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ManavalanG
Copy link

@ManavalanG ManavalanG commented Feb 25, 2020

This MR makes following modifications to facilitate printing error messages when job submission fails.

1. Adds try-except block when submitting jobs to print error messages directly in the terminal.
2. Prints stdout from job submission (res) as typical string instead of binary string.

@brentp
Copy link
Owner

brentp commented Feb 25, 2020

don't you think it should still raise an exception if the job doesn't submit?

@ManavalanG
Copy link
Author

ManavalanG commented Feb 25, 2020

My bad, did you mean to say slurmpy already does this?

@brentp
Copy link
Owner

brentp commented Feb 26, 2020

well, before, the exception was unchecked, but raised (by python); now, you catch the exception but don't re-raise it. how about raise e after the printed messages?

@ManavalanG
Copy link
Author

Turns out slurmpy already deals with error messages as expected. I use slurmpy to submit Snakemake jobs. When snakemake is run in dryrun mode, its error messages get redirected to stdout (instead of stdout), while those same errors get redirected to stderr when NOT in dryrun mode. I just realized this "gotcha", which makes try..except modification made in this PR unnecessary.

However, I would still like to go ahead with the suggestion of printing res not as a binary string, as this makes long strings difficult to read.

PS - Yes, I should have raised exception.

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