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

Handle empty files #5720

Merged
merged 65 commits into from
Jul 15, 2024
Merged

Handle empty files #5720

merged 65 commits into from
Jul 15, 2024

Conversation

k1sauce
Copy link
Contributor

@k1sauce k1sauce commented May 29, 2024

This PR fixes #5612
This PR also handles empty FASTQ files by including a empty fastq emit channel

@k1sauce k1sauce requested a review from matthdsm as a code owner May 29, 2024 23:22
@k1sauce k1sauce linked an issue May 29, 2024 that may be closed by this pull request
2 tasks
Copy link
Contributor

@glichtenstein glichtenstein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a way to avoid sending falco empty fastqs. Modifying falco module itself looked straighforward to me but it seems I was not aware that it will cause other troubles as stated by the reviewer: #5769 (review), I will close my PR then.

@SPPearce
Copy link
Contributor

SPPearce commented Jun 6, 2024

Why is the read group parsing being removed?

@k1sauce
Copy link
Contributor Author

k1sauce commented Jun 7, 2024

This is a mistake on my part. I thought the read group parsing would download a local copy of the FASTQ file when the file was in cloud storage. Thus, I decided to remove it. However, I see now that it is a buffered Input stream and the file is not downloaded. I will add the read group parsing back in.

@k1sauce k1sauce changed the title Handle empty files, remove read group parsing Handle empty files Jun 11, 2024
@k1sauce
Copy link
Contributor Author

k1sauce commented Jul 8, 2024

This should be ready to go now. I had trouble with the nf-test for BCL Convert. The problem was that the test data file permissions were too restrictive. Because the GitHub action is self hosted with rootless docker, the test is executed as the host user and it could not read the test data when the volume is mounted. The fix was to change the test data file permissions to 775. It was an easy fix but it took me awhile to figure out what was going wrong. I found this explanation helpful https://joeeey.com/blog/rootless-docker-avoiding-common-caveats/#storage-fixes.

@k1sauce k1sauce requested a review from matthdsm July 8, 2024 19:41
@SPPearce
Copy link
Contributor

@matthdsm , @glichtenstein, are you happy with this?

Copy link
Contributor

@matthdsm matthdsm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

@k1sauce k1sauce added this pull request to the merge queue Jul 15, 2024
Merged via the queue into master with commit cc87c4d Jul 15, 2024
11 checks passed
@SPPearce SPPearce deleted the 5612-bcl-demultiplex branch July 15, 2024 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

new File breaks cloud execution in bcl_demultiplex
4 participants