DisplayCommandBase returns two tuples bad comment

Bug #1477199 reported by Terry Howe
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
python-openstackclient
Fix Released
Wishlist
Tang Chen

Bug Description

The comment:

    # DisplayCommandBase.take_action() returns two tuples

Is all over the test code and it is wrong. We have no DisplayCommandBase class and take_action returns a tuple, not two tuples I think. Just git rid of this garbage.

Terry Howe (thowe-g)
description: updated
Revision history for this message
Steve Martinelli (stevemar) wrote :

i'd normally close this as invalid since it only touches tests, but the amount of hate is hilariously high, so i'm confirming it.

Changed in python-openstackclient:
status: New → Confirmed
importance: Undecided → Wishlist
assignee: nobody → Terry Howe (thowe-g)
Revision history for this message
Dean Troyer (dtroyer) wrote :

DisplayCommandBase is a cliff class and is the common ancestor of all of the commands that return values.

The error in the comment is that it is the descentant classes of cliff.DisplayCommandBase that define take_actions() return as a pair of tuples: http://git.openstack.org/cgit/openstack/cliff/tree/cliff/show.py#n28

If anything, the corection here is to expand the comment to be more precise, as DisplayCommandBase doesn't declare a take_action() method, but it is the ancestor of both ShowOne and Lister that both document it that way.

Revision history for this message
Terry Howe (thowe-g) wrote :

dtroyer_zz: http://git.openstack.org/cgit/openstack/cliff/tree/cliff/show.py#n28
[11:42am] dtroyer_zz: This is what expects the two tuples: http://git.openstack.org/cgit/openstack/cliff/tree/cliff/display.py#n92
[11:43am] dtroyer_zz: notice in the tests how often we either coerce the tuples into lists or the lists into tuples to do the comparison
[11:45am] terrylhowe: so, deep down there is some truth to it perhaps
[11:45am] dtroyer_zz: the only thing not true is the precision that DisplayComamndBase doesn't declare take_action() itself, but the descendant classes do.
[11:45am] dtroyer_zz: feel free to make that correction.
[11:45am] dtroyer_zz: otherwise the two-tuple return is valuable information

Revision history for this message
Terry Howe (thowe-g) wrote :

The way we have implemented it

* List generally returns a tuple which contains a tuple of columns and generator
* Show and Create return an iterator which contains a tuple of columns and tuple of data

I don't have the energy to go through all those comments and sort them.

Changed in python-openstackclient:
assignee: Terry Howe (thowe-g) → nobody
Tang Chen (tangchen)
Changed in python-openstackclient:
assignee: nobody → Tang Chen (tangchen)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to python-openstackclient (master)

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

Changed in python-openstackclient:
status: Confirmed → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to python-openstackclient (master)

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

commit b9de23d906a0ba640c336006e1b373a079846995
Author: Tang Chen <email address hidden>
Date: Sat Feb 6 09:34:51 2016 +0800

    Compute: Fix DisplayCommandBase comments for cliff Command subclass tests

    As bug #1477199 describes, the wrong comment below is all over the
    unit test code of OSC.

        # DisplayCommandBase.take_action() returns two tuples

    There is no such class named DisplayCommandBase in OSC. It is in cliff.

    All OSC command classes inherit from the base classes in cliff,
    class Command, class Lister and class ShowOne. It is like this:

    Object
    |--> Command
         |--> DisplayCommandBase
             |--> Lister
             |--> ShowOne

    take_action() is an abstract method of class Command, and generally is
    overwritten by subclasses.

    * Command.take_action() returns nothing.
    * Lister.take_action() returns a tuple which contains a tuple of
      columns and a generator used to generate the data.
    * ShowOne.take_action() returns an iterator which contains a tuple
      of columns and a tuple of data

    So, this problem should be fixed in 3 steps:
    1. Remove all DisplayCommandBase comments for tests of classes
       inheriting from class Command in cliff as it returns nothing.
    2. Fix all DisplayCommandBase comments for tests of classes
       inheriting from class Lister in cliff. Lister.take_action()
       returns a tuple and a generator.
    3. Fix all DisplayCommandBase comments for tests of classes
       inheriting from class ShowOne in cliff. ShowOne.take_action()
       returns two tuples.

    This patch finishes step 1 in compute tests.

    Change-Id: I99ab42a7de69af0e5de802a1bb5aac647245a200
    Partial-bug: #1477199

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

