Code review comment for lp:~vishvananda/nova/orm_deux

Revision history for this message
termie (termie) wrote :

Re: Soren, why they take a values object.

There were already even in just making these classes instances where the values we were updating included 'instance_id' and the like, having those as kwargs resulted in duplicate values when using positional and keyword args. In the best case we would write out all the paremters that can be changed in the function signature and document them all but that seemed like a lot to ask at this point.

Re: Eric, context as a param to a function vs as an instance attribute in a method. There are a few reasons.

1. Context can be different from call to call as context is intended to include the permissions of the role who is making the call. An example of this would be us having to make background bookkeeping changes when a user changes something they own, the user is allowed to make the call for the initial change but the bookkeeping may require higher privileges so a different context would be passed to it.

2. Functional interfaces are significantly easier to test than OO ones that include state.

3. If you don't have the context param there, something like:

db.user_get(context, blah)

will usually become:

db = Db.fromContext(context)
db.user_get(blah)

So even in the case where the context would be static throughout the calls, you still end up instantiating new db objects in every method down the chain or passing along 'db' keyword args that do the same thing as context. In most cases we are only making one or two calls with the db so the extra boilerplate becomes more trouble than it is worth.

« Back to merge proposal