Clean up caching logic (don't swallow 4xx responses)

[jimallman]

We were previously discarding the contents of non-200 responses, but we want to be more discriminating. We should return any payload if possible, regardless sub-500 responses, AND cache the response.

NB - This is working now on devtree

[bredelings]

This works, but looks a little funny. I think we can write status_code=... instead of status=..., but I might be wrong.

[jimallman]

Yes, I see that pyramid.response.Response has three(!) status properties, which could easily get out of sync:

• status (descriptive string, starting with status code)
• status_code (as number)
• status_int (as integer)

Out of curiosity, I tried changing each of these in a local debugger, and in all cases they stay in sync:

(Pdb) test
<Response at 0x1069f00d0 200 OK>
(Pdb) test.status
'200 OK'
(Pdb) test.status_code
200
(Pdb) test.status_int
200

Now let's assert a bogus status code+string:

(Pdb) test.status = "499 fooBAR"
(Pdb) test.status
'499 fooBAR'
(Pdb) test.status_code
499
(Pdb) test.status_int
499

And let's try some known status codes (no string provided):

(Pdb) test.status_code = 403
(Pdb) test.status
'403 Forbidden'
(Pdb) test.status_int
403
(Pdb) test.status = 301
(Pdb) test.status
'301 Moved Permanently'
(Pdb) test.status_int
301
(Pdb) test.status_code
301

So it's pretty robust! But yes, for consistency I'll do as you suggest above. For a known-good status code, the correct string will be assigned automatically. If an odd code shows up (e.g. 499), a general description will be provided based on the initial digit:

(Pdb) test.status_code = 499
(Pdb) test.status
'499 Unknown Client Error'

[jimallman]

NB These changes are now live on dev, if you want to kick the tires.

[jimallman]

This change is working well for me on dev, try for example:

curl -v https://devapi.opentreeoflife.org/v3/tree_of_life/subtree -d '{"format":"arguson","height_limit":3,"node_id":"ott372706"}'  -vvv

This returns the complete payload, but with a 400 Bad Request status, as desired:

        // lots of JSON payload
        },
        "mrca": "mrcaott47497ott110766"
    },
    "message": "[/v3/tree_of_life/subtree] Error: node_id was not found (broken taxon).\n"
}