Validate IO in SortBam to provide nicer exceptions #994
Conversation
Using the `Io.assert*` within the constructor will yield better error messages for the user. Also added an explicit close to the input `SamSource`
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #994 +/- ##
==========================================
- Coverage 95.62% 95.61% -0.02%
==========================================
Files 126 126
Lines 7364 7365 +1
Branches 500 510 +10
==========================================
Hits 7042 7042
- Misses 322 323 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| Io.assertReadable(input) | ||
| Io.assertCanWriteFile(output) |
There was a problem hiding this comment.
Naive question, what changes when these are moved to the constructor/class body?
Since the constructor doesn't do anything else, and nothing happens in execute before the assert, I would have thought that asserting at either location would be functionally the same
There was a problem hiding this comment.
Exceptions in the constructor are ValidationExceptions, and any exceptions (typically validation errors of the arguments) are nicely formatted on the command line, versus a huge stacktrace. And it will print out the usage too. But none of those things will happen when running the tool
| val in = SamSource(input) | ||
| val out = SamWriter(output, in.header.clone(), sort=Some(sortOrder), maxRecordsInRam=maxRecordsInRam) | ||
| out ++= in | ||
| out.close() | ||
| in.safelyClose() |
There was a problem hiding this comment.
Also a naive question, why doesn't out also take safelyClose()?
There was a problem hiding this comment.
The idea is that we care that the output files are closed, but do not want the tool to fail/crash/except when we're cleaning up resources after the main effort of the tool has completed, so we do not propagate errors during closing inputs in this case.
Using the
Io.assert*within the constructor will yield better error messages for the user.Also added an explicit close to the input
SamSource