set/unset command classes should inherit from Command, not ShowOne.

[Tang Chen]

In OSC, set/unset command classes should inherit from class Command. There are 5 classes remain inheriting class ShowOne.

1. SetSecurityGroup
2. SetAgent
3. SetAggregate
4. SetFlavor
5. UnsetFlavor

They should be changed to inherit from class Command.

[Tang Chen (tangchen)]

BTW, two questions:

1. Is unset command necessary ? For example, "set xxx=None" means unset this property. But of course, I think this depends on the implementation on the server side.

2. For example, "flavor set --property ram=128", this won't update the ram of the flavor, but set a property named "ram" in the flavor._info. And the output may confuse the users：

+----------------------------+--------------------------------------+
| Field | Value |
+----------------------------+--------------------------------------+
| OS-FLV-DISABLED:disabled | False |
| OS-FLV-EXT-DATA:ephemeral | 0 |
| disk | 0 |
| id | c0215db9-7751-412a-a4d6-b9b429dcd7f7 |
| name | m1.aaa |
| os-flavor-access:is_public | True |
| properties | ram='1024' |
| ram | 256 |
| rxtx_factor | 1.0 |
| swap | |
| vcpus | 1 |
+----------------------------+--------------------------------------+

Shall we fix this problem ?

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/280663

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/280664

[OpenStack Infra (hudson-openstack)]

Change abandoned by Tang Chen (<email address hidden>) on branch: master
Review: https://review.openstack.org/280664
Reason: Merged to another review. https://review.openstack.org/#/c/280663/

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/281087

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/281088

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/281089

[Dean Troyer (dtroyer)]

To answer the questions:

1. Yes unset is necessary. We do not use 'magic values' such as setting a property to 'None'. We also need to allow a property to be empty but present. The unset command removed the property. All of this is independent of how the API actually is implemented, we want to present the same interface to the user.

2. A property name that overlaps an attribute name is a bad idea, but _some_ user set it to begin with. This may be one instance where we are giving the users the ability to hurt themselves by doing such things, but that is not always the wrong thing to do. In other places we restrict property names from duplicating known attributes. I am not sure we should do that and we'll need to follow up and make things consistent.

[Tang Chen (tangchen)]

Hi Dean,

1. OK.

2. I think properties should not duplicate attributes.

Both of these 2 problems influence lots of commands. I will post bug reports for them.

Thanks. :)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/280663
Committed: https://git.openstack.org/cgit/openstack/python-openstackclient/commit/?id=f37eda3a27dc88d3186d21eca328cca086ee3647
Submitter: Jenkins
Branch: master

commit f37eda3a27dc88d3186d21eca328cca086ee3647
Author: Tang Chen <email address hidden>
Date: Wed Feb 17 10:07:50 2016 +0800

    Make SetFlavor and UnsetFlavor inherit from cliff.Command

    set/unset comamnd classes should inherit from cliff.Command class.

    Change-Id: I54e5608ac0768d7d94b7f7d516ea1948daefdc1b
    Partial-Bug: 1546065

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/281087
Committed: https://git.openstack.org/cgit/openstack/python-openstackclient/commit/?id=859bfaf8757086c8607c1520c8018ab20e91a3ac
Submitter: Jenkins
Branch: master

commit 859bfaf8757086c8607c1520c8018ab20e91a3ac
Author: Tang Chen <email address hidden>
Date: Wed Feb 17 15:20:36 2016 +0800

    Make SetSecurityGroup inherit from cliff.Command

    set/unset comamnd classes should inherit from cliff.Command class.

    Change-Id: Ie28711ac8823dc9eb13cf83877864ca436b928bc
    Partial-Bug: 1546065

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/281088
Committed: https://git.openstack.org/cgit/openstack/python-openstackclient/commit/?id=9c91c1df4147cbd277c3384b0c648a6069c5f723
Submitter: Jenkins
Branch: master

commit 9c91c1df4147cbd277c3384b0c648a6069c5f723
Author: Tang Chen <email address hidden>
Date: Wed Feb 17 14:39:57 2016 +0800

    Make SetAgent inherit from cliff.Command

    set/unset command classes should inherit from cliff.Command class.

    Also, this patch adds functional tests for compute agent.

    Change-Id: I25eafffd1167f82aa0d430628c22dee7516b1e19
    Partial-Bug: 1546065

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/281089
Committed: https://git.openstack.org/cgit/openstack/python-openstackclient/commit/?id=ba826fa04fd5f16658da0319f34e26f14d7716d2
Submitter: Jenkins
Branch: master

commit ba826fa04fd5f16658da0319f34e26f14d7716d2
Author: Tang Chen <email address hidden>
Date: Wed Feb 17 15:16:33 2016 +0800

    Make SetAggregate inherit from cliff.Command

    set/unset comamnd classes should inherit from cliff.Command class.

    Also, this patch adds functional tests for aggregate.

    And also, use utils.format_dict() to format the output of the
    properties dict.

    Change-Id: Idb50bef8990da95666960e2414dfd7c9be234bba
    Partial-bug: #1519503
    Closes-Bug: 1546065

[Doug Hellmann (doug-hellmann)]

This issue was fixed in the openstack/python-openstackclient 2.2.0 release.