Merge lp:~salvatore-orlando/neutron/quantum-api-alignment into lp:neutron/diablo

[Salvatore Orlando]

This is the branch that updates the API code in order to make it 100% compliant with the API specification.

This branch DOES NOT include the status proposal, but includes all feedback received by the community.

API Unit tests have been adapted were necessary(e.g.: attribute names changed)
Client Library Unit tests pass as well.

[dan wendlandt (danwent)]

Hi salvatore, haven't run it yet and only skimmed the test code, but looks good overall.

I do think though that we need to get to the bottom of the "state + status" discussion before pushing this branch, as even this branch had some things that surprised me, particularly that one could not make an attachment to a port in the down state. This seems at odds to my understanding from the email thread that "state" was really akin to "admin state" on a traditional switch. When a switch is "admin down", my understanding is that the common behavior one can still configure the port, its just that no packets will pass.

Also, the spec still seems somewhat unclear on the issue of port "state". For example, the following section, as I interpret it, seems to imply that the state is either "down" or "active" based on whether a VIF is plugged in (i.e., something similar to the operational "link status" we discussed in the last email):

"The VIF will be connected to the network via a logical port. The logical port begins in a “down” state. Logic ports are transient when in the “down” state, by this we mean they don’t have any binding to a physical implementation. The plugin must create this binding as a part of the attachment process. When a VIF is attached to a logical port the plugin creates the binding to the physical hardware required to allow operation of the VIF. This is indicated by the change in port state from “down” to “active”. As a result of the attachment operation, quantum can now knows the capabilities of the VIF as dictated by the physical binding. As a result the logical port may advertise capabilities once it moves to the “active” state."

If you'd prefer to push this code first to avoid merge conflicts and then continue having the discussion that's fine with me, but I want to make sure we clear up this point before finalizing 1.0 (An alternative would be to drop the state/status notion all together for v1.0, and revisit it for a later version).

[Salvatore Orlando (salvatore-orlando)]

> Hi salvatore, haven't run it yet and only skimmed the test code, but looks
> good overall.
>
> I do think though that we need to get to the bottom of the "state + status"
> discussion before pushing this branch, as even this branch had some things
> that surprised me, particularly that one could not make an attachment to a
> port in the down state. This seems at odds to my understanding from the email
> thread that "state" was really akin to "admin state" on a traditional switch.
> When a switch is "admin down", my understanding is that the common behavior
> one can still configure the port, its just that no packets will pass.
>
> Also, the spec still seems somewhat unclear on the issue of port "state". For
> example, the following section, as I interpret it, seems to imply that the
> state is either "down" or "active" based on whether a VIF is plugged in (i.e.,
> something similar to the operational "link status" we discussed in the last
> email):
>
> "The VIF will be connected to the network via a logical port. The logical port
> begins in a “down” state. Logic ports are transient when in the “down” state,
> by this we mean they don’t have any binding to a physical implementation. The
> plugin must create this binding as a part of the attachment process. When a
> VIF is attached to a logical port the plugin creates the binding to the
> physical hardware required to allow operation of the VIF. This is indicated by
> the change in port state from “down” to “active”. As a result of the
> attachment operation, quantum can now knows the capabilities of the VIF as
> dictated by the physical binding. As a result the logical port may advertise
> capabilities once it moves to the “active” state."
>
> If you'd prefer to push this code first to avoid merge conflicts and then
> continue having the discussion that's fine with me, but I want to make sure we
> clear up this point before finalizing 1.0 (An alternative would be to drop the
> state/status notion all together for v1.0, and revisit it for a later
> version).

I think there are enough comments and questions to drop everything related to "resource state" in any form.
It looks like we all feel the need for some sort of notion of "state", but we are coming at it from rather different angles.
I therefore think it would be better to remove any state concept, including the administrative state for the port, and then resume the discussion post Diablo-4, finalize it at the summit in October, and include the state notion in API v1.1

[Salvatore Orlando (salvatore-orlando)]

Reverting to WIP to remove all references to resource state.

[dan wendlandt (danwent)]

One other question for you: as part of nova + quantum integration, I'm prototyping the "admin API" interfaces to communicate information like interface-ownership (for authenticating attachments) and interface-bindings (for knowing where a vif was deployed).

My gut says that we should not hold up the 1.0 version while waiting to include these items, but I wanted to get your thoughts there. We definitely will need to document these eventually though, as there will be other openstack services that want to integrate with Quantum.

review: Needs Information

[Salvatore Orlando (salvatore-orlando)]

> One other question for you: as part of nova + quantum integration, I'm
> prototyping the "admin API" interfaces to communicate information like
> interface-ownership (for authenticating attachments) and interface-bindings
> (for knowing where a vif was deployed).
>
> My gut says that we should not hold up the 1.0 version while waiting to
> include these items, but I wanted to get your thoughts there. We definitely
> will need to document these eventually though, as there will be other
> openstack services that want to integrate with Quantum.

Good point. I guess the admin API has a different endpoint, as it should be exposed by nova, is that correct?
In that case I'm not entirely sure this should be part of the Quantum API at all (for instance if it going to be an Openstack API extension, why should it be documented in Quantum API). I can't find a blueprint for tracking this piece of work, can you give me a pointer?

My plan to lock down the API for this week is motivated by the fact that we have to lock down features by Aug 25, and then do bug fixing and system testing until diablo release. If you think that we only have two weeks left, and given that code review process usually takes a week, this means that the best way to ensure a timely completion for the API blueprint is to start locking down the spec this week, and then work the week after on the code.

[dan wendlandt (danwent)]

I think this is the right call. We don't necessarily have to wait that long
(or even to stop talking about it at all, but I agree with getting v1.0 out
ASAP).

dan

On Mon, Aug 8, 2011 at 2:47 PM, Salvatore Orlando <
<email address hidden>> wrote:

> > Hi salvatore, haven't run it yet and only skimmed the test code, but
> looks
> > good overall.
> >
> > I do think though that we need to get to the bottom of the "state +
> status"
> > discussion before pushing this branch, as even this branch had some
> things
> > that surprised me, particularly that one could not make an attachment to
> a
> > port in the down state. This seems at odds to my understanding from the
> email
> > thread that "state" was really akin to "admin state" on a traditional
> switch.
> > When a switch is "admin down", my understanding is that the common
> behavior
> > one can still configure the port, its just that no packets will pass.
> >
> > Also, the spec still seems somewhat unclear on the issue of port "state".
> For
> > example, the following section, as I interpret it, seems to imply that
> the
> > state is either "down" or "active" based on whether a VIF is plugged in
> (i.e.,
> > something similar to the operational "link status" we discussed in the
> last
> > email):
> >
> > "The VIF will be connected to the network via a logical port. The logical
> port
> > begins in a “down” state. Logic ports are transient when in the “down”
> state,
> > by this we mean they don’t have any binding to a physical implementation.
> The
> > plugin must create this binding as a part of the attachment process. When
> a
> > VIF is attached to a logical port the plugin creates the binding to the
> > physical hardware required to allow operation of the VIF. This is
> indicated by
> > the change in port state from “down” to “active”. As a result of the
> > attachment operation, quantum can now knows the capabilities of the VIF
> as
> > dictated by the physical binding. As a result the logical port may
> advertise
> > capabilities once it moves to the “active” state."
> >
> > If you'd prefer to push this code first to avoid merge conflicts and then
> > continue having the discussion that's fine with me, but I want to make
> sure we
> > clear up this point before finalizing 1.0 (An alternative would be to
> drop the
> > state/status notion all together for v1.0, and revisit it for a later
> > version).
>
> I think there are enough comments and questions to drop everything related
> to "resource state" in any form.
> It looks like we all feel the need for some sort of notion of "state", but
> we are coming at it from rather different angles.
> I therefore think it would be better to remove any state concept, including
> the administrative state for the port, and then resume the discussion post
> Diablo-4, finalize it at the summit in October, and include the state notion
> in API v1.1
> --
>
> https://code.launchpad.net/~salvatore-orlando/quantum/quantum-api-alignment/+merge/70788
> You are requested to review the proposed merge of
> lp:~salvatore-orlando/quantum/quantum-api-alignment into lp:quantum.
>

--
~~~~~~~~~~~~~~~~~~~~~~~~~~~
Dan Wendlandt
Nicira Networks, Inc.
www...

[dan wendlandt (danwent)]

On Mon, Aug 8, 2011 at 3:13 PM, Salvatore Orlando <
<email address hidden>> wrote:

> > One other question for you: as part of nova + quantum integration, I'm
> > prototyping the "admin API" interfaces to communicate information like
> > interface-ownership (for authenticating attachments) and
> interface-bindings
> > (for knowing where a vif was deployed).
> >
> > My gut says that we should not hold up the 1.0 version while waiting to
> > include these items, but I wanted to get your thoughts there. We
> definitely
> > will need to document these eventually though, as there will be other
> > openstack services that want to integrate with Quantum.
>
> Good point. I guess the admin API has a different endpoint, as it should be
> exposed by nova, is that correct?
>

I think one could think of doing it in two ways:

1) exposing an endpoint on nova, with quantum as the client.
2) exposing an endpoints on quantum, with nova as the client.

My mental model had always been #2, but I'm not wedded to it.

#2 seemed natural to me as the "interface service" (e.g., nova) seemed to
need to be able to "push" an interface binding to quantum (e.g., in the case
where a VM migrates, changing the binding). Also, requring "interface
service" to make a JSON call (ideally with the help of the client lib)
seemed easier than requiring each interface service to expose and
authenticate a new API call. This would be particularly true if an interface
service was already acting as a quantum client for other reasons, which is
the case with nova.

One case where #2 is a bit funky is if quantum is deployed after an
interface service like nova is already running (and thus has already spun up
a set of VMs). It would seem that nova would need to support a method where
it "resends" all of its data. Seems possible, but kind of clunky.

Do you have a strong feeling on this one?

> In that case I'm not entirely sure this should be part of the Quantum API
> at all (for instance if it going to be an Openstack API extension, why
> should it be documented in Quantum API). I can't find a blueprint for
> tracking this piece of work, can you give me a pointer?
>

