Sylvanie85/issue61

[sylvanie85]

I did some refactoring as my first issue on nachet-backend.
The issue #61 was deprecated because the code has changed a lot since it was created. But I did make some fixes to the exception handling.

[ChromaticPanic]

Can we move the Error class definitions into a separate module so that we are not redefining them in multiple files?

There is a custom_exceptions.py file

[Francois-Werbrouck]

Can we move the Error class definitions into a separate module so that we are not redefining them in multiple files?

There is a custom_exceptions.py file

The issue this PR is based upon #61 which basically has the goal to remove the custom_exceptions file and redistribute the error to be module based instead

[ChromaticPanic]

Can we move the Error class definitions into a separate module so that we are not redefining them in multiple files?
There is a custom_exceptions.py file

The issue this PR is based upon #61 which basically has the goal to remove the custom_exceptions file and redistribute the error to be module based instead

That's okay for exceptions unique to a module, but some exceptions are shared. For example we do not redefine KeyError because that is something that is common.

For example the proposed approach has ApiErrors defined in multiple files. That seems wrong on so many levels.

[ChromaticPanic]

Can we move the Error class definitions into a separate module so that we are not redefining them in multiple files?
There is a custom_exceptions.py file

The issue this PR is based upon #61 which basically has the goal to remove the custom_exceptions file and redistribute the error to be module based instead

I can elaborate on the issue as follows

Module 1 defines ApiError
Module 2 defines ApiError
Module 3 imports Module 1 and 2 and wants to handle the exceptions = name collision
There are also now 2 potential definitions of what ApiError means, when the reality is that they probably both refer to an issue with connecting an azure end point

Now if ApiError in Module 1 and 2 are in fact different but kinda related then they should inherit from a common parent ApiError but then they should have their own name

[Francois-Werbrouck]

Can we move the Error class definitions into a separate module so that we are not redefining them in multiple files?
There is a custom_exceptions.py file

The issue this PR is based upon #61 which basically has the goal to remove the custom_exceptions file and redistribute the error to be module based instead

That's okay for exceptions unique to a module, but some exceptions are shared. For example we do not redefine KeyError because that is something that is common.

For example the proposed approach has ApiErrors defined in multiple files. That seems wrong on so many levels.

Reviewing the changes I agree. The custom_exception file had too many exceptions in it, which are now module based. However defining APIErrors in 6+ files seems wrong.

[Francois-Werbrouck]

Based on the discussion with @ChromaticPanic we should have a module to inherit the reccuring error Ex: class ProcessInferenceResultsError(APIErrors) :

[sylvanie85]

Based on the discussion with @ChromaticPanic we should have a module to inherit the reccuring error Ex: class ProcessInferenceResultsError(APIErrors) :

So i need to create a new file to inherit the exception as was custom_exeptions ?

[ChromaticPanic]

Based on the discussion with @ChromaticPanic we should have a module to inherit the reccuring error Ex: class ProcessInferenceResultsError(APIErrors) :

So i need to create a new file to inherit the exception as was custom_exeptions ?

Yeah I would make that common ApiError

But also in each file I would use unique names for example in swin.py instead of ProcessInferenceResultsError(ApiError) I would name it SwinError(ApiError) or SwinApiError (I don't know at this point how many error types will be thrown by each module)
Then it would work with the idea of having unique module errors defined in #61

[Francois-Werbrouck]

I like the new exception file and custom naming per module. I willl look into adapting the datastore to this standard!

LGTM! 👍🏻