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

Revision history for this message
Eric Day (eday) wrote :

Here are my notes and questions from the first pass through all this:

What is context used for in all the DB calls? Is it really needed? If it is, I think we should create instances of various objects and set it as a class instance member, not passed through to every method.

Why do we have services and managers? Seems like services should just be removed, or move manager->service. If we need a service wrapper around the manager, we could pass a manager class into nova/service.py directly to create the service, removing nova/{compute,network,...}/service.py files entirely.

nova/datastore.old.py
Why is this still here?

nova/db/api.py
This should either be an interface class that IMPL overrides or not even have an API class and just document what methods an IMPL needs to provide.

nova/db/sqlalchemy/api.py
Again, class with methods (context being an instance member) would be cleaner. A
lso, some methods accept _context but then pass them through to other methods. T
he underscore should be removed in these cases.

nova/db/sqlalchemy/models.py
Why doesn't NovaBase inherit from BASE, instead of all models?

nova/virt/libvirt_conn.py
Text conflict in nova/virt/libvirt_conn.py, re-merge with trunk.

run_tests.py
Replace/remove nova.tests.model_unittest?

review: Needs Information

« Back to merge proposal