> Hi salvatore, thanks for checking with me about this branch.  My main concern
> before was just regarding the issue of state/status, which we've cleared up.
> Below are some minor comments, but overall things look great and I am
> switching my review to Approve.
> 
> quantum/api/api_common.py
> 
> I have a general question about the API validation performed here, based on a
> problem I saw a while back.  The code uses appears to use syntax like "if not
> param_value and param['required']:" to determine if a parameter exists.   This
> can be dangerous, as the above expression will evaluate to false if the
> parameter value is zero, or an empty dictionary, a boolean false, etc. which
> may in fact be valid values.  It seems like really the code should either be
> checking to see if the key exists in the params dictionary, or use an explicit
> check for None (e.g., if param_value is None).

This is a widely used 'pattern' in python programming. It makes me feel a Pythonista although the C++/C#/Java guy which is in me would probably write: 

param_value is None and param['required']

possibly adding a bunch of parenthesis as well :)
On this specific issue, given the way our deserializer currently works, param_value can only be a string. As the deserializer can change in the future, or there might be cases in which an empty string is a valid parameter value, it makes sense to file a bug.

> 
> quantum/api/faults.py
> 
> Is this still left in as a TODO?
> 
> +        #MUST RETURN 202???

I think we discussed in an email thread or earlier meeting this issue. From the spec the API should return 202, but the implementation returns 200. It turns out the Openstack API has this problem as well. We 'inherited' it by grabbing their wsgi framework. I was planning to file a bug and have this problem resolved before integration freeze. It is not trivial and I did not want to halt progress on the API work for this issue.

> 
> quantum/db/api.py,
> 
> definitely feel free to remove the check_duplicate_net_name method, as your
> comment suggests.

I'll file a bug for this; low priority should be enough for this work item.

> 
> quantum/tools/batch_config.py
> 
> Great that you updated the cli.py and tests.  Could you also update
> batch_config.py?  Looks like the name change from 'net-name' at least is an
> issue, though perhaps there are others.
> 

I've updated them, but the tests are not yet there. I have a rather large merge coming through today with tests and some CLI changes (templated output and removal of the direct plugin mode). 

I hope to find some time to fix batch_config as well, but I also need to fix XML deserialization on the client side, so I might run short on time!

> 
> danwent@ubuntu:~/quantum-api-alignment$ PYTHONPATH=. python
> ./tools/batch_config.py foo net1=foo:net2=bar
> nets: {'net2': ['bar'], 'net1': ['foo']}
> Traceback (most recent call last):
>   File "./tools/batch_config.py", line 111, in <module>
>     create_net_with_attachments(client, net_name, iface_ids)
>   File "./tools/batch_config.py", line 46, in create_net_with_attachments
>     res = client.create_network(data)
>   File "/home/danwent/quantum-api-alignment/quantum/client.py", line 53, in
> with_params
>     ret = self.function(instance, *args)
>   File "/home/danwent/quantum-api-alignment/quantum/client.py", line 240, in
> create_network
>     return self.do_request("POST", self.networks_path, body=body)
>   File "/home/danwent/quantum-api-alignment/quantum/client.py", line 174, in
> do_request
>     raise EXCEPTIONS[res.status]()
> quantum.common.exceptions.BadInputError