Deepomatic / deepomatic-client-python

Python Client
Other
14 stars 0 forks source link

api errors 4xx and 5xx should be distinguished #25

Closed thomas-riccardi closed 4 years ago

thomas-riccardi commented 6 years ago

Currently all API errors (4xx and 5xx) are exposed identically: BadStatus

https://github.com/Deepomatic/deepomatic-client-python/blob/3640becf728fb0be861e8a50e6e448dd8faecf49/deepomatic/http_helper.py#L185-L186

We should at least distinguish between 4xx and 5xx. Later we could do some retry on 5xx, cf #17

maingoh commented 6 years ago

The user can retrieve the status with exc.status_code

thomas-riccardi commented 6 years ago

OK. Using inference() makes it hard though, as the wait() is done there transparently, and the exception bypasses all that. So we could re-qualify the issue to support 5xx retries in wait().

vdel commented 5 years ago

+1

I had issue with that

vdel commented 5 years ago

What about splitting BadStatus into BadRequest and ServerError ? Getting rid of BadStatus will allow to spot places where the BadStatus is caught in the code that uses this client, in order to take correct actions. We will need to bump to the next version, like 0.9 as we will break the compatibility.

maingoh commented 5 years ago

This could be ClientError instead of BadRequest (Only corresponding to 400 status code)

vdel commented 5 years ago

This could be ClientError instead of BadRequest (Only corresponding to 400 status code)

Indeed