Code review comment for lp://qastaging/~justin-fathomdb/nova/justinsb-openstack-api-volumes

OK, I've re-read this patch about 7 times now. I hate to do this, but I'm going to say Disapprove on this one. What is unfortunate is that this patch gloms together a whole bunch of *new* stuff along with tests of that new stuff. It doesn't actually add tests for the existing OpenStack API functionality, which is what was needed for Cactus stability. I believe this patch only destabilizes Cactus by adding "test-only" functionality to the OpenStack API that isn't even in the OpenStack API spec and then testing these new codepaths while not testing anything else.

I'm sorry, Justin, but this code just doesn't pass muster on something to go into Cactus, IMHO.

If this patch (and a number of the merge props that depend on it) were broken down into more manageable chunks, and chunks that had bugs and blueprints specific to Cactus, it may have been doable, but the string of merge proposals that this depends on as well as this patch just don't work for me.

If others feel very strongly about this, feel free to comment on this merge proposal. I have to move on to the other merge proposals tied to specific blueprints for Cactus, though.

-jay