The original blueprint for this work is:
https://blueprints.launchpad.net/quantum/+spec/quantum-interface-binding-api

There's not a whole lot there. I'm hoping to do this as part of the Quantum
Manager work in nova:
https://blueprints.launchpad.net/nova/+spec/implement-network-api

Just last night I started stubbing out out a possible Controller for this
admin API. I've decided my first cut wasn't how I wanted it to work, so I'm
going to tweak it (hopefully tonight), then I will push the branch and see
what feedback people have. Main goals are:

1) transmit "vif ownership" info to quantum, so we can tell what tenant owns
a vif (and thus whether they are allowed to plug it in to one of their
ports).
2) for plugins that need it, have a channel to report interface bindings.

#2 is somewhat trickier (not clear if it should live in the network-manager,
in the vif-plug driver, somewhere else), but #1 seems pretty
straightforward.

>
> My plan to lock down the API for this week is motivated by the fact that we
> ha...

[Salvatore Orlando (salvatore-orlando)]

Re-proposing for merge.
Community feedback has been addressed.

[lp:~salvatore-orlando/neutron/quantum-api-alignment]

44. By Salvatore Orlando on 2011-08-17

Removing unused error response codes

[Tyler Smith (tylesmit)]

A conflict file accidentally got added:
quantum/tests/unit/test_clientlib.py.THIS

Other than that, LGTM.

review: Needs Fixing

[lp:~salvatore-orlando/neutron/quantum-api-alignment]

45. By Salvatore Orlando on 2011-08-22

Removing "excess" file

46. By Salvatore Orlando on 2011-08-22

Merge trunk
Solving conflict in common/exceptions.py
Removing unused error code from client.py

[Tyler Smith (tylesmit)]

Great work getting a coherent API spec implemented; looks good.

review: Approve

[Somik Behera (somikbehera)]

Hi Salvatore,

This is a good step in the right direction, towards 1.0!

A reviewed, this changeset, the API wiki, quantum_plugin_base that species the data structure that plugins are mandated to use while passing data to the API layer to transform into XML/JSON for external world to consume.

I see a few shortcomings:

1) quantum_plugin_base.py specifies the "interface" for plugin developers, so that plugin developers can follow this file and implement their plugins without looking at any other place. Along those lines, this "interface" file also specifies the data structure/ "format" that plugins are supposed to use while returning data to the API layer.

Currently, the API layer's expectation regarding data "format" are not aligned with what quantum_plugin_base specifies. Therefore, one or other, or both might need fixing.

2) I noticed we upped the version to /v1.0 and I just wanted ask you if we have appropriate bugs on client-side, if any, already filed to ensure that the client is compliant.

3) One thing I have noticed that is not very consistent, within the API, Unit test and documentation implementation is the data format of for the "attachment"

