Error handling #62
Error handling #62
Conversation
…ase class for all exoscale exceptions,`ExoscaleAPIClientException` for handling client-side errors 4xx and `ExoscaleAPIServerException` for server-side errors 5xx.
|
Fixes #61 |
There was a problem hiding this comment.
Thanks @elkezza. This looks good but it would be useful to have some basic tests:
- mock the HTTP library to simulate HTTP error codes
- make API requests and ensure they raise the appropriate exceptions
This can be done in tests/test_client.py. Thanks 👍
…ss, it will be raised in any case when the API responds with a 403.
|
@brutasse thank you for your feedback, I will do accordingly. |
tests/test_client.py
Outdated
|
|
||
| # Mock a 500 server error | ||
| requests_mock.get( | ||
| "https://api-ch-gva-2.exoscale.com/v2/500_error", |
There was a problem hiding this comment.
Can you simulate a proper API operation raising a 500? This custom path means you have to resort to using client.session below which isn't part of the documented interface.
There was a problem hiding this comment.
I didn't find any use-case to trigger 5xx error, however in the new commit I tried to mock use-case where the end point is disabled.
Please let me know if any change needed.
tests/test_client.py
Outdated
| except Exception as e: | ||
| assert "503 Server Error" in str(e) |
There was a problem hiding this comment.
| except Exception as e: | |
| assert "503 Server Error" in str(e) |
Not sure why this would be necessary - other than that the adjustment looks good 👍
There was a problem hiding this comment.
Yes it is redundant, good catch, thank you : ).
There was a problem hiding this comment.
@brutasse after your green light, is more reviewer required, to merge the branch?
Co-authored-by: Bruno Renié <[email protected]>
|
ack, thanks! |
elkezza
deleted the
salehelkaza/sc-95846/gh-issue-python-exoscale-implement-proper
branch
Adding exception handling, by introducing
ExoscaleAPIExceptionas base class for all exoscale exceptions,ExoscaleAPIClientExceptionfor handling client-side errors 4xx andExoscaleAPIServerExceptionfor server-side errors 5xx.Tested them locally and works as expected.