[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[keystone][placement][neutron][api-sig] http404 to NotFound, or how should a http json error body look like?

Thanks Chris for the reply, let's wait for keystone folks for comments.

On 2019. 05. 17. 13:42, Chris Dent wrote:
> On Fri, 17 May 2019, Lajos Katona wrote:
>> keystoneauth1 expects the http body in case of for example http404 to be
>> something like this (python dict comes now):
>> body= {'error': {'message': 'Foooooo', 'details': 'Baaaaar'}}
>> see:
>> But placement started to adopt one suggestion from the API-SIG:
>> and that is something like this (again python):
>> body={'errors': [{'status': 404, 'title': 'Not Found', 'detail': 'The
>> resource could not be found.... ', 'request_id': '...'}]}
> Thanks for all the detail in this message and on the bug report. It
> helps make understanding what's going on a lot easier.
> As you've discovered placement is following the guidelines for how
> errors are supposed to be formatted. If keystoneauth1 can't speak
> that format, that's probably the primary bug.
> However, it also sounds like you're encountering a few different
> evolutions in placement that may need to be addressed in older
> versions of neutron's placement client:
> * For quite a while placement was strict about responding to Accept
>   headers appropriately. This was based on its interaction with
>   Webob. If you didn't ask for json in the Accept header, errors
>   could come in HTML or Text. The most reliable fix for this in any
>   client of any API is to always send an Accept header that states
>   how you want responses to be presented (in this case
>   application/json). This can lead to interesting parsing troubles
>   if you are rely on the bodies of responses.
> * In microversion 1.23 we started sending 'code' with error
>   responses in an effort to avoid needing to parse error responses.
> I've got a few different suggestions that you might want to explore.
> None of them are a direct fix for the issue you are seeing but may
> lead to some ideas.
> First off, this would be a big change, but I do not think it is good
> practice to raise exceptions when getting 4xx responses. Instead in
> the neutron-lib placement client it would be better to branch on
> status code in the resp object.
> If it doesn't matter why you 404d, just that you did, you could log
> just that, not the detail.
> Another thing to think about is in the neutron placement client you
> have a get_inventory [1] method which has both resource provider
> uuid and resource class in the URL and thus can lead to the "two
> different types of 404s" issue that is making parsing the error
> response required. You could potentially avoid this by implementing
> get_inventories [2] which would only 404 on bad resource provider
> uuid and would return inventory for all resource classes on that rp.
> You can use PUT on the same URL to replace all inventory on that
> rp.
> Make sure you send Accept: application/json in all your requests in
> the client.
> Make keystoneauth1 interpret two diferent types of errors response:
> the one it does, and the one in the api-sig guidelines. Note that
> I've yet to see any context where there is more than one error in
> the list, so it is always errors[0] that gets inspected.
> On the placement side we should probably add codes to the few
> different places where a 404 can happen for different reasons (they
> are almost all combinations of not finding a resource provider or
> not finding a resource class). If that were present you could branch
> the library code on the code in the errors structure instead of the
> detail. However if its possible to avoid choosing which 404, that's
> probably better.
> I hope some of that helps. Hopefully some keystoneauth folk will
> chime in too.
> [1]
> [2]