Code review comment for lp:~rackspace-titan/nova/openstack-api-version-split

Revision history for this message
Naveed Massjouni (ironcamel) wrote :

> 40: nova.context should not hold anything openstack API specific. You probably
> want to store version in the req.environ dict if you need it somewhere.

Done. Thanks.

>
> 592: how did that get in there? :)

Modified that line to make pep8 happy, after merge from trunk.

>
> Overall, I'm not sure I agree with the approach. It versions the
> serialization, but not the controllers. I'm wondering if we should instead use
> the old serialization conventions and just make versioned controllers that
> would call the correct one. You would then specify versioned controllers in
> the paste config which seems a bit more straightforward.

I agree that controllers should also be versioned. The good thing about this merge prop as it stands is that it separates out the view logic from the controller logic. We considered versioning controllers as well. We decided to hold off on that until it became necessary for a feature we were working on. We will be working in that direction, as we are implementing rest of 1.1 features for the openstack api in the next couple of days. We wanted to make our approach public as soon as possible to get feedback.

« Back to merge proposal