commit 3c67e8dd6ed20f9a3d6c1f8f000214cc7a8f384e
Author: Tang Chen <email address hidden>
Date: Sat Feb 6 10:30:34 2016 +0800

    Compute: Fix DisplayCommandBase comments for cliff Lister subclass tests

    As bug #1477199 describes, the wrong comment below is all over the
    unit test code of OSC.

        # DisplayCommandBase.take_action() returns two tuples

    There is no such class named DisplayCommandBase in OSC. It is in cliff.

    All OSC command classes inherit from the base classes in cliff,
    class Command, class Lister and class ShowOne. It is like this:

    Object
    |--> Command
         |--> DisplayCommandBase
             |--> Lister
             |--> ShowOne

    take_action() is an abstract method of class Command, and generally is
    overwritten by subclasses.

    * Command.take_action() returns nothing.
    * Lister.take_action() returns a tuple which contains a tuple of
      columns and a generator used to generate the data.
    * ShowOne.take_action() returns an iterator which contains a tuple
      of columns and a tuple of data

    So, this problem should be fixed in 3 steps:
    1. Remove all DisplayCommandBase comments for tests of classes
       inheriting from class Command in cliff as it returns nothing.
    2. Fix all DisplayCommandBase comments for tests of classes
       inheriting from class Lister in cliff. Lister.take_action()
       returns a tuple and a generator.
    3. Fix all DisplayCommandBase comments for tests of classes
       inheriting from class ShowOne in cliff. ShowOne.take_action()
       returns two tuples.

    This patch finishes step 2 in compute tests.

    Change-Id: Idc54ad21eaa1371ebd601327b8d962c7039f2de0
    Partial-bug: #1477199

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

commit 23faa33b1b2914bbc3e103fca75fde70eb317021
Author: Tang Chen <email address hidden>
Date: Sat Feb 6 10:47:56 2016 +0800

    Compute: Fix DisplayCommandBase comments for cliff ShowOne subclass tests

    As bug #1477199 describes, the wrong comment below is all over the
    unit test code of OSC.

        # DisplayCommandBase.take_action() returns two tuples

    There is no such class named DisplayCommandBase in OSC. It is in cliff.

    All OSC command classes inherit from the base classes in cliff,
    class Command, class Lister and class ShowOne. It is like this:

    Object
    |--> Command
         |--> DisplayCommandBase
             |--> Lister
             |--> ShowOne

    take_action() is an abstract method of class Command, and generally is
    overwritten by subclasses.

    * Command.take_action() returns nothing.
    * Lister.take_action() returns a tuple which contains a tuple of
      columns and a generator used to generate the data.
    * ShowOne.take_action() returns an iterator which contains a tuple
      of columns and a tuple of data

    So, this problem should be fixed in 3 steps:
    1. Remove all DisplayCommandBase comments for tests of classes
       inheriting from class Command in cliff as it returns nothing.
    2. Fix all DisplayCommandBase comments for tests of classes
       inheriting from class Lister in cliff. Lister.take_action()
       returns a tuple and a generator.
    3. Fix all DisplayCommandBase comments for tests of classes
       inheriting from class ShowOne in cliff. ShowOne.take_action()
       returns two tuples.

    This patch finishes step 3 in compute tests.

    Change-Id: I4df224ec82b5d82a3d6d3f366c0f68a7ea0d87cd
    Partial-bug: #1477199

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to python-openstackclient (master)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to python-openstackclient (master)

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

commit 43f80505cbaa0945380e366a8cada71b55296463
Author: Tang Chen <email address hidden>
Date: Tue Feb 9 18:27:48 2016 +0800

    Fix DisplayCommandBase comments for cliff Command subclass tests

    As bug #1477199 describes, the wrong comment below is all over the
    unit test code of OSC.

        # DisplayCommandBase.take_action() returns two tuples

    There is no such class named DisplayCommandBase in OSC. It is in cliff.

    All OSC command classes inherit from the base classes in cliff,
    class Command, class Lister and class ShowOne. It is like this:

    Object
    |--> Command
         |--> DisplayCommandBase
              |--> Lister
              |--> ShowOne

    take_action() is an abstract method of class Command, and generally is
    overwritten by subclasses.
    * Command.take_action() returns nothing.
    * Lister.take_action() returns a tuple which contains a tuple of columns
      and a generator used to generate the data.
    * ShowOne.take_action() returns an iterator which contains a tuple of
      columns and a tuple of data.

    So, this problem should be fixed in 3 steps:
    1. Remove all DisplayCommandBase comments for tests of classes inheriting
       from class Command in cliff as it returns nothing.
    2. Fix all DisplayCommandBase comments for tests of classes inheriting
       from class Lister in cliff. Lister.take_action() returns a tuple and
       a generator.
    3. Fix all DisplayCommandBase comments for tests of classes inheriting
       from class ShowOne in cliff. ShowOne.take_action() returns two tuples.

    This patch finishes step 1 in all but identity tests. There are too many
    such comments in identity tests. So fix them all in another patch.

    Change-Id: I9849baa8141ea8af2042a69afd540b77ce6ae6bd
    Partial-bug: #1477199

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

