Feature/exception handler #26
Conversation
bunq/sdk/exception.py
Outdated
| _GLUE_ERROR_MESSAGES = '\n' | ||
|
|
||
| def __init__(self, response_code, messages): | ||
| class BunqError(Exception): |
There was a problem hiding this comment.
Let's call the base class ApiException and then extend from it. For the case of an unknown exception, we can throw the ApiException itself or make something like UnknownApiErrorException!
bunq/sdk/exception_hanlder.py
Outdated
| from bunq.sdk.exception import BunqError | ||
|
|
||
|
|
||
| class ExceptionHandler(Exception): |
There was a problem hiding this comment.
It is not handler. Let's perhaps call it ExceptionFactory?
|
@dnl-blkv please 👀 and ✅ the first item on the todo list above if you agree plz. |
|
@OGKevin Current state is good! Please proceed with the .md :) |
|
@dnl-blkv please final 👀 |
bunq/sdk/client.py
Outdated
| @@ -6,6 +6,7 @@ | |||
|
|
|||
| from bunq.sdk import context | |||
| from bunq.sdk import exception | |||
| from bunq.sdk.exception_factory import ExceptionFactory | |||
There was a problem hiding this comment.
@OGKevin Oh, just realised one thing: is the order of import statements correct here? :D
There was a problem hiding this comment.
So was it correct? :D
Feels like organise imports would change it :)
There was a problem hiding this comment.
Well there is nothing wrong with these statements :P Its grouped correctly. The thing that reformat does is from bunq.sdk import context, exception, security which is against PEP8
EXCEPTIONS.md
Outdated
| @@ -0,0 +1,73 @@ | |||
| ## Exceptions | |||
|
|
|||
There was a problem hiding this comment.
Like in every SDK, this newline can go (just internal formatting :D)
EXCEPTIONS.md
Outdated
|
|
||
| ---- | ||
| #### Possible Exceptions | ||
|
|
|
|
||
| def glue_messages(self, messages): | ||
| @property | ||
| def message(self): |
There was a problem hiding this comment.
Can't we use the base class property here?
There was a problem hiding this comment.
Hmm well python handles the message a little bit different tho take a look here: https://stackoverflow.com/questions/4690600/python-exception-message-capturing
So what do you think based on this🤔
There was a problem hiding this comment.
@OGKevin thanks for explanation! Let's keep it as we have it now :)
bunq/sdk/client.py
Outdated
| @@ -6,6 +6,7 @@ | |||
|
|
|||
| from bunq.sdk import context | |||
| from bunq.sdk import exception | |||
| from bunq.sdk.exception_factory import ExceptionFactory | |||
There was a problem hiding this comment.
So was it correct? :D
Feels like organise imports would change it :)
#25
Exception factory review
Explain exceptions in a .md file
Final review