3.1) This is what plugin_base specifies( API can provide a different format to Northbound API client while keeping the specified format on southbound side plugin side):

    @abstractmethod
    def get_port_details(self, tenant_id, net_id, port_id):
        """
        This method allows the user to retrieve a remote interface
        that is attached to this particular port.

        :returns: a mapping sequence with the following signature:
                    {'port-id': uuid representing the port on
                                 specified quantum network
                     'net-id': uuid representing the particular
                                quantum network
                     'attachment': uuid of the virtual interface
                                   bound to the port, None otherwise
                    }

3.2) Here's the attachment format for Northbound API from wiki:

{
 "attachment":
     {
         "id": "test_interface_identifier"
     }
}

3.3) file quantum/api/views/attachments.py looks for "attachment-id" when looking for the identifier representing the attachment, which is inconsistent on the southbound(quantum_plugin_base) and northbound( API wiki) side.

Looking at file history, it looks like its being changed from attachment -> attachment-id

I would prefer that the southbound side( quantum_plugin_base specification) should be left as attachment if there is not good reason to change the naming as it will break compatibility with all the plugins that have been written, and won't provide any enhanced functionality. We can definitely change things northbound as thats our public API.

3.4) Similarly, net-name was changed to "name" on northbound and southbound side, I have similar feelings that we keep the the southbound side( data format specified in quantum_plugin_base) be kept same while changing northbound. Same is try for port-state -> "state" change.

4) Based on final conclusions, we will need to update unit tests and quantum_plugin_base.py doc as well. I am...

review: Needs Fixing (netstack-core)

[Salvatore Orlando (salvatore-orlando)]

Hi Somik,

thanks for your review.

I understand your concerns about different the different data structures in northbound/southbound interfaces. After all, those are exactly the concerns I had when we discussed a uniform data model for the API and the plugin interface a few weeks ago, and I thought we concluded that the API and the plugin interface represented two distinct interfaces.

If we decide to unify these interface, I'd like to use the current API spec as a guideline for changing the plugin interface and not vice-versa, as I would like to keep Quantum API as close as possible the ones exposed by Openstack and other services in the ecosystem. However, this is not a "conditio sine qua non" for me.

You did pretty well in summarizing the differences between northbound and southbound interfaces, I would also add the following:
1) net-id --> id
2) port-id --> id
3) port-state --> state
But I have to agree the attachment-id thing you noticed in your comment #2 is actually a bug, which is due to the fact that the FakePlugin does not implement very well the plugin interface. API code, FakePlugin, and unit tests will be fixed accordingly.

If we want the northbound and southbound interface to have the same shape, then having two interfaces (API and quantum_plugin_base) is not a good idea. We can just decide that the plugin interface is "THE" interface, and the only responsibility of the API layer is converting data structures into the appropriate format; or we can do the opposite. Either way, plugins or clients will be affected.

But, at this stage, I don't think we really need to unify these interfaces. Plugin implementors will talk the language of the plugin interface, while API users will talk the language of the API interface. The API layer will "translate" between these two languages (after all it is not a big effort, as it can be seen from the view builders, which are very small modules). Another advantage of separating interfaces is that the API can evolve indepedently of the plugin interface, and vice-versa; for instance, in the future, object ownership information, fetched from the identity service, could be added to API data structures without interfering at all with the plugin interface.

Please let me know what do you think of my proposal.
I'm totally open to unify API and quantum_plugin_base; my proposal of keeping the interfaces separate arises from the considerations reported above.

Thanks again,
Salvatore

> Hi Salvatore,
>
> This is a good step in the right direction, towards 1.0!
>
> A reviewed, this changeset, the API wiki, quantum_plugin_base that species the
> data structure that plugins are mandated to use while passing data to the API
> layer to transform into XML/JSON for external world to consume.
>
> I see a few shortcomings:
>
> 1) quantum_plugin_base.py specifies the "interface" for plugin developers, so
> that plugin developers can follow this file and implement their plugins
> without looking at any other place. Along those lines, this "interface" file
> also specifies the data structure/ "format" that plugins are supposed to use
> while returning data to the API layer.
>
> Currently, the API layer's...

[Somik Behera (somikbehera)]

Hi Salvatore,

Overall, I am in line with your thinking and I think thats a good way to
proceed.

Rest inline.

On Mon, Aug 22, 2011 at 12:43 PM, Salvatore Orlando <
<email address hidden>> wrote:

> Hi Somik,
>
> thanks for your review.
>
> I understand your concerns about different the different data structures in
> northbound/southbound interfaces. After all, those are exactly the concerns
> I had when we discussed a uniform data model for the API and the plugin
> interface a few weeks ago, and I thought we concluded that the API and the
> plugin interface represented two distinct interfaces.
>
> If we decide to unify these interface, I'd like to use the current API spec
> as a guideline for changing the plugin interface and not vice-versa, as I
> would like to keep Quantum API as close as possible the ones exposed by
> Openstack and other services in the ecosystem. However, this is not a
> "conditio sine qua non" for me.
>
> You did pretty well in summarizing the differences between northbound and
> southbound interfaces, I would also add the following:
> 1) net-id --> id
> 2) port-id --> id
> 3) port-state --> state
> But I have to agree the attachment-id thing you noticed in your comment #2
> is actually a bug, which is due to the fact that the FakePlugin does not
> implement very well the plugin interface. API code, FakePlugin, and unit
> tests will be fixed accordingly.

> If we want the northbound and southbound interface to have the same shape,
> then having two interfaces (API and quantum_plugin_base) is not a good idea.
> We can just decide that the plugin interface is "THE" interface, and the
> only responsibility of the API layer is converting data structures into the
> appropriate format; or we can do the opposite. Either way, plugins or
> clients will be affected.
>
>
[SB] I agree that API layer can convert to the quantum_plugin_base -> API
format. Since this change is an API change, we can't really prevent API
clients from getting affected. But, if we don't have pressing need to change
quantum_plugin_base then the atleast plugins wont be affected, which would
be good for this late stage change.

> But, at this stage, I don't think we really need to unify these
> interfaces. Plugin implementors will talk the language of the plugin
> interface, while API users will talk the language of the API interface. The
> API layer will "translate" between these two languages (after all it is not
> a big effort, as it can be seen from the view builders, which are very small
> modules). Another advantage of separating interfaces is that the API can
> evolve indepedently of the plugin interface, and vice-versa; for instance,
> in the future, object ownership information, fetched from the identity
> service, could be added to API data structures without interfering at all
> with the plugin interface.
>
>
[SB] I totally agree with this line of thinking and this would be a great
way to proceed. This approach causes least disruption and provides greatest
flexibility.

> Please let me know what do you think of my proposal.
> I'm totally open to unify API and quantum_plugin_base; my proposal of
> keeping the interfaces separate arise...

[lp:~salvatore-orlando/neutron/quantum-api-alignment]

47. By Salvatore Orlando on 2011-08-22

Fixing issue in view builders concerning attachment identifiers

[Salvatore Orlando (salvatore-orlando)]

Hi Somik,

thanks for your reply.
It seems we have now an agreement and can finally close down API v1 for Quantum.
I've updated the branch for taking into account the issue with the 'attachment-id' identifier you spotted.

However, prompted by your review, I double-checked consistency with specification in quantum-plugin-base, and I spotted a problem with get_network_details.
Quantum_plugin_base states that id should return a sequence whose elements have the following attributes: 'net-id', 'net-name', 'net-ifaces', with the latter being a list of interface identifiers.

However:
- the API does not look for 'net-ifaces' at all. It instead invoke get_all_ports and get_port_details for each port; the API therefore works, but not in the most efficient way. This could be fixed quite easily;
- however the plugins (FakePlugin, Cisco, and even Openvswitch), return a sequence
whose elements have the following attributes: 'net-id', 'net-name', 'net-ports' (instead of 'net-ifaces').

I think we probably want to sort this out. We can either change plugin interface documentation, or the plugins. Whatever we decide, we should also update the API to ensure it works in an efficient way.

What's the best approach to tackle this problem in your opinion?

Cheers,
Salvatore

[lp:~salvatore-orlando/neutron/quantum-api-alignment]

48. By Salvatore Orlando on 2011-08-24

Merge trunk

[Somik Behera (somikbehera)]

> Hi Somik,
>
> thanks for your reply.
> It seems we have now an agreement and can finally close down API v1 for
> Quantum.
> I've updated the branch for taking into account the issue with the
> 'attachment-id' identifier you spotted.
>
> However, prompted by your review, I double-checked consistency with
> specification in quantum-plugin-base, and I spotted a problem with
> get_network_details.
> Quantum_plugin_base states that id should return a sequence whose elements
> have the following attributes: 'net-id', 'net-name', 'net-ifaces', with the
> latter being a list of interface identifiers.
>
> However:
> - the API does not look for 'net-ifaces' at all. It instead invoke
> get_all_ports and get_port_details for each port; the API therefore works, but
> not in the most efficient way. This could be fixed quite easily;
> - however the plugins (FakePlugin, Cisco, and even Openvswitch), return a
> sequence
> whose elements have the following attributes: 'net-id', 'net-name', 'net-
> ports' (instead of 'net-ifaces').
>

Good find Salvatore, this is an interesting issue where the plugin interface, API interface and the plugin implementation all diverge.

This is what I suggest, we should keep the API working efficiently, having net-ifaces, doesn't add overhead and makes the API efficient. With that in mind, this seems to be a good thing to keep in quantum_plugin_base interface.

For the other issues, I believe we should file bugs, and once the plugins have fixed themselves, the API can then take advantage of the more performant mechanism.

Iterating over all ports, while ok in short term, will be an issue in large scale environments.

> I think we probably want to sort this out. We can either change plugin
> interface documentation, or the plugins. Whatever we decide, we should also
> update the API to ensure it works in an efficient way.
>
> What's the best approach to tackle this problem in your opinion?
>
> Cheers,
> Salvatore

Everything else looks good!

But, I noticed, the somehow some pep8 violations have creeped into the trunk, I'll be filing those bugs soon.

review: Approve (netstack-core)

[Salvatore Orlando (salvatore-orlando)]

> > Hi Somik,
> >
> > thanks for your reply.
> > It seems we have now an agreement and can finally close down API v1 for
> > Quantum.
> > I've updated the branch for taking into account the issue with the
> > 'attachment-id' identifier you spotted.
> >
> > However, prompted by your review, I double-checked consistency with
> > specification in quantum-plugin-base, and I spotted a problem with
> > get_network_details.
> > Quantum_plugin_base states that id should return a sequence whose elements
> > have the following attributes: 'net-id', 'net-name', 'net-ifaces', with the
> > latter being a list of interface identifiers.
> >
> > However:
> > - the API does not look for 'net-ifaces' at all. It instead invoke
> > get_all_ports and get_port_details for each port; the API therefore works,
> but
> > not in the most efficient way. This could be fixed quite easily;
> > - however the plugins (FakePlugin, Cisco, and even Openvswitch), return a
> > sequence
> > whose elements have the following attributes: 'net-id', 'net-name', 'net-
> > ports' (instead of 'net-ifaces').
> >
>
> Good find Salvatore, this is an interesting issue where the plugin interface,
> API interface and the plugin implementation all diverge.
>
> This is what I suggest, we should keep the API working efficiently, having
> net-ifaces, doesn't add overhead and makes the API efficient. With that in
> mind, this seems to be a good thing to keep in quantum_plugin_base interface.
>

Definetely agree.

> For the other issues, I believe we should file bugs, and once the plugins have
> fixed themselves, the API can then take advantage of the more performant
> mechanism.
>
> Iterating over all ports, while ok in short term, will be an issue in large
> scale environments.

It is not efficient at all, as it will force clients to do a lot of largely unnecessary round trips.
I will file one or more bugs to improve API and client library accordingly (for instance the client library does not currently leverage the 'detail' actions)

>
> > I think we probably want to sort this out. We can either change plugin
> > interface documentation, or the plugins. Whatever we decide, we should also
> > update the API to ensure it works in an efficient way.
> >
> > What's the best approach to tackle this problem in your opinion?
> >
> > Cheers,
> > Salvatore
>
> Everything else looks good!
>

Thanks!

> But, I noticed, the somehow some pep8 violations have creeped into the trunk,
> I'll be filing those bugs soon.

Good thing you spotted this. I do pep8 tests executing run_tests.sh, which does not check .py files in root directory!

[dan wendlandt (danwent)]

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).

quantum/api/faults.py

Is this still left in as a TODO?

+ #MUST RETURN 202???

quantum/db/api.py,

definitely feel free to remove the check_duplicate_net_name method, as your comment suggests.

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.

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

review: Approve

[Salvatore Orlando (salvatore-orlando)]

> 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/...

[dan wendlandt (danwent)]

On Thu, Aug 25, 2011 at 2:38 AM, Salvatore Orlando <
<email address hidden>> wrote:

> > 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.

Hmm... the python experts I know (I am certainly not one myself) all seem to
recommend against using this approach to check for None, exactly due to the
ambiguity we're talking about. PEP-8 states:

'Also, beware of writing "if x" when you really mean "if x is not None" --
e.g. when testing whether a variable or argument that defaults to None was
set to some other value. The other value might have a type (such as a
container) that could be false in a boolean context!'

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.

Is that true? I was under the impression that a value could also be a
nested array or dictionary, in which case an empty array or dictionary would
be interpreted as an invalid value, even if it was valid. If that is the
case, this could impact anyone writing an extension with nested
arrays/dictionary.

> 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 cl...

=== modified file 'etc/quantum.conf'
--- etc/quantum.conf	2011-08-16 23:03:32 +0000
+++ etc/quantum.conf	2011-08-24 11:55:39 +0000
@@ -17,7 +17,7 @@
 [composite:quantum]
 use = egg:Paste#urlmap
 /: quantumversions
-/v0.1: quantumapi
+/v1.0: quantumapi
 
 [pipeline:quantumapi]
 pipeline = extensions quantumapiapp
 
=== modified file 'quantum/api/__init__.py'
--- quantum/api/__init__.py	2011-08-01 06:22:39 +0000
+++ quantum/api/__init__.py	2011-08-24 11:55:39 +0000
@@ -26,6 +26,7 @@
 
 from quantum import manager
 from quantum.api import faults
+from quantum.api import attachments
 from quantum.api import networks
 from quantum.api import ports
 from quantum.common import flags
@@ -58,24 +59,29 @@
                         path_prefix=uri_prefix)
         mapper.resource('port', 'ports',
                         controller=ports.Controller(plugin),
+                        collection={'detail': 'GET'},
+                        member={'detail': 'GET'},
                         parent_resource=dict(member_name='network',
                                              collection_name=uri_prefix +\
                                                  'networks'))
+
+        attachments_ctrl = attachments.Controller(plugin)
+
         mapper.connect("get_resource",
                        uri_prefix + 'networks/{network_id}/' \
                                     'ports/{id}/attachment{.format}',
-                       controller=ports.Controller(plugin),
+                       controller=attachments_ctrl,
                        action="get_resource",
                        conditions=dict(method=['GET']))
         mapper.connect("attach_resource",
                        uri_prefix + 'networks/{network_id}/' \
                                     'ports/{id}/attachment{.format}',
-                       controller=ports.Controller(plugin),
+                       controller=attachments_ctrl,
                        action="attach_resource",
                        conditions=dict(method=['PUT']))
         mapper.connect("detach_resource",
                        uri_prefix + 'networks/{network_id}/' \
                                     'ports/{id}/attachment{.format}',
-                       controller=ports.Controller(plugin),
+                       controller=attachments_ctrl,
                        action="detach_resource",
                        conditions=dict(method=['DELETE']))
 
=== modified file 'quantum/api/api_common.py'
--- quantum/api/api_common.py	2011-08-01 14:40:29 +0000
+++ quantum/api/api_common.py	2011-08-24 11:55:39 +0000
@@ -38,7 +38,7 @@
         for param in params:
             param_name = param['param-name']
             param_value = None
-            # 1- parse request body
+            # Parameters are expected to be in request body only
             if req.body:
                 des_body = self._deserialize(req.body,
                                              req.best_match_content_type())
@@ -50,22 +50,13 @@
                         LOG.error(line)
                     raise exc.HTTPBadRequest(msg)
                 param_value = data.get(param_name, None)
-            if not param_value:
-                # 2- parse request headers
-                # prepend param name with a 'x-' prefix
-                param_value = req.headers.get("x-" + param_name, None)
-                # 3- parse request query parameters
-                if not param_value:
-                    try:
-                        param_value = req.str_GET[param_name]
-                    except KeyError:
-                        #param not found
-                        pass
-                if not param_value and param['required']:
-                    msg = ("Failed to parse request. " +
-                           "Parameter: " + param_name + " not specified")
-                    for line in msg.split('\n'):
-                        LOG.error(line)
-                    raise exc.HTTPBadRequest(msg)
+
+            # If the parameter wasn't found and it was required, return 400
+            if not param_value and param['required']:
+                msg = ("Failed to parse request. " +
+                       "Parameter: " + param_name + " not specified")
+                for line in msg.split('\n'):
+                    LOG.error(line)
+                raise exc.HTTPBadRequest(msg)
             results[param_name] = param_value or param.get('default-value')
         return results
 
=== added file 'quantum/api/attachments.py'
--- quantum/api/attachments.py	1970-01-01 00:00:00 +0000
+++ quantum/api/attachments.py	2011-08-24 11:55:39 +0000
@@ -0,0 +1,86 @@
+# Copyright 2011 Citrix Systems.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import logging
+
+from webob import exc
+
+from quantum.api import api_common as common
+from quantum.api import faults
+from quantum.api.views import attachments as attachments_view
+from quantum.common import exceptions as exception
+
+LOG = logging.getLogger('quantum.api.ports')
+
+
+class Controller(common.QuantumController):
+    """ Port API controller for Quantum API """
+
+    _attachment_ops_param_list = [{
+        'param-name': 'id',
+        'required': True}, ]
+
+    _serialization_metadata = {
+        "application/xml": {
+            "attributes": {
+                "attachment": ["id"], }
+        },
+    }
+
+    def __init__(self, plugin):
+        self._resource_name = 'attachment'
+        super(Controller, self).__init__(plugin)
+
+    def get_resource(self, request, tenant_id, network_id, id):
+        try:
+            att_data = self._plugin.get_port_details(
+                            tenant_id, network_id, id)
+            builder = attachments_view.get_view_builder(request)
+            result = builder.build(att_data)['attachment']
+            return dict(attachment=result)
+        except exception.NetworkNotFound as e:
+            return faults.Fault(faults.NetworkNotFound(e))
+        except exception.PortNotFound as e:
+            return faults.Fault(faults.PortNotFound(e))
+
+    def attach_resource(self, request, tenant_id, network_id, id):
+        try:
+            request_params = \
+                self._parse_request_params(request,
+                                           self._attachment_ops_param_list)
+        except exc.HTTPError as e:
+            return faults.Fault(e)
+        try:
+            self._plugin.plug_interface(tenant_id, network_id, id,
+                                        request_params['id'])
+            return exc.HTTPNoContent()
+        except exception.NetworkNotFound as e:
+            return faults.Fault(faults.NetworkNotFound(e))
+        except exception.PortNotFound as e:
+            return faults.Fault(faults.PortNotFound(e))
+        except exception.PortInUse as e:
+            return faults.Fault(faults.PortInUse(e))
+        except exception.AlreadyAttached as e:
+            return faults.Fault(faults.AlreadyAttached(e))
+
+    def detach_resource(self, request, tenant_id, network_id, id):
+        try:
+            self._plugin.unplug_interface(tenant_id,
+                                          network_id, id)
+            return exc.HTTPNoContent()
+        except exception.NetworkNotFound as e:
+            return faults.Fault(faults.NetworkNotFound(e))
+        except exception.PortNotFound as e:
+            return faults.Fault(faults.PortNotFound(e))
 
=== modified file 'quantum/api/faults.py'
--- quantum/api/faults.py	2011-08-06 06:12:32 +0000
+++ quantum/api/faults.py	2011-08-24 11:55:39 +0000
@@ -31,7 +31,6 @@
             401: "unauthorized",
             420: "networkNotFound",
             421: "networkInUse",
-            422: "networkNameExists",
             430: "portNotFound",
             431: "requestedStateInvalid",
             432: "portInUse",
@@ -92,22 +91,6 @@
     explanation = ('Unable to remove the network: attachments still plugged.')
 
 
-class NetworkNameExists(webob.exc.HTTPClientError):
-    """
-    subclass of :class:`~HTTPClientError`
-
-    This indicates that the server could not set the network name to the
-    specified value because another network for the same tenant already has
-    that name.
-
-    code: 422, title: Network Name Exists
-    """
-    code = 422
-    title = 'Network Name Exists'
-    explanation = ('Unable to set network name: tenant already has network' \
-                        ' with same name.')
-
-
 class PortNotFound(webob.exc.HTTPClientError):
     """
     subclass of :class:`~HTTPClientError`
 
