Code review comment for lp:~rackspace-titan/nova/metadata_v1.1

It looks like I can post multiple keys as long as one of those is the 'right' key e.g.

PUT /servers/1/meta/key1
{ "key1": "value1", "key2": "value2" }

That's fine with me, but might be unexpected.

I'm really confused by the spec. Sometimes it's meta, sometimes metadata, sometimes there's a values collection, sometimes there's not. So it's a bit difficult to verify this against the spec. But as long as you're consistent (and it looks like you are), we can always fix the spec to match...

This ones a total nit-pick: do you think that delete_instance_metadata should filter by deleted=False, so that we don't keep marking a key as deleted once it is deleted?

However, as a consequence: What happens if I delete a key, and then try to create (or update) the same key. It looks like you'll get a PK violation?

A general discussion point (not for this patch, probably): should our DB layer be throwing exceptions when it doesn't find things? It seems to cause complexity... I like the convention that get_X throws, whereas find_X returns None if not found.

What are we doing about XML formatting? I don't see how this gets marshalled to the correct XML (though it might be - I didn't actually try this) I'd rather see this merged and then XML fixed separately if there are problems. In general, I'd like to see us testing the XML, because the use of schema means that we get a lot of tests "for free".

They're all a bit nit-picky, but I'm marking as "Needs information" for now because collectively I think they're sort of important.