Code review comment for lp:~morita-kazutaka/nova/snapshot-volume

Thanks for your comments!

> I haven't really waded into the volume stuff yet, but I thought I'd give a few
> comments from a first pass.
>
> Minor nits:
> - you should bump your database migration version up to the latest in trunk +
> 1

I've merged the latest branch and incremented the version number.

> - we recently made pylint a little happier with our migrations by switching
> from
>
> 265 +from sqlalchemy import *
> 266 +from migrate import *
>
> to
>
> from sqlalchemy import Column, Table # ...
>
> you might want to change your branch to do the same.

Done.

>
> - I see you've added a sensible test to the database api functions you added.
> Is it possible to add some tests to the ec2 api functionality? What about the
> volume api and manager functionality?

I've added a test for EC2 snapshot APIs. It also covers the volume api
functionality. I think all functionalities in this proposal are covered now.

>
> - There is a blueprint coming down the pipe to get rid of database messaging.
> I wonder if it would be possible to adopt something along the lines of that
> approach in this merge. The part that concerns me slightly is the fact that
> both the volume api and volume manager are writing to the database. Perhaps
> more information could be added to the rpc cast so that the db writes in the
> volume api are unnecessary?

Yes, we can remove DB messaging in this proposal.
I think of starting the work after Vish finishes his work on nova-volume:
  https://code.launchpad.net/~vishvananda/nova/no-db-messaging/+merge/61189

Thanks,