commit e69b88ef525445325843c129e1a113a92ed98e3a
Author: Tang Chen <email address hidden>
Date: Tue Feb 9 17:30:39 2016 +0800

    Fix DisplayCommandBase comments for cliff Lister subclass tests

    As bug #1477199 describes, the wrong comment below is all over the
    unit test code of OSC.

        # DisplayCommandBase.take_action() returns two tuples

    There is no such class named DisplayCommandBase in OSC. It is in cliff.

    All OSC command classes inherit from the base classes in cliff,
    class Command, class Lister and class ShowOne. It is like this:

    Object
    |--> Command
         |--> DisplayCommandBase
              |--> Lister
              |--> ShowOne

    take_action() is an abstract method of class Command, and generally is
    overwritten by subclasses.
    * Command.take_action() returns nothing.
    * Lister.take_action() returns a tuple which contains a tuple of columns
      and a generator used to generate the data.
    * ShowOne.take_action() returns an iterator which contains a tuple of
      columns and a tuple of data.

    So, this problem should be fixed in 3 steps:
    1. Remove all DisplayCommandBase comments for tests of classes inheriting
       from class Command in cliff as it returns nothing.
    2. Fix all DisplayCommandBase comments for tests of classes inheriting
       from class Lister in cliff. Lister.take_action() returns a tuple and
       a generator.
    3. Fix all DisplayCommandBase comments for tests of classes inheriting
       from class ShowOne in cliff. ShowOne.take_action() returns two tuples.

    This patch finishes step 2 in all but identity tests. There are too many
    such comments in identity tests. So fix them all in another patch.

    Change-Id: I00f38d12f55abe20fa708f6349073da658622f8d
    Partial-bug: #1477199

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

commit 35833f7bd82ac5d7cad4b5fba0e498d8936e6f48
Author: Tang Chen <email address hidden>
Date: Tue Feb 9 21:04:49 2016 +0800

    Fix DisplayCommandBase comments for cliff ShowOne subclass tests

    As bug #1477199 describes, the wrong comment below is all over the
    unit test code of OSC.

        # DisplayCommandBase.take_action() returns two tuples

    There is no such class named DisplayCommandBase in OSC. It is in cliff.

    All OSC command classes inherit from the base classes in cliff,
    class Command, class Lister and class ShowOne. It is like this:

    Object
    |--> Command
         |--> DisplayCommandBase
              |--> Lister
              |--> ShowOne

    take_action() is an abstract method of class Command, and generally is
    overwritten by subclasses.
    * Command.take_action() returns nothing.
    * Lister.take_action() returns a tuple which contains a tuple of columns
      and a generator used to generate the data.
    * ShowOne.take_action() returns an iterator which contains a tuple of
      columns and a tuple of data.

    So, this problem should be fixed in 3 steps:
    1. Remove all DisplayCommandBase comments for tests of classes inheriting
       from class Command in cliff as it returns nothing.
    2. Fix all DisplayCommandBase comments for tests of classes inheriting
       from class Lister in cliff. Lister.take_action() returns a tuple and
       a generator.
    3. Fix all DisplayCommandBase comments for tests of classes inheriting
       from class ShowOne in cliff. ShowOne.take_action() returns two tuples.

    This patch finishes step 3 in all but identity tests. There are too many
    such comments in identity tests. So fix them all in another patch.

    Change-Id: I1afe4852069d25d562a9448ec2bf2cff58955052
    Partial-bug: #1477199

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to python-openstackclient (master)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to python-openstackclient (master)

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

commit e1feed52217012da285ef94144ed82704b20d4e7
Author: Tang Chen <email address hidden>
Date: Wed Feb 10 19:01:52 2016 +0800

    Trivial: Fix "abstractmethod" to "abstract method"

    As Richard <email address hidden> has pointed out, "abstractmethod"
    should be "abstract method". This is a small typo I have made
    when I fix DisplayCommandBase comment bug.

    Change-Id: I84f1a3158896257686a0a7efa1123eef1b85139f
    Partial-bug: #1477199

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

