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

Revision history for this message
Jay Pipes (jaypipes) wrote :

Yo!

Overall, I do like the new interface. I agree with Eric that an object-oriented approach would be nice, but I certainly wouldn't disapprove this because of that. :)

Some tiny nits to note and deal with at some later point:

* redis_db flag should be removed
* I recommend changing the name of the db_backend flag to db_driver to match other flags (like compute_driver
* Typos:
 s/Represates/Represents/g
 s/Rignt/Right/g
* For these new flags:

4174 +DEFINE_string('compute_manager', 'nova.compute.manager.ComputeManager',
4175 + 'Manager for compute')
4176 +DEFINE_string('network_manager', 'nova.network.manager.VlanManager',
4177 + 'Manager for network')
4178 +DEFINE_string('volume_manager', 'nova.volume.manager.AOEManager',
4179 + 'Manager for volume')

I'm wondering why the need for those? Aren't the manager classes fixed and become adaptable via their internal drivers? For instance, the compute manager uses the flag compute_driver for its adaptation, no?

Or am I missing something?

Cheers!
jay

review: Approve

« Back to merge proposal