Code review comment for lp:~chemikadze/nova/contrib-extention-networks

Revision history for this message
Nikolay Sokolov (chemikadze) wrote :

Matt, thanks for your comments.

I checked admin api bluprints and now see that unlike openstackx, core nova extentions doesn't use special admin route. So I changed it to common-style 'os-networks'.

Copyrights were made like in floating ips extention, and I thought that if it is in trunk, then it is ok. I'm master in legal issues, but reviewing other sources that my patch is outstanding in that case too, and I fixed that.

But the last one is not clear to me: I agree with thought, that tests passing on mock underlaying code and broken with real code is not good (by the way, just today i've read Sandy's article about that). I tried to find sample of how to deal with that, but I didn't find it neither in extention tests, nor in core OS API tests. In that particular case, I think there is possible to write separate test case, just checking that "fake net" object fits to database. But in general, I'm confused that other tests doesn't fit that requirement. From the other side, isn't verifying backward compatability a problem of database tests?

« Back to merge proposal