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

Correct error message when drmaa job exits with non-zero status. #59

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

Conversation

bjpop
Copy link

@bjpop bjpop commented Jul 1, 2015

Previously it was incorrectly reported as being terminated by a signal.

bjpop added 2 commits July 1, 2015 11:28
…iously it was incorrectly reported as being termnated by a signal
@bunbun
Copy link
Collaborator

bunbun commented Jul 2, 2015

Hi Bernie,

Thanks for the fixes. It is such an unbelievable luxury to have a TravisCI-passing merge-ready pull request.

Retaining stdout and stderr files will obviously be extremely useful for debugging.

Can I ask what the use case is? Do we really need such fine grained control in exceptional (debugging) circumstances. Do you not think that they should be retained (as per your code) when retain_job_scripts = True rather than having separate flags for the stdout and stderr files? Or are their pipelines where you retain stdout and stderr as a matter of course? In which case, how do you know which file names are associated with which jobs?

Thanks again,

Leo

@bjpop
Copy link
Author

bjpop commented Jul 3, 2015

Hi Leo,

It looks like I accidentally put two commits in the one pull request. Sorry about that. I had intended to put the bugfix in its own pull request, and the retain outputs in a separate request.

Please let me know if you would like them separated, I'm happy to do it.

I was trying to be as flexible as possible with the retain flags. In many cases I just want to keep the stderr, and not the stdout.

I thought we associated the job ID with the output files? If that is the case then I think that is a reasonable way to figure out where the outputs belong.

However, I have been thinking it would be nice for the saved files to optionally have a user-supplied string as part of their name, to make the identification easier.

Cheers,
Bernie

@bjpop
Copy link
Author

bjpop commented Jul 3, 2015

I should also note that I had a request from one of my colleagues to be able to keep the stdout and stderr as a matter of normal operation in the pipeline. They were frustrated that the files were removed upon successful completion.

In our previous wrappers for Ruffus we used to keep all the stdout and stderr files by default. Just in case people wanted to look at them.

In fact, if we always kept them then we could just print the path to the files in exceptions instead of reading and printing their contents, as is done in Ruffus at the moment.

@bunbun
Copy link
Collaborator

bunbun commented Jul 3, 2015

No problems about the two commits.
If you are saving the files as a matter of course then it would be much more sensible to be able to give the stdout / tderr files more friendly names... This alas will only further weigh down the number of parameters this poor function has. Perhaps this should have been a class rather than a standalone function... Perhaps some of these extra parameters can be grouped together in some sort of lightweight parameter object...
Will stare at the code at work tomorrow!
Thanks again.
Leo

@bjpop
Copy link
Author

bjpop commented Aug 12, 2016

I seem to have lost track of what happened with this pull request.

I don't seem to see the 3d30bfb appearing in the master branch of ruffus.

I would like this functionality to be included if possible as it is incredibly useful for debugging.

Apologies if I've got things confused

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