search

bcgov / ligo-lib

A Python library to support the Ligo linking application.
https://github.com/bcgov/ligo
Apache License 2.0
1 stars 1 forks source link

General Exception and Error Checking #12

Open NovaVic opened 7 years ago

NovaVic commented 7 years ago

What if file open fails here https://github.com/bcgov-c/data-linking/blob/develop/cdi-linking/cdilinker/linker/commands.py#L10 ?

Should not there be some check and return instead of doing conditional importing few lines down (in the same file)? https://github.com/bcgov-c/data-linking/blob/develop/cdi-linking/cdilinker/linker/commands.py#L137

Same here: https://github.com/bcgov-c/data-linking/blob/develop/cdi-linking/cdilinker/linker/chunked_link.py#L194 ... what if the file does not exist? There are more statements like this in the same file. Please consider those as well. Thanks.

The exception handlers are too open or generic (which is not a good practice) https://github.com/bcgov-c/data-linking/blob/develop/web/linkage/linkage/linking/forms.py#L59 https://github.com/bcgov-c/data-linking/blob/develop/web/linkage/linkage/linking/forms.py#L98 There are similar statements in utils.py too. Let's try to be more specific if/wherever we can.

Khalegh-H3 commented 7 years ago

The file existence checking is done during project validation. No need to check it again here.

Exception handling and logging improved by commit https://github.com/bcgov-c/data-linking/commit/40b9187aadd57ecfe30ac2f4a2ee1c640ec498a3

NovaVic commented 7 years ago

"Should not there be some check and return instead of doing conditional importing few lines down (in the same file)?" - this one would prevent extra time in execution for some scanarios. However am I guessing it right that is now hard to locate that part and handle as the branch it is referring to has been deleted? Thanks.