mycobactopia-org / MTBseq-nf

MTBSeq made simple and easy using Nextflow and nf-core standard.
https://doi.org/10.5281/zenodo.5498063
MIT License
8 stars 1 forks source link

[DEV] Adding instructions to create log files using `stdout` and `stderr` #24

Closed Mxrcon closed 3 years ago

Mxrcon commented 3 years ago

Hey I've took some time to add instructions to write stdout and stderr to files that could be retrieved as a form of log file that should be enough for tracking now. Im using standard bash redirection as we're using only one command on each module, and for now it should be enough for the logging. I'm using this instruction on each module 2>err.log 1>out.log

This Pull Request closes #10 Feel free to question my changes and make any necessary modifications, I'm also available to discuss those changes in the comments section. Kindly, Davi.

abhi18av commented 3 years ago

Hi @Mxrcon , thanks for this PR.

However, the question is, how is it different from .command.out and .command.err which NF uses for capturing the stdout and stderr?

Mxrcon commented 3 years ago

Hey @abhi18av, the main difference is that using this we can capture the stderr and stdout only of the mtbseq command, instead of the complete script/shell part that Nextflow captures, other thing is that using this we may output the logs with the files, into a output/logs dir for users that don't have a good knowledge about debugging Nextflow

abhi18av commented 3 years ago

Ahh, refresh my memory here hmm - mtbseq wasn't populating .command.out and .command.err like other tools right?

Mxrcon commented 3 years ago

Ahh, refresh my memory here hmm - mtbseq wasn't populating .command.out and .command.err like other tools right?

Exactly, this gave us troubles to debbug using Google life science

abhi18av commented 3 years ago

Ah, okay. Then, I'll be merging this one.

Though one suggestion, could you please add genomeId to these err.log and out.log otherwise they'll end up over-writing similarly named files from other processes.

${genomeID}_err.log is one possible solution.

Mxrcon commented 3 years ago

yes sure, just give me a second to implement that!

Mxrcon commented 3 years ago

${genomeID}_err.log is one possible solution. what you think about ${task.process}_${genomeId}_err.log?

abhi18av commented 3 years ago

Hmm, good point 👍

Mxrcon commented 3 years ago

@abhi18av i also found a small typo on tbjoin module that i fixed, the problem was the usage of param mtbseq_project_name instead of just project_name

Mxrcon commented 3 years ago

@abhi18av let me fix the stubs in another PR so, let's open an issue and them fix it in the pr.

abhi18av commented 3 years ago

Sure, merging this PR.

abhi18av commented 3 years ago

For future reference, mentioning it here.

Using the ${task.process}_${genomeId}_err.log leads to names like test:TBBWA_5800_err.log because in DSL2, workflow name is appended to process name 😅

I think that ${task.process.split(":")[1]}_${genomeId}_err.log should be enough for our use cases.

Mxrcon commented 3 years ago

yes! makes sense, so let's update file name structure?

abhi18av commented 3 years ago

Nope, not yet. No rush Davi, I'm trying out somethings and will take care of this.

You can relax and focus on other things 😉

abhi18av commented 3 years ago

Update: I noticed that if we redirect the output to .command.out then it shows up in Tower's task level logs 🤓

BEFORE (with 1>test:TBBWA_5800_out.log ) https://tower.nf/orgs/BioSharp_OU/workspaces/Davi_Internship/watch/1h9Z1lPRfEag5s

image

AFTER (with 1>>.command.out )

https://tower.nf/orgs/BioSharp_OU/workspaces/Davi_Internship/watch/4R9I9HcSBHCv4P

image
Mxrcon commented 3 years ago

Nice to see those more informative messages, maybe this will fix the invalid exit status! Otherwise the messages are very useful on tower log.