git.net

[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:
>> https://protect2.fireeye.com/url?k=e824ec9f-b4f7fcf7-e824ac04-8691b328a8b8-1c7b10b9a8d8e9c8&q=1&u=https%3A%2F%2Fopendev.org%2Fopenstack%2Fkeystoneauth%2Fsrc%2Fbranch%2Fmaster%2Fkeystoneauth1%2Fexceptions%2Fhttp.py%23L406-L415 
>>
>>
>> But placement started to adopt one suggestion from the API-SIG:
>> http://specs.openstack.org/openstack/api-wg/guidelines/errors.html,
>>
>> 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]
> https://protect2.fireeye.com/url?k=301a51a6-6cc941ce-301a113d-8691b328a8b8-87f5962f00f50154&q=1&u=https%3A%2F%2Fopendev.org%2Fopenstack%2Fneutron-lib%2Fsrc%2Fbranch%2Fmaster%2Fneutron_lib%2Fplacement%2Fclient.py%23L417k 
>
>
> [2] 
> https://developer.openstack.org/api-ref/placement/#resource-provider-inventories