=== modified file 'quantum/api/networks.py'
--- quantum/api/networks.py	2011-08-01 17:11:16 +0000
+++ quantum/api/networks.py	2011-08-24 11:55:39 +0000
@@ -29,7 +29,7 @@
     """ Network API controller for Quantum API """
 
     _network_ops_param_list = [{
-        'param-name': 'net-name',
+        'param-name': 'name',
         'required': True}, ]
 
     _serialization_metadata = {
@@ -37,38 +37,43 @@
             "attributes": {
                 "network": ["id", "name"],
                 "port": ["id", "state"],
-            },
-            "plurals": {"networks": "network"}
-        },
+                "attachment": ["id"]},
+            "plurals": {"networks": "network",
+                        "ports": "port"}},
     }
 
     def __init__(self, plugin):
         self._resource_name = 'network'
         super(Controller, self).__init__(plugin)
 
-    def index(self, request, tenant_id):
-        """ Returns a list of network ids """
-        #TODO: this should be for a given tenant!!!
-        return self._items(request, tenant_id)
-
     def _item(self, req, tenant_id, network_id,
               net_details=True, port_details=False):
         # We expect get_network_details to return information
         # concerning logical ports as well.
         network = self._plugin.get_network_details(
                             tenant_id, network_id)
+        port_list = self._plugin.get_all_ports(
+                            tenant_id, network_id)
+        ports_data = [self._plugin.get_port_details(
+                                   tenant_id, network_id, port['port-id'])
+                      for port in port_list]
         builder = networks_view.get_view_builder(req)
-        result = builder.build(network, net_details, port_details)['network']
+        result = builder.build(network, net_details,
+                               ports_data, port_details)['network']
         return dict(network=result)
 
-    def _items(self, req, tenant_id, net_details=False, port_details=False):
+    def _items(self, req, tenant_id, net_details=False):
         """ Returns a list of networks. """
         networks = self._plugin.get_all_networks(tenant_id)
         builder = networks_view.get_view_builder(req)
-        result = [builder.build(network, net_details, port_details)['network']
+        result = [builder.build(network, net_details)['network']
                   for network in networks]
         return dict(networks=result)
 
+    def index(self, request, tenant_id):
+        """ Returns a list of network ids """
+        return self._items(request, tenant_id)
+
     def show(self, request, tenant_id, id):
         """ Returns network details for the given network id """
         try:
@@ -80,23 +85,13 @@
     def detail(self, request, **kwargs):
         tenant_id = kwargs.get('tenant_id')
         network_id = kwargs.get('id')
-        try:
-            if network_id:
-                # show details for a given network
-                return self._item(request, tenant_id, network_id,
-                                  net_details=True, port_details=True)
-            else:
-                # show details for all networks
-                return self._items(request, tenant_id,
-                                   net_details=True, port_details=False)
-            network = self._plugin.get_network_details(
-                            tenant_id, id)
-            builder = networks_view.get_view_builder(request)
-            #build response with details
-            result = builder.build(network, True)
-            return dict(networks=result)
-        except exception.NetworkNotFound as e:
-            return faults.Fault(faults.NetworkNotFound(e))
+        if network_id:
+            # show details for a given network
+            return self._item(request, tenant_id, network_id,
+                              net_details=True, port_details=True)
+        else:
+            # show details for all networks
+            return self._items(request, tenant_id, net_details=True)
 
     def create(self, request, tenant_id):
         """ Creates a new network for a given tenant """
@@ -107,15 +102,13 @@
                                            self._network_ops_param_list)
         except exc.HTTPError as e:
             return faults.Fault(e)
-        try:
-            network = self._plugin.\
-                       create_network(tenant_id,
-                                      request_params['net-name'])
-            builder = networks_view.get_view_builder(request)
-            result = builder.build(network)
-            return dict(networks=result)
-        except exception.NetworkNameExists as e:
-            return faults.Fault(faults.NetworkNameExists(e))
+        network = self._plugin.\
+                   create_network(tenant_id,
+                                  request_params['name'])
+        builder = networks_view.get_view_builder(request)
+        result = builder.build(network)['network']
+        #MUST RETURN 202???
+        return dict(network=result)
 
     def update(self, request, tenant_id, id):
         """ Updates the name for the network with the given id """
@@ -127,18 +120,16 @@
             return faults.Fault(e)
         try:
             self._plugin.rename_network(tenant_id, id,
-                                        request_params['net-name'])
-            return exc.HTTPAccepted()
+                                        request_params['name'])
+            return exc.HTTPNoContent()
         except exception.NetworkNotFound as e:
             return faults.Fault(faults.NetworkNotFound(e))
-        except exception.NetworkNameExists as e:
-            return faults.Fault(faults.NetworkNameExists(e))
 
     def delete(self, request, tenant_id, id):
         """ Destroys the network with the given id """
         try:
             self._plugin.delete_network(tenant_id, id)
-            return exc.HTTPAccepted()
+            return exc.HTTPNoContent()
         except exception.NetworkNotFound as e:
             return faults.Fault(faults.NetworkNotFound(e))
         except exception.NetworkInUse as e:
 
=== modified file 'quantum/api/ports.py'
--- quantum/api/ports.py	2011-07-22 09:51:22 +0000
+++ quantum/api/ports.py	2011-08-24 11:55:39 +0000
@@ -29,55 +29,79 @@
     """ Port API controller for Quantum API """
 
     _port_ops_param_list = [{
-        'param-name': 'port-state',
+        'param-name': 'state',
         'default-value': 'DOWN',
         'required': False}, ]
 
-    _attachment_ops_param_list = [{
-        'param-name': 'attachment-id',
-        'required': True}, ]
-
     _serialization_metadata = {
         "application/xml": {
             "attributes": {
-                "port": ["id", "state"], },
-            "plurals": {"ports": "port"}
-        },
+                "port": ["id", "state"],
+                "attachment": ["id"]},
+            "plurals": {"ports": "port"}},
     }
 
     def __init__(self, plugin):
         self._resource_name = 'port'
         super(Controller, self).__init__(plugin)
 
+    def _items(self, request, tenant_id, network_id,
+               port_details=False):
+        """ Returns a list of ports. """
+        try:
+            port_list = self._plugin.get_all_ports(tenant_id, network_id)
+            builder = ports_view.get_view_builder(request)
+
+            # Load extra data for ports if required.
+            if port_details:
+                port_list_detail = \
+                    [self._plugin.get_port_details(
+                                tenant_id, network_id, port['port-id'])
+                      for port in port_list]
+                port_list = port_list_detail
+
+            result = [builder.build(port, port_details)['port']
+                      for port in port_list]
+            return dict(ports=result)
+        except exception.NetworkNotFound as e:
+            return faults.Fault(faults.NetworkNotFound(e))
+
+    def _item(self, request, tenant_id, network_id, port_id,
+              att_details=False):
+        """ Returns a specific port. """
+        port = self._plugin.get_port_details(
+                        tenant_id, network_id, port_id)
+        builder = ports_view.get_view_builder(request)
+        result = builder.build(port, port_details=True,
+                               att_details=att_details)['port']
+        return dict(port=result)
+
     def index(self, request, tenant_id, network_id):
         """ Returns a list of port ids for a given network """
-        return self._items(request, tenant_id, network_id, is_detail=False)
-
-    def _items(self, request, tenant_id, network_id, is_detail):
-        """ Returns a list of networks. """
-        try:
-            ports = self._plugin.get_all_ports(tenant_id, network_id)
-            builder = ports_view.get_view_builder(request)
-            result = [builder.build(port, is_detail)['port']
-                      for port in ports]
-            return dict(ports=result)
-        except exception.NetworkNotFound as e:
-            return faults.Fault(faults.NetworkNotFound(e))
+        return self._items(request, tenant_id, network_id, port_details=False)
 
     def show(self, request, tenant_id, network_id, id):
         """ Returns port details for given port and network """
         try:
-            port = self._plugin.get_port_details(
-                            tenant_id, network_id, id)
-            builder = ports_view.get_view_builder(request)
-            #build response with details
-            result = builder.build(port, True)['port']
-            return dict(port=result)
+            return self._item(request, tenant_id, network_id, id)
         except exception.NetworkNotFound as e:
             return faults.Fault(faults.NetworkNotFound(e))
         except exception.PortNotFound as e:
             return faults.Fault(faults.PortNotFound(e))
 
+    def detail(self, request, **kwargs):
+        tenant_id = kwargs.get('tenant_id')
+        network_id = kwargs.get('network_id')
+        port_id = kwargs.get('id')
+        if port_id:
+            # show details for a given network
+            return self._item(request, tenant_id,
+                              network_id, port_id, att_details=True)
+        else:
+            # show details for all port
+            return self._items(request, tenant_id,
+                               network_id, port_details=True)
+
     def create(self, request, tenant_id, network_id):
         """ Creates a new port for a given network """
         #look for port state in request
@@ -89,10 +113,10 @@
         try:
             port = self._plugin.create_port(tenant_id,
                                             network_id,
-                                            request_params['port-state'])
+                                            request_params['state'])
             builder = ports_view.get_view_builder(request)
-            result = builder.build(port)
-            return dict(ports=result)
+            result = builder.build(port)['port']
+            return dict(port=result)
         except exception.NetworkNotFound as e:
             return faults.Fault(faults.NetworkNotFound(e))
         except exception.StateInvalid as e:
@@ -107,11 +131,9 @@
         except exc.HTTPError as e:
             return faults.Fault(e)
         try:
-            port = self._plugin.update_port(tenant_id, network_id, id,
-                                            request_params['port-state'])
-            builder = ports_view.get_view_builder(request)
-            result = builder.build(port, True)
-            return dict(ports=result)
+            self._plugin.update_port(tenant_id, network_id, id,
+                                     request_params['state'])
+            return exc.HTTPNoContent()
         except exception.NetworkNotFound as e:
             return faults.Fault(faults.NetworkNotFound(e))
         except exception.PortNotFound as e:
@@ -124,53 +146,10 @@
         #look for port state in request
         try:
             self._plugin.delete_port(tenant_id, network_id, id)
-            return exc.HTTPAccepted()
-            # TODO(salvatore-orlando): Handle portInUse error
-        except exception.NetworkNotFound as e:
-            return faults.Fault(faults.NetworkNotFound(e))
-        except exception.PortNotFound as e:
-            return faults.Fault(faults.PortNotFound(e))
-        except exception.PortInUse as e:
-            return faults.Fault(faults.PortInUse(e))
-
-    def get_resource(self, request, tenant_id, network_id, id):
-        try:
-            result = self._plugin.get_port_details(
-                            tenant_id, network_id, id).get('attachment-id',
-                                                           None)
-            return dict(attachment=result)
-        except exception.NetworkNotFound as e:
-            return faults.Fault(faults.NetworkNotFound(e))
-        except exception.PortNotFound as e:
-            return faults.Fault(faults.PortNotFound(e))
-
-    def attach_resource(self, request, tenant_id, network_id, id):
-        try:
-            request_params = \
-                self._parse_request_params(request,
-                                           self._attachment_ops_param_list)
-        except exc.HTTPError as e:
-            return faults.Fault(e)
-        try:
-            self._plugin.plug_interface(tenant_id,
-                                            network_id, id,
-                                            request_params['attachment-id'])
-            return exc.HTTPAccepted()
-        except exception.NetworkNotFound as e:
-            return faults.Fault(faults.NetworkNotFound(e))
-        except exception.PortNotFound as e:
-            return faults.Fault(faults.PortNotFound(e))
-        except exception.PortInUse as e:
-            return faults.Fault(faults.PortInUse(e))
-        except exception.AlreadyAttached as e:
-            return faults.Fault(faults.AlreadyAttached(e))
-
-    def detach_resource(self, request, tenant_id, network_id, id):
-        try:
-            self._plugin.unplug_interface(tenant_id,
-                                          network_id, id)
-            return exc.HTTPAccepted()
-        except exception.NetworkNotFound as e:
-            return faults.Fault(faults.NetworkNotFound(e))
-        except exception.PortNotFound as e:
-            return faults.Fault(faults.PortNotFound(e))
+            return exc.HTTPNoContent()
+        except exception.NetworkNotFound as e:
+            return faults.Fault(faults.NetworkNotFound(e))
+        except exception.PortNotFound as e:
+            return faults.Fault(faults.PortNotFound(e))
+        except exception.PortInUse as e:
+            return faults.Fault(faults.PortInUse(e))
 
=== modified file 'quantum/api/versions.py'
--- quantum/api/versions.py	2011-05-31 17:15:00 +0000
+++ quantum/api/versions.py	2011-08-24 11:55:39 +0000
@@ -31,11 +31,11 @@
         """Respond to a request for all Quantum API versions."""
         version_objs = [
             {
-                "id": "v0.1",
+                "id": "v1.0",
                 "status": "CURRENT",
             },
             {
-                "id": "v1.0",
+                "id": "v1.1",
                 "status": "FUTURE",
             },
         ]
 
=== added file 'quantum/api/views/attachments.py'
--- quantum/api/views/attachments.py	1970-01-01 00:00:00 +0000
+++ quantum/api/views/attachments.py	2011-08-24 11:55:39 +0000
@@ -0,0 +1,37 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2011 Citrix Systems
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+
+def get_view_builder(req):
+    base_url = req.application_url
+    return ViewBuilder(base_url)
+
+
+class ViewBuilder(object):
+
+    def __init__(self, base_url):
+        """
+        :param base_url: url of the root wsgi application
+        """
+        self.base_url = base_url
+
+    def build(self, attachment_data):
+        """Generic method used to generate an attachment entity."""
+        if attachment_data['attachment']:
+            return dict(attachment=dict(id=attachment_data['attachment']))
+        else:
+            return dict(attachment={})
 
=== modified file 'quantum/api/views/networks.py'
--- quantum/api/views/networks.py	2011-08-01 01:21:59 +0000
+++ quantum/api/views/networks.py	2011-08-24 11:55:39 +0000
@@ -15,8 +15,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-from quantum.api.views import ports as ports_view
-
 
 def get_view_builder(req):
     base_url = req.application_url
@@ -31,17 +29,16 @@
         """
         self.base_url = base_url
 
-    def build(self, network_data, net_detail=False, port_detail=False):
+    def build(self, network_data, net_detail=False,
+              ports_data=None, port_detail=False):
         """Generic method used to generate a network entity."""
         if net_detail:
             network = self._build_detail(network_data)
         else:
             network = self._build_simple(network_data)
         if port_detail:
-            builder = ports_view.ViewBuilder(self.base_url)
-            ports = [builder.build(port_data, port_detail)['port']
-                     for port_data in network_data['net-ports'].values()]
-            network['ports'] = ports
+            ports = [self._build_port(port_data) for port_data in ports_data]
+            network['network']['ports'] = ports
         return network
 
     def _build_simple(self, network_data):
@@ -55,6 +52,8 @@
 
     def _build_port(self, port_data):
         """Return details about a specific logical port."""
-        return dict(port=dict(id=port_data['port-id'],
-                              state=port_data['port-state'],
-                              attachment=port_data['attachment']))
+        port_dict = dict(id=port_data['port-id'],
+                         state=port_data['port-state'])
+        if port_data['attachment']:
+            port_dict['attachment'] = dict(id=port_data['attachment'])
+        return port_dict
 
=== modified file 'quantum/api/views/ports.py'
--- quantum/api/views/ports.py	2011-07-22 09:51:22 +0000
+++ quantum/api/views/ports.py	2011-08-24 11:55:39 +0000
@@ -29,19 +29,11 @@
         """
         self.base_url = base_url
 
-    def build(self, port_data, is_detail=False):
+    def build(self, port_data, port_details=False, att_details=False):
         """Generic method used to generate a port entity."""
-        if is_detail:
-            port = self._build_detail(port_data)
-        else:
-            port = self._build_simple(port_data)
+        port = dict(port=dict(id=port_data['port-id']))
+        if port_details:
+            port['port']['state'] = port_data['port-state']
+        if att_details and port_data['attachment']:
+            port['port']['attachment'] = dict(id=port_data['attachment'])
         return port
-
-    def _build_simple(self, port_data):
-        """Return a simple model of a port."""
-        return dict(port=dict(id=port_data['port-id']))
-
-    def _build_detail(self, port_data):
-        """Return a simple model of a port (with its state)."""
-        return dict(port=dict(id=port_data['port-id'],
-                              state=port_data['port-state']))
 
=== modified file 'quantum/cli.py'
--- quantum/cli.py	2011-08-23 19:36:06 +0000
+++ quantum/cli.py	2011-08-24 11:55:39 +0000
@@ -69,7 +69,7 @@
     LOG.debug(res)
     nid = None
     try:
-        nid = res["networks"]["network"]["id"]
+        nid = res["network"]["id"]
     except Exception, e:
         print "Failed to create network"
         # TODO(bgh): grab error details from ws request result
@@ -104,7 +104,7 @@
 def api_detail_net(client, *args):
     tid, nid = args
     try:
-        res = client.show_network_details(nid)["networks"]["network"]
+        res = client.show_network_details(nid)["network"]
     except Exception, e:
         LOG.error("Failed to get network details: %s" % e)
         return
@@ -121,7 +121,7 @@
         pid = port["id"]
         res = client.show_port_attachment(nid, pid)
         LOG.debug(res)
-        remote_iface = res["attachment"]
+        remote_iface = res["attachment"]["id"]
         print "\tRemote interface:%s" % remote_iface
 
 
@@ -133,7 +133,7 @@
 
 def api_rename_net(client, *args):
     tid, nid, name = args
-    data = {'network': {'net-name': '%s' % name}}
+    data = {'network': {'name': '%s' % name}}
     try:
         res = client.update_network(nid, data)
     except Exception, e:
@@ -179,7 +179,7 @@
     except Exception, e:
         LOG.error("Failed to create port: %s" % e)
         return
-    new_port = res["ports"]["port"]["id"]
+    new_port = res["port"]["id"]
     print "Created Virtual Port:%s " \
           "on Virtual Network:%s" % (new_port, nid)
 
@@ -214,16 +214,17 @@
 def api_detail_port(client, *args):
     tid, nid, pid = args
     try:
-        port = client.show_port_details(nid, pid)["ports"]["port"]
+        port = client.show_port_details(nid, pid)["port"]
+        att = client.show_port_attachment(nid, pid)
     except Exception, e:
         LOG.error("Failed to get port details: %s" % e)
         return
 
-    id = port["id"]
-    attachment = port["attachment"]
+    id = port['id']
+    interface_id = att['id']
     LOG.debug(port)
     print "Virtual Port:%s on Virtual Network:%s " \
-          "contains remote interface:%s" % (pid, nid, attachment)
+          "contains remote interface:%s" % (pid, nid, interface_id)
 
 
 def plug_iface(manager, *args):
@@ -236,7 +237,7 @@
 def api_plug_iface(client, *args):
     tid, nid, pid, vid = args
     try:
-        data = {'port': {'attachment-id': '%s' % vid}}
+        data = {'attachment': {'id': '%s' % vid}}
         res = client.attach_resource(nid, pid, data)
     except Exception, e:
         LOG.error("Failed to plug iface \"%s\" to port \"%s\": %s" % (vid,
 
=== modified file 'quantum/client.py'
--- quantum/client.py	2011-08-24 10:15:54 +0000
+++ quantum/client.py	2011-08-24 11:55:39 +0000
@@ -27,13 +27,10 @@
     401: exceptions.NotAuthorized,
     420: exceptions.NetworkNotFound,
     421: exceptions.NetworkInUse,
-    422: exceptions.NetworkNameExists,
     430: exceptions.PortNotFound,
     431: exceptions.StateInvalid,
     432: exceptions.PortInUse,
-    440: exceptions.AlreadyAttached,
-    441: exceptions.AttachmentNotReady,
-}
+    440: exceptions.AlreadyAttached}
 
 
 class ApiCall(object):
@@ -72,7 +69,7 @@
 
     def __init__(self, host="127.0.0.1", port=9696, use_ssl=False, tenant=None,
                 format="xml", testingStub=None, key_file=None, cert_file=None,
-                logger=None, action_prefix="/v0.1/tenants/{tenant_id}"):
+                logger=None, action_prefix="/v1.0/tenants/{tenant_id}"):
         """
         Creates a new client to some service.
 
 
=== modified file 'quantum/common/exceptions.py'
--- quantum/common/exceptions.py	2011-08-18 19:49:20 +0000
+++ quantum/common/exceptions.py	2011-08-24 11:55:39 +0000
@@ -111,16 +111,6 @@
                 "already plugged into port %(att_port_id)s")
 
 
-class AttachmentNotReady(QuantumException):
-    message = _("The attachment %(att_id)s is not ready")
-
-
-class NetworkNameExists(QuantumException):
-    message = _("Unable to set network name to %(net_name). " \
-                "Network with id %(net_id) already has this name for " \
-                "tenant %(tenant_id)")
-
-
 class Duplicate(Error):
     pass
 
 
=== modified file 'quantum/common/wsgi.py'
--- quantum/common/wsgi.py	2011-08-02 04:49:32 +0000
+++ quantum/common/wsgi.py	2011-08-24 11:55:39 +0000
@@ -122,6 +122,7 @@
         Based on the query extension then the Accept header.
 
         """
+        # First lookup http request
         parts = self.path.rsplit('.', 1)
         LOG.debug("Request parts:%s", parts)
         if len(parts) > 1:
@@ -129,21 +130,26 @@
             if format in ['json', 'xml']:
                 return 'application/{0}'.format(parts[1])
 
+        #Then look up content header
+        type_from_header = self.get_content_type()
+        if type_from_header:
+            return type_from_header
         ctypes = ['application/json', 'application/xml']
+
+        #Finally search in Accept-* headers
         bm = self.accept.best_match(ctypes)
         return bm or 'application/json'
 
     def get_content_type(self):
         allowed_types = ("application/xml", "application/json")
         if not "Content-Type" in self.headers:
-            msg = _("Missing Content-Type")
-            LOG.debug(msg)
-            raise webob.exc.HTTPBadRequest(msg)
+            LOG.debug(_("Missing Content-Type"))
+            return None
         type = self.content_type
         if type in allowed_types:
             return type
         LOG.debug(_("Wrong Content-Type: %s") % type)
-        raise webob.exc.HTTPBadRequest("Invalid content type")
+        return None
 
 
 class Application(object):
 
=== modified file 'quantum/db/api.py'
--- quantum/db/api.py	2011-08-10 04:58:15 +0000
+++ quantum/db/api.py	2011-08-24 11:55:39 +0000
@@ -17,6 +17,8 @@
 # @author: Brad Hall, Nicira Networks, Inc.
 # @author: Dan Wendlandt, Nicira Networks, Inc.
 
+import logging
+
 from sqlalchemy import create_engine
 from sqlalchemy.orm import sessionmaker, exc
 
@@ -27,6 +29,7 @@
 _ENGINE = None
 _MAKER = None
 BASE = models.BASE
+LOG = logging.getLogger('quantum.db.api')
 
 
 def configure_db(options):
@@ -78,6 +81,11 @@
 
 
 def _check_duplicate_net_name(tenant_id, net_name):
+    """Checks whether a network with the same name
+       already exists for the tenant.
+    """
+
+    #TODO(salvatore-orlando): Not used anymore - candidate for removal
     session = get_session()
     try:
         net = session.query(models.Network).\
@@ -94,7 +102,6 @@
 def network_create(tenant_id, name):
     session = get_session()
 
-    _check_duplicate_net_name(tenant_id, name)
     with session.begin():
         net = models.Network(tenant_id, name)
         session.add(net)
 
=== modified file 'quantum/db/models.py'
--- quantum/db/models.py	2011-08-02 04:49:32 +0000
+++ quantum/db/models.py	2011-08-24 11:55:39 +0000
@@ -70,13 +70,14 @@
     uuid = Column(String(255), primary_key=True)
     network_id = Column(String(255), ForeignKey("networks.uuid"),
                         nullable=False)
-    interface_id = Column(String(255))
+    interface_id = Column(String(255), nullable=True)
     # Port state - Hardcoding string value at the moment
     state = Column(String(8))
 
     def __init__(self, network_id):
         self.uuid = str(uuid.uuid4())
         self.network_id = network_id
+        self.interface_id = None
         self.state = "DOWN"
 
     def __repr__(self):
 
=== modified file 'quantum/plugins/SamplePlugin.py'
--- quantum/plugins/SamplePlugin.py	2011-08-10 04:58:15 +0000
+++ quantum/plugins/SamplePlugin.py	2011-08-24 11:55:39 +0000
@@ -352,7 +352,7 @@
         LOG.debug("FakePlugin.get_port_details() called")
         port = self._get_port(tenant_id, net_id, port_id)
         return {'port-id': str(port.uuid),
-                'attachment-id': port.interface_id,
+                'attachment': port.interface_id,
                 'port-state': port.state}
 
     def create_port(self, tenant_id, net_id, port_state=None):
