Code review comment for lp://staging/~adeuring/launchpad/api-query-permissions-on-object

Revision history for this message
Robert Collins (lifeless) wrote :

I have a couple of concerns here.

Firstly, if these are properties they will be calculated on -every- render of the object. Permission checks are not always cheap and are sometimes very expensive. We will be shooting our performance in the foot.

Secondly, will it scale performance wise ;)

I think that rather than calculating every possible permission on every API generation we should instead permit specifying the precise things needed for a page and only calculate those.

I'm very confident that the overhead of this will mount up, so I'm going to needsfixing this and put the branch into in-progress. Sorry :(.

I'm happy to talk/IRC/email about implementation strategies if needed; offhand it looks to me like any given page knows the permissions that will be needed a-priori so can inject them into the json cache at runtime. This is probably simpler and may remove the need to expose the userCanWrite logic etc entirely.

review: Needs Fixing

« Back to merge proposal