Expose exceptions in DumpProcessingController #526
Conversation
Codecov Report
@@ Coverage Diff @@
## master #526 +/- ##
============================================
+ Coverage 81.02% 81.07% +0.04%
- Complexity 2085 2128 +43
============================================
Files 149 148 -1
Lines 7293 7354 +61
Branches 895 902 +7
============================================
+ Hits 5909 5962 +53
- Misses 1115 1122 +7
- Partials 269 270 +1 Continue to review full report at Codecov.
|
| @@ -374,7 +374,8 @@ public Sites getSitesInformation() throws IOException { | |||
| * @see DumpProcessingController#processDump(MwDumpFile) | |||
| * @see DumpProcessingController#getMostRecentDump(DumpContentType) | |||
| */ | |||
| public void processAllRecentRevisionDumps() { | |||
| public void processAllRecentRevisionDumps() | |||
| throws IOException, FileAlreadyExistsException { | |||
There was a problem hiding this comment.
FileAlreadyExistsException is a sub class of IOException (same in other places)
There was a problem hiding this comment.
Perhaps there is still a case for including both of them explicitly in the signature, because the caller might want to handle them differently?
| + " already exists. Try deleting the file or dumpfile directory to attempt a new download."); | ||
| + " already exists. Try deleting the file or dumpfile directory to attempt a new download."; | ||
| logger.error(errorMessage); | ||
| throw new FileAlreadyExistsException(errorMessage); |
There was a problem hiding this comment.
Would be nice to give to the constructor the file names to allow getFile() to be used by the caller. C.f. doc
| + dumpFile.toString() | ||
| + " could not be processed since file " | ||
| + e.getFile() | ||
| + " already exists. Try deleting the file or dumpfile directory to attempt a new download."); | ||
| + " already exists. Try deleting the file or dumpfile directory to attempt a new download."; | ||
| logger.error(errorMessage); |
There was a problem hiding this comment.
I am not sure the logging is still useful now that we raise the exception again.
Those are the changes proposed by @brett-matson in #523, minus the unrelated commits.