Page MenuHomePhabricator
Create Task
Maniphest T18819

Unknown file extension causes Command Line Modules to throw errors.
Closed, ResolvedPublic
Actions

Description

All output files from a command line module get automatically loaded when the command line module finishes. However, if the file extension is unrecognised it obviously cant be loaded.

In MITK 2014.03, this was simply a warning message to console.
In MITK 2014.10, an exception is thrown.

It may well be the case that a command line module generates output such as .csv files. Clearly these should not even be attempted to be automatically reloaded.

So, we should check the file extension before attempting to load.

Event Timeline

clarkson added projects: MITK, Bugzilla.Mar 6 2015, 11:20 AM
zelzer added a subscriber: zelzer.Mar 9 2015, 2:25 PM

Agreed. Could we just catch the execption and log a warning, emulation the 2014.10 behaviour?

clarkson added a subscriber: clarkson.Mar 11 2015, 4:33 PM

What about:

https://github.com/MattClarkson/MITK/commit/5f851e17db5c5bf1e233cd65d82e24810d17aece

zelzer added a comment.Mar 11 2015, 4:48 PM

Looks okay to me. Was there a special reason for using the LoadInfo object instead of just catching mitk::Exception from the IOUtil::Load() call?

clarkson added a comment.Mar 11 2015, 5:52 PM

Not really. I just try to avoid exceptions if I can. In Java it used to be quite expensive to raise exceptions. Maybe this is an invalid approach now. I am getting old after all.

zelzer added a comment.Mar 11 2015, 5:56 PM

I see ;-)

Unfortunately, we should still catch the exception because the LoadInfo "trick" does not mean that there won't be an exception during the IOUtil::Load() call. The reader could still encounter errors when actually reading the file.

clarkson added a comment.Mar 11 2015, 6:15 PM

Oh yes.

Something like this then:
https://github.com/MattClarkson/MITK/commit/ea6e0b1171f20b7a82c48611463cf9c48c555a55

zelzer added a comment.Mar 11 2015, 6:49 PM

Yes, looks good to me. Thanks for working on this! I could merge a PR today.

clarkson added a comment.Mar 11 2015, 7:01 PM

Add the moment I have no program with which to test it.

I was wanting John Hipwell (UCL) to test it. But he's not available today.

clarkson added a comment.Mar 11 2015, 9:59 PM

Hi Sascha,

i iterated a bit in testing, and ended up here:

https://github.com/MITK/MITK/pull/97

Matt

zelzer added a comment.Mar 12 2015, 4:59 PM

[71e2eb]: Merge branch 'bug-18819-check-file-extension' of https://github.com/Ma

Merged commits:

2015-03-06 11:13:09 Matt Clarkson [933db1]
Check file extension, and use try/catch for CLI module.

kislinsk edited projects, added MITK (2015-03); removed MITK.Aug 1 2016, 10:49 PM
MITK · MIC · IMSY · DKFZ