commit 1225ad5f7efc91fda27b7c6a1c84f1c5cadc54a6
Author: Tang Chen <email address hidden>
Date: Wed Feb 10 19:15:37 2016 +0800

    Identity: Fix DisplayCommandBase comments for cliff Command subclass tests

    As bug #1477199 describes, the wrong comment below is all over the
    unit test code of OSC.

        # DisplayCommandBase.take_action() returns two tuples

    There is no such class named DisplayCommandBase in OSC. It is in cliff.

    All OSC command classes inherit from the base classes in cliff,
    class Command, class Lister and class ShowOne. It is like this:

    Object
    |--> Command
         |--> DisplayCommandBase
              |--> Lister
              |--> ShowOne

    take_action() is an abstract method of class Command, and generally is
    overwritten by subclasses.
    * Command.take_action() returns nothing.
    * Lister.take_action() returns a tuple which contains a tuple of columns
      and a generator used to generate the data.
    * ShowOne.take_action() returns an iterator which contains a tuple of
      columns and a tuple of data.

    So, this problem should be fixed in 3 steps:
    1. Remove all DisplayCommandBase comments for tests of classes inheriting
       from class Command in cliff as it returns nothing.
    2. Fix all DisplayCommandBase comments for tests of classes inheriting
       from class Lister in cliff. Lister.take_action() returns a tuple and
       a generator.
    3. Fix all DisplayCommandBase comments for tests of classes inheriting
       from class ShowOne in cliff. ShowOne.take_action() returns two tuples.

    This patch finishes step 1 in all identity tests.

    Change-Id: Id7180d10c050c6286b2c05cd990e2e275fbc3d38
    Partial-bug: #1477199

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

commit d0c0cefb84b4d7abff23e7e313acd9725df1be9b
Author: Tang Chen <email address hidden>
Date: Wed Feb 10 20:51:58 2016 +0800

    Identity: Fix DisplayCommandBase comments for cliff Lister subclass tests

    As bug #1477199 describes, the wrong comment below is all over the
    unit test code of OSC.

        # DisplayCommandBase.take_action() returns two tuples

    There is no such class named DisplayCommandBase in OSC. It is in cliff.

    All OSC command classes inherit from the base classes in cliff,
    class Command, class Lister and class ShowOne. It is like this:

    Object
    |--> Command
         |--> DisplayCommandBase
              |--> Lister
              |--> ShowOne

    take_action() is an abstract method of class Command, and generally is
    overwritten by subclasses.
    * Command.take_action() returns nothing.
    * Lister.take_action() returns a tuple which contains a tuple of columns
      and a generator used to generate the data.
    * ShowOne.take_action() returns an iterator which contains a tuple of
      columns and a tuple of data.

    So, this problem should be fixed in 3 steps:
    1. Remove all DisplayCommandBase comments for tests of classes inheriting
       from class Command in cliff as it returns nothing.
    2. Fix all DisplayCommandBase comments for tests of classes inheriting
       from class Lister in cliff. Lister.take_action() returns a tuple and
       a generator.
    3. Fix all DisplayCommandBase comments for tests of classes inheriting
       from class ShowOne in cliff. ShowOne.take_action() returns two tuples.

    This patch finishes step 2 in all identity tests.

    Change-Id: I2929ee688b1d7afc52c6ab325982bdc24c60a995
    Partial-bug: #1477199

Changed in python-openstackclient:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

commit 9f71b777ac7357c96ac7fa662ba18fa67b93baa0
Author: Tang Chen <email address hidden>
Date: Wed Feb 10 23:11:58 2016 +0800

    Identity: Fix DisplayCommandBase comments for cliff ShowOne subclass tests

    As bug #1477199 describes, the wrong comment below is all over the
    unit test code of OSC.

        # DisplayCommandBase.take_action() returns two tuples

    There is no such class named DisplayCommandBase in OSC. It is in cliff.

    All OSC command classes inherit from the base classes in cliff,
    class Command, class Lister and class ShowOne. It is like this:

    Object
    |--> Command
         |--> DisplayCommandBase
              |--> Lister
              |--> ShowOne

    take_action() is an abstract method of class Command, and generally is
    overwritten by subclasses.
    * Command.take_action() returns nothing.
    * Lister.take_action() returns a tuple which contains a tuple of columns
      and a generator used to generate the data.
    * ShowOne.take_action() returns an iterator which contains a tuple of
      columns and a tuple of data.

    So, this problem should be fixed in 3 steps:
    1. Remove all DisplayCommandBase comments for tests of classes inheriting
       from class Command in cliff as it returns nothing.
    2. Fix all DisplayCommandBase comments for tests of classes inheriting
       from class Lister in cliff. Lister.take_action() returns a tuple and
       a generator.
    3. Fix all DisplayCommandBase comments for tests of classes inheriting
       from class ShowOne in cliff. ShowOne.take_action() returns two tuples.

    This patch finishes step 3 in all identity tests.

    Change-Id: I1f05e833cdacd30915954e4220b6e1f16ac1ed40
    Closes-bug: #1477199

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.