Skip to content
This repository has been archived by the owner on Feb 22, 2020. It is now read-only.

#206 IsADirectoryError fix #224

Merged
merged 1 commit into from Nov 17, 2017
Merged

Conversation

musale
Copy link
Contributor

@musale musale commented Nov 14, 2017

Loading http://localhost:8080/ throws an IsADirectoryError: [Errno 21] Is a directory: '/'

Description

The endpoint is called every time the front end loads. Since the dicom.readfile() method is an IO op, I do a try/except to catch the IOError 21. As it is, this error is only anticipated on page load because there's no default path to a dicom file. I return an 'empty' response back so that the error is handled on the server-side and presents the client side an opportunity to handle it too.

To handle the empty response from the server.

Cornerstone throws errors if handed empty fields on the front end. Since the functions trigger calls to the server endpoint on load (I am assuming this is because of the async/await computed functions -- please correct my reasoning here if I'm wrong) and has nothing to display, catering for instances of empty values mitigates these unnecessary errors. Check the changes in OpenDICOM.vue file.

Reference to official issue

Fixes issue #206

Motivation and Context

This change removes the error logs for IsADirectoryError and also handles instances where a dicom file is not found.

How Has This Been Tested?

CLA

  • I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

@lamby
Copy link
Contributor

lamby commented Nov 15, 2017

Hi @musale! Did you mean to squash these into one commit? If you still have the separate changes on your local branch it would be great if you could push them instead; I'm seeing a bunch of whitespace changes that is making the rest hard to review in terms of what it fixes! No worries if not, but I thought I would just ping first instead of picking it apart manually :)

@musale
Copy link
Contributor Author

musale commented Nov 16, 2017

hi @lamby yes. I meant to squash it into one commit.

Also, this fixes warning on .vue files with v-for and lacking :key declarations, it fixes hot-reloading of the vue app
@musale
Copy link
Contributor Author

musale commented Nov 16, 2017

@lamby I think the update should be clearer on the changed files now 😄

try:
ds = dicom.read_file(path, force=True)
except IOError as err:
print(err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, we shouldn't be printing during a view method, no? :)

@lamby lamby merged commit 54ce001 into drivendataorg:master Nov 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants