Merge lp:~benji/lazr.restful/bug-413174 into lp:lazr.restful
Status: | Merged |
---|---|
Approved by: | Gary Poster |
Approved revision: | 146 |
Merged at revision: | 145 |
Proposed branch: | lp:~benji/lazr.restful/bug-413174 |
Merge into: | lp:lazr.restful |
Diff against target: |
363 lines (+208/-31) 7 files modified
src/lazr/restful/_resource.py (+40/-25) src/lazr/restful/configure.zcml (+9/-0) src/lazr/restful/directives/__init__.py (+1/-1) src/lazr/restful/error.py (+38/-3) src/lazr/restful/interfaces/_rest.py (+15/-0) src/lazr/restful/testing/webservice.py (+17/-2) src/lazr/restful/tests/test_error.py (+88/-0) |
To merge this branch: | bzr merge lp:~benji/lazr.restful/bug-413174 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gary Poster | Approve | ||
Review via email: mp+34686@code.launchpad.net |
Description of the change
When raising exceptions it is sometimes useful to allow web service API
users to see the error details. OOPS-1318S626 and OOPS-1694EB1486 are
examples of this kind of situation.
Therefore this branch adds a method (lazr.restful.
lets you signal to lazr.restful that the message associated with the
error should be relayed to the web service API user.
The approach was discussed with Gary Poster and Leonard Richardson.
The expose function decorates the exception instance with a marker
interface which has an exception view (ClientErrorView) associated with
it.
The test for this feature can be run with the command
bin/test -t TestLaunchpadlibAPI
Very nice catch with the check for isinstance of BaseException.
I question the view pattern for the registration and lookup--using "index.html" as the distinguishing marker. I'd prefer a custom interface myself. If, in retrospect, you agree, great. If not, I can see where you are coming from: leave as is.
We need a linter. Either ``from types import ClassType`` in src/lazr/ restful/ error.py shouldn't be there now, or it should have been caught earlier. Which is it?
In error.py, you say "# Bug 11512 describes the possibility of controlling the HTTP status of...." I think you mean bug 631711.