Code review comment for lp://qastaging/~rvb/maas/maas-security-model-api2

Revision history for this message
Gavin Panella (allenap) wrote :

Looks good :)

[1]

+ try:
...
+ except PermissionDenied:
+ return rc.FORBIDDEN

This appears in many places. Is it worth creating a decorator that
changes uncaught PermissionDenied exceptions into rc.FORBIDDEN?

[2]

There's get_visible_node_or_404 and visible_nodes. I think the latter
ought to be renamed get_visible_nodes to match, and so that it doesn't
resemble a property.

[3]

+ :type user: django.contrib.auth.models.User

s/[.]$//

This is shown withing braces in the generated docs and looks funny
with a full-stop at the end.

review: Approve

« Back to merge proposal