@@ -407,10 +407,10 @@
         specified Virtual Network.
         """
         LOG.debug("FakePlugin.plug_interface() called")
+        port = self._get_port(tenant_id, net_id, port_id)
         # Validate attachment
         self._validate_attachment(tenant_id, net_id, port_id,
                                   remote_interface_id)
-        port = self._get_port(tenant_id, net_id, port_id)
         if port['interface_id']:
             raise exc.PortInUse(net_id=net_id, port_id=port_id,
                                 att_id=port['interface_id'])
 
=== modified file 'tests/unit/test_api.py'
--- tests/unit/test_api.py	2011-08-23 19:36:06 +0000
+++ tests/unit/test_api.py	2011-08-24 11:55:39 +0000
@@ -48,7 +48,7 @@
         if expected_res_status == 200:
             network_data = Serializer().deserialize(network_res.body,
                                                     content_type)
-            return network_data['networks']['network']['id']
+            return network_data['network']['id']
 
     def _create_port(self, network_id, port_state, format,
                      custom_req_body=None, expected_res_status=200):
@@ -61,7 +61,7 @@
         self.assertEqual(port_res.status_int, expected_res_status)
         if expected_res_status == 200:
             port_data = Serializer().deserialize(port_res.body, content_type)
-            return port_data['ports']['port']['id']
+            return port_data['port']['id']
 
     def _test_create_network(self, format):
         LOG.debug("_test_create_network - format:%s - START", format)
@@ -74,8 +74,7 @@
         self.assertEqual(show_network_res.status_int, 200)
         network_data = Serializer().deserialize(show_network_res.body,
                                                 content_type)
-        self.assertEqual(network_id,
-                         network_data['network']['id'])
+        self.assertEqual(network_id, network_data['network']['id'])
         LOG.debug("_test_create_network - format:%s - END", format)
 
     def _test_create_network_badrequest(self, format):
@@ -102,6 +101,25 @@
         self.assertEqual(len(network_data['networks']), 2)
         LOG.debug("_test_list_networks - format:%s - END", format)
 
+    def _test_list_networks_detail(self, format):
+        LOG.debug("_test_list_networks_detail - format:%s - START", format)
+        content_type = "application/%s" % format
+        self._create_network(format, "net_1")
+        self._create_network(format, "net_2")
+        list_network_req = testlib.network_list_detail_request(self.tenant_id,
+                                                               format)
+        list_network_res = list_network_req.get_response(self.api)
+        self.assertEqual(list_network_res.status_int, 200)
+        network_data = self._net_serializer.deserialize(
+                           list_network_res.body, content_type)
+        # Check network count: should return 2
+        self.assertEqual(len(network_data['networks']), 2)
+        # Check contents - id & name for each network
+        for network in network_data['networks']:
+            self.assertTrue('id' in network and 'name' in network)
+            self.assertTrue(network['id'] and network['name'])
+        LOG.debug("_test_list_networks_detail - format:%s - END", format)
+
     def _test_show_network(self, format):
         LOG.debug("_test_show_network - format:%s - START", format)
         content_type = "application/%s" % format
@@ -118,6 +136,25 @@
                          network_data['network'])
         LOG.debug("_test_show_network - format:%s - END", format)
 
+    def _test_show_network_detail(self, format):
+        LOG.debug("_test_show_network_detail - format:%s - START", format)
+        content_type = "application/%s" % format
+        # Create a network and a port
+        network_id = self._create_network(format)
+        port_id = self._create_port(network_id, "ACTIVE", format)
+        show_network_req = testlib.show_network_detail_request(
+                                    self.tenant_id, network_id, format)
+        show_network_res = show_network_req.get_response(self.api)
+        self.assertEqual(show_network_res.status_int, 200)
+        network_data = self._net_serializer.deserialize(
+                           show_network_res.body, content_type)
+        self.assertEqual({'id': network_id,
+                          'name': self.network_name,
+                          'ports': [{'id': port_id,
+                                     'state': 'ACTIVE'}]},
+                         network_data['network'])
+        LOG.debug("_test_show_network_detail - format:%s - END", format)
+
     def _test_show_network_not_found(self, format):
         LOG.debug("_test_show_network_not_found - format:%s - START", format)
         show_network_req = testlib.show_network_request(self.tenant_id,
@@ -137,7 +174,7 @@
                                                             new_name,
                                                             format)
         update_network_res = update_network_req.get_response(self.api)
-        self.assertEqual(update_network_res.status_int, 202)
+        self.assertEqual(update_network_res.status_int, 204)
         show_network_req = testlib.show_network_request(self.tenant_id,
                                                         network_id,
                                                         format)
@@ -187,7 +224,7 @@
                                                             network_id,
                                                             format)
         delete_network_res = delete_network_req.get_response(self.api)
-        self.assertEqual(delete_network_res.status_int, 202)
+        self.assertEqual(delete_network_res.status_int, 204)
         list_network_req = testlib.network_list_request(self.tenant_id,
                                                         format)
         list_network_res = list_network_req.get_response(self.api)
@@ -213,7 +250,7 @@
                                                         port_id,
                                                         attachment_id)
         attachment_res = attachment_req.get_response(self.api)
-        self.assertEquals(attachment_res.status_int, 202)
+        self.assertEquals(attachment_res.status_int, 204)
 
         LOG.debug("Deleting network %(network_id)s"\
                   " of tenant %(tenant_id)s", locals())
@@ -241,6 +278,27 @@
         self.assertEqual(len(port_data['ports']), 2)
         LOG.debug("_test_list_ports - format:%s - END", format)
 
+    def _test_list_ports_detail(self, format):
+        LOG.debug("_test_list_ports_detail - format:%s - START", format)
+        content_type = "application/%s" % format
+        port_state = "ACTIVE"
+        network_id = self._create_network(format)
+        self._create_port(network_id, port_state, format)
+        self._create_port(network_id, port_state, format)
+        list_port_req = testlib.port_list_detail_request(self.tenant_id,
+                                                         network_id, format)
+        list_port_res = list_port_req.get_response(self.api)
+        self.assertEqual(list_port_res.status_int, 200)
+        port_data = self._port_serializer.deserialize(
+                        list_port_res.body, content_type)
+        # Check port count: should return 2
+        self.assertEqual(len(port_data['ports']), 2)
+        # Check contents - id & name for each network
+        for port in port_data['ports']:
+            self.assertTrue('id' in port and 'state' in port)
+            self.assertTrue(port['id'] and port['state'])
+        LOG.debug("_test_list_ports_detail - format:%s - END", format)
+
     def _test_show_port(self, format):
         LOG.debug("_test_show_port - format:%s - START", format)
         content_type = "application/%s" % format
@@ -258,6 +316,44 @@
                          port_data['port'])
         LOG.debug("_test_show_port - format:%s - END", format)
 
+    def _test_show_port_detail(self, format):
+        LOG.debug("_test_show_port - format:%s - START", format)
+        content_type = "application/%s" % format
+        port_state = "ACTIVE"
+        network_id = self._create_network(format)
+        port_id = self._create_port(network_id, port_state, format)
+
+        # Part 1 - no attachment
+        show_port_req = testlib.show_port_detail_request(self.tenant_id,
+                                    network_id, port_id, format)
+        show_port_res = show_port_req.get_response(self.api)
+        self.assertEqual(show_port_res.status_int, 200)
+        port_data = self._port_serializer.deserialize(
+                        show_port_res.body, content_type)
+        self.assertEqual({'id': port_id, 'state': port_state},
+                         port_data['port'])
+
+        # Part 2 - plug attachment into port
+        interface_id = "test_interface"
+        put_attachment_req = testlib.put_attachment_request(self.tenant_id,
+                                                            network_id,
+                                                            port_id,
+                                                            interface_id,
+                                                            format)
+        put_attachment_res = put_attachment_req.get_response(self.api)
+        self.assertEqual(put_attachment_res.status_int, 204)
+        show_port_req = testlib.show_port_detail_request(self.tenant_id,
+                                    network_id, port_id, format)
+        show_port_res = show_port_req.get_response(self.api)
+        self.assertEqual(show_port_res.status_int, 200)
+        port_data = self._port_serializer.deserialize(
+                        show_port_res.body, content_type)
+        self.assertEqual({'id': port_id, 'state': port_state,
+                          'attachment': {'id': interface_id}},
+                         port_data['port'])
+
+        LOG.debug("_test_show_port_detail - format:%s - END", format)
+
     def _test_show_port_networknotfound(self, format):
         LOG.debug("_test_show_port_networknotfound - format:%s - START",
                   format)
@@ -343,7 +439,7 @@
                                                       network_id, port_id,
                                                       format)
         delete_port_res = delete_port_req.get_response(self.api)
-        self.assertEqual(delete_port_res.status_int, 202)
+        self.assertEqual(delete_port_res.status_int, 204)
         list_port_req = testlib.port_list_request(self.tenant_id, network_id,
                                                   format)
         list_port_res = list_port_req.get_response(self.api)
@@ -367,7 +463,7 @@
                                                         port_id,
                                                         attachment_id)
         attachment_res = attachment_req.get_response(self.api)
-        self.assertEquals(attachment_res.status_int, 202)
+        self.assertEquals(attachment_res.status_int, 204)
         LOG.debug("Deleting port %(port_id)s for network %(network_id)s"\
                   " of tenant %(tenant_id)s", locals())
         delete_port_req = testlib.port_delete_request(self.tenant_id,
@@ -417,7 +513,7 @@
                                                         new_port_state,
                                                         format)
         update_port_res = update_port_req.get_response(self.api)
-        self.assertEqual(update_port_res.status_int, 200)
+        self.assertEqual(update_port_res.status_int, 204)
         show_port_req = testlib.show_port_request(self.tenant_id,
                                                   network_id, port_id,
                                                   format)
@@ -427,6 +523,22 @@
                         show_port_res.body, content_type)
         self.assertEqual({'id': port_id, 'state': new_port_state},
                          port_data['port'])
+        # now set it back to the original value
+        update_port_req = testlib.update_port_request(self.tenant_id,
+                                                        network_id, port_id,
+                                                        port_state,
+                                                        format)
+        update_port_res = update_port_req.get_response(self.api)
+        self.assertEqual(update_port_res.status_int, 204)
+        show_port_req = testlib.show_port_request(self.tenant_id,
+                                                  network_id, port_id,
+                                                  format)
+        show_port_res = show_port_req.get_response(self.api)
+        self.assertEqual(show_port_res.status_int, 200)
+        port_data = self._port_serializer.deserialize(
+                        show_port_res.body, content_type)
+        self.assertEqual({'id': port_id, 'state': port_state},
+                         port_data['port'])
         LOG.debug("_test_set_port_state - format:%s - END", format)
 
     def _test_set_port_state_networknotfound(self, format):
@@ -491,7 +603,7 @@
                                                             interface_id,
                                                             format)
         put_attachment_res = put_attachment_req.get_response(self.api)
-        self.assertEqual(put_attachment_res.status_int, 202)
+        self.assertEqual(put_attachment_res.status_int, 204)
         get_attachment_req = testlib.get_attachment_request(self.tenant_id,
                                                             network_id,
                                                             port_id,
@@ -499,7 +611,7 @@
         get_attachment_res = get_attachment_req.get_response(self.api)
         attachment_data = Serializer().deserialize(get_attachment_res.body,
                                                    content_type)
-        self.assertEqual(attachment_data['attachment'], interface_id)
+        self.assertEqual(attachment_data['attachment']['id'], interface_id)
         LOG.debug("_test_show_attachment - format:%s - END", format)
 
     def _test_show_attachment_networknotfound(self, format):
@@ -544,7 +656,7 @@
                                                             interface_id,
                                                             format)
         put_attachment_res = put_attachment_req.get_response(self.api)
-        self.assertEqual(put_attachment_res.status_int, 202)
+        self.assertEqual(put_attachment_res.status_int, 204)
         LOG.debug("_test_put_attachment - format:%s - END", format)
 
     def _test_put_attachment_networknotfound(self, format):
@@ -593,13 +705,13 @@
                                                             interface_id,
                                                             format)
         put_attachment_res = put_attachment_req.get_response(self.api)
-        self.assertEqual(put_attachment_res.status_int, 202)
+        self.assertEqual(put_attachment_res.status_int, 204)
         del_attachment_req = testlib.delete_attachment_request(self.tenant_id,
                                                                network_id,
                                                                port_id,
                                                                format)
         del_attachment_res = del_attachment_req.get_response(self.api)
-        self.assertEqual(del_attachment_res.status_int, 202)
+        self.assertEqual(del_attachment_res.status_int, 204)
         LOG.debug("_test_delete_attachment - format:%s - END", format)
 
     def _test_delete_attachment_networknotfound(self, format):
@@ -670,6 +782,12 @@
     def test_list_networks_xml(self):
         self._test_list_networks('xml')
 
+    def test_list_networks_detail_json(self):
+        self._test_list_networks_detail('json')
+
+    def test_list_networks_detail_xml(self):
+        self._test_list_networks_detail('xml')
+
     def test_create_network_json(self):
         self._test_create_network('json')
 
@@ -694,6 +812,12 @@
     def test_show_network_xml(self):
         self._test_show_network('xml')
 
+    def test_show_network_detail_json(self):
+        self._test_show_network_detail('json')
+
+    def test_show_network_detail_xml(self):
+        self._test_show_network_detail('xml')
+
     def test_delete_network_json(self):
         self._test_delete_network('json')
 
@@ -730,12 +854,24 @@
     def test_list_ports_xml(self):
         self._test_list_ports('xml')
 
+    def test_list_ports_detail_json(self):
+        self._test_list_ports_detail('json')
+
+    def test_list_ports_detail_xml(self):
+        self._test_list_ports_detail('xml')
+
     def test_show_port_json(self):
         self._test_show_port('json')
 
     def test_show_port_xml(self):
         self._test_show_port('xml')
 
+    def test_show_port_detail_json(self):
+        self._test_show_port_detail('json')
+
+    def test_show_port_detail_xml(self):
+        self._test_show_port_detail('xml')
+
     def test_show_port_networknotfound_json(self):
         self._test_show_port_networknotfound('json')
 
 
=== modified file 'tests/unit/test_clientlib.py'
--- tests/unit/test_clientlib.py	2011-08-10 02:15:14 +0000
+++ tests/unit/test_clientlib.py	2011-08-24 11:55:39 +0000
@@ -43,7 +43,7 @@
             return self.content
 
         def status(self):
-            return status
+            return self.status
 
     # To test error codes, set the host to 10.0.0.1, and the port to the code
     def __init__(self, host, port=9696, key_file="", cert_file=""):
 
=== modified file 'tests/unit/testlib_api.py'
--- tests/unit/testlib_api.py	2011-08-01 14:40:29 +0000
+++ tests/unit/testlib_api.py	2011-08-24 11:55:39 +0000
@@ -12,26 +12,45 @@
     return req
 
 
+def _network_list_request(tenant_id, format='xml', detail=False):
+    method = 'GET'
+    detail_str = detail and '/detail' or ''
+    path = "/tenants/%(tenant_id)s/networks" \
+           "%(detail_str)s.%(format)s" % locals()
+    content_type = "application/%s" % format
+    return create_request(path, None, content_type, method)
+
+
 def network_list_request(tenant_id, format='xml'):
+    return _network_list_request(tenant_id, format)
+
+
+def network_list_detail_request(tenant_id, format='xml'):
+    return _network_list_request(tenant_id, format, detail=True)
+
+
+def _show_network_request(tenant_id, network_id, format='xml', detail=False):
     method = 'GET'
-    path = "/tenants/%(tenant_id)s/networks.%(format)s" % locals()
+    detail_str = detail and '/detail' or ''
+    path = "/tenants/%(tenant_id)s/networks" \
+           "/%(network_id)s%(detail_str)s.%(format)s" % locals()
     content_type = "application/%s" % format
     return create_request(path, None, content_type, method)
 
 
 def show_network_request(tenant_id, network_id, format='xml'):
-    method = 'GET'
-    path = "/tenants/%(tenant_id)s/networks" \
-           "/%(network_id)s.%(format)s" % locals()
-    content_type = "application/%s" % format
-    return create_request(path, None, content_type, method)
+    return _show_network_request(tenant_id, network_id, format)
+
+
+def show_network_detail_request(tenant_id, network_id, format='xml'):
+    return _show_network_request(tenant_id, network_id, format, detail=True)
 
 
 def new_network_request(tenant_id, network_name='new_name',
                         format='xml', custom_req_body=None):
     method = 'POST'
     path = "/tenants/%(tenant_id)s/networks.%(format)s" % locals()
-    data = custom_req_body or {'network': {'net-name': '%s' % network_name}}
+    data = custom_req_body or {'network': {'name': '%s' % network_name}}
     content_type = "application/%s" % format
     body = Serializer().serialize(data, content_type)
     return create_request(path, body, content_type, method)
@@ -42,7 +61,7 @@
     method = 'PUT'
     path = "/tenants/%(tenant_id)s/networks" \
            "/%(network_id)s.%(format)s" % locals()
-    data = custom_req_body or {'network': {'net-name': '%s' % network_name}}
+    data = custom_req_body or {'network': {'name': '%s' % network_name}}
     content_type = "application/%s" % format
     body = Serializer().serialize(data, content_type)
     return create_request(path, body, content_type, method)
@@ -56,20 +75,41 @@
     return create_request(path, None, content_type, method)
 
 
+def _port_list_request(tenant_id, network_id, format='xml', detail=False):
+    method = 'GET'
+    detail_str = detail and '/detail' or ''
+    path = "/tenants/%(tenant_id)s/networks/" \
+           "%(network_id)s/ports%(detail_str)s.%(format)s" % locals()
+    content_type = "application/%s" % format
+    return create_request(path, None, content_type, method)
+
+
 def port_list_request(tenant_id, network_id, format='xml'):
+    return _port_list_request(tenant_id, network_id, format)
+
+
+def port_list_detail_request(tenant_id, network_id, format='xml'):
+    return _port_list_request(tenant_id, network_id,
+                              format, detail=True)
+
+
+def _show_port_request(tenant_id, network_id, port_id,
+                       format='xml', detail=False):
     method = 'GET'
-    path = "/tenants/%(tenant_id)s/networks/" \
-           "%(network_id)s/ports.%(format)s" % locals()
+    detail_str = detail and '/detail' or ''
+    path = "/tenants/%(tenant_id)s/networks/%(network_id)s" \
+           "/ports/%(port_id)s%(detail_str)s.%(format)s" % locals()
     content_type = "application/%s" % format
     return create_request(path, None, content_type, method)
 
 
 def show_port_request(tenant_id, network_id, port_id, format='xml'):
-    method = 'GET'
-    path = "/tenants/%(tenant_id)s/networks/%(network_id)s" \
-           "/ports/%(port_id)s.%(format)s" % locals()
-    content_type = "application/%s" % format
-    return create_request(path, None, content_type, method)
+    return _show_port_request(tenant_id, network_id, port_id, format)
+
+
+def show_port_detail_request(tenant_id, network_id, port_id, format='xml'):
+    return _show_port_request(tenant_id, network_id, port_id,
+                              format, detail=True)
 
 
 def new_port_request(tenant_id, network_id, port_state,
@@ -78,7 +118,7 @@
     path = "/tenants/%(tenant_id)s/networks/" \
            "%(network_id)s/ports.%(format)s" % locals()
     data = custom_req_body or port_state and \
-           {'port': {'port-state': '%s' % port_state}}
+           {'port': {'state': '%s' % port_state}}
     content_type = "application/%s" % format
     body = data and Serializer().serialize(data, content_type)
     return create_request(path, body, content_type, method)
@@ -97,7 +137,7 @@
     method = 'PUT'
     path = "/tenants/%(tenant_id)s/networks" \
            "/%(network_id)s/ports/%(port_id)s.%(format)s" % locals()
-    data = custom_req_body or {'port': {'port-state': '%s' % port_state}}
+    data = custom_req_body or {'port': {'state': '%s' % port_state}}
     content_type = "application/%s" % format
     body = Serializer().serialize(data, content_type)
     return create_request(path, body, content_type, method)
@@ -116,7 +156,7 @@
     method = 'PUT'
     path = "/tenants/%(tenant_id)s/networks/" \
            "%(network_id)s/ports/%(port_id)s/attachment.%(format)s" % locals()
-    data = {'port': {'attachment-id': attachment_id}}
+    data = {'attachment': {'id': attachment_id}}
     content_type = "application/%s" % format
     body = Serializer().serialize(data, content_type)
     return create_request(path, body, content_type, method)