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

piped commands that fail don't trigger an error in subprocess.check_call #15

Closed
brentp opened this issue May 16, 2013 · 3 comments
Closed

Comments

@brentp
Copy link
Contributor

brentp commented May 16, 2013

With a command like this in ngsalign/bwa.py:

                cmd = ("{bwa} mem -M -t {num_cores} -R '{rg_info}' -v 1 {ref_file} "
                       "{fastq_file} {pair_file} "
                       "| {samtools} view -b -S -u - "
                       "| {samtools} sort -@ {num_cores} -m {max_mem} - {tx_out_prefix}")

if bwa mem fails, the process will continue running and error farther in the pipeline so the bam transaction completes and something later in the pipeline that's expecting a valid BAM files errors. Since this part of the transaction completes, it makes this harder to debug. I'm not sure what is portable, but, in bash, we can have the check_call fail by using:

                cmd = ("set -o pipefail && "
                       "{bwa} mem -M -t {num_cores} -R '{rg_info}' -v 1 {ref_file} "
                       "{fastq_file} {pair_file} "
                       "| {samtools} view -b -S -u - "
                       "| {samtools} sort -@ {num_cores} -m {max_mem} - {tx_out_prefix}")

where we added set -o pipefail at the start.

@chapmanb
Copy link
Member

Brent;
Nice one. Thanks for the pipefail tip. The piping problem was annoying me but I didn't know any tricks to work around it. What I'll plan to do is provide a general function to call out to subprocess that:

I'll update this once I have a chance to work this through. Thanks again.

@brentp
Copy link
Contributor Author

brentp commented May 16, 2013

maybe this could be implemented by some check_?? decoroator.

e.g. for this, it'd be check_bam_ok for other functions, it could be check_file_exists or check_file_non_empty

they could all expect a path to a file as the only argument and raise an Exception if the appropriate checks did not pass.

@chapmanb
Copy link
Member

Brent;
Thanks for all the great ideas. I checked in improvements which hopefully handle this, using your check function approach. I agree this is a lot less magic, which is good. My only worry is if failed commands result in partially truncated intermediates, which will be harder to diagnose but this is a good starting point. Let me know if you run into any issues with this and feel free to re-open.

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

No branches or pull requests

2 participants