Handling Download Errors #15
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This comment was marked as resolved.
This comment was marked as resolved.
apetkau
requested changes
Apr 9, 2024
tests/workflows/fastq_download_prefetch_fasterqdump_sratools/main.nf.test
Show resolved
Hide resolved
tests/workflows/fastq_download_prefetch_fasterqdump_sratools/main.nf.test
Outdated
Show resolved
Hide resolved
tests/workflows/fastq_download_prefetch_fasterqdump_sratools/main.nf.test
Show resolved
Hide resolved
Updated the README: e31e41f |
apetkau
reviewed
Apr 9, 2024
apetkau
requested changes
Apr 9, 2024
There was a problem hiding this comment.
Thanks so much for all your changes. This looks great.
I do have one more comment. I'm wondering if we could add a test to make sure that if there is an error, the errored sample does not end up with references to any data in the iridanext.output.json.gz file?
So, I'm wondering if you could add a sample that triggers an error to the samplesheet used in the end-to-end pipeline testing at https://github.com/phac-nml/fetchdatairidanext/blob/handle-errors/tests/data/samplesheet.csv
And then make sure there's no entry in the final iridanext.output.json.gz file, which is already handled by comparison to the expected JSON file here: https://github.com/phac-nml/fetchdatairidanext/blob/handle-errors/tests/pipelines/fetchdatairidanext.nf.test#L19 (so there's no need for changes for this).
I was also trying to think if it's possible to have partially downloaded fastqs in the IRIDA Next JSON output during e.g., a network error, but I don't think that's possible. The only step that could have a network error is prefetch, and that downloads a *.sra file, which is converted to fastq in the fasterq-dump step. Errors in the fasterq-dump step will still trigger an error with the full pipeline, so it's not possible for the ignored error in prefetch to ever lead to incomplete fastqs that wind up in IRIDA Next.
Does the above logic make sense to you?
|
I added another test as you suggested in ecca1f1 It uses the I also think there probably won't be partial reads in |
apetkau
approved these changes
Apr 10, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added the ability to handle errors when downloading data.
The pipeline will continue despite individual sample download errors.
The individual sample errors (if they exist) will be reported in
results/prefetch/failures_report.csv.