Create basic resource.props for id and ids attributes

[Terry Howe]

Create basic resource.props for id and ids attributes.

    [Done] block_store - https://review.openstack.org/#/c/280723/
    [Done] cluster - https://review.openstack.org/#/c/280748/
    [Done] compute
          - https://review.openstack.org/#/c/279618/
          - https://review.openstack.org/#/c/280280/
    [No Changes] database
    [Done] identity - https://review.openstack.org/#/c/279089/
    [Done] image - https://review.openstack.org/#/c/279581/
    [No Changes] key_manager
    [Done] message - https://review.openstack.org/#/c/281903/
    [No Changes] metric
    [Done] network - https://review.openstack.org/#/c/270900/
    [No Changes] object_store
    [Done] orchestration - https://review.openstack.org/#/c/280409/
    [Done] telemetry - https://review.openstack.org/#/c/278625/

See comments #3 and #4 for details.

Related: https://bugs.launchpad.net/python-openstacksdk/+bug/1417175

[Terry Howe (thowe-g)]

https://review.openstack.org/#/c/216237/

[OpenStack Infra (hudson-openstack)]

Change abandoned by Terry Howe (<email address hidden>) on branch: master
Review: https://review.openstack.org/207972
Reason: Got this covered elsewhere

[Richard Theis (rtheis)]

After investigating the network resources with respect to this bug, I'm wondering if we should leave the *_id properties as-is. Here are some of the concerns noted from my investigation:

1) The resulting resource only contains an ID (which we would need to document to avoid confusion). This isn't much better from a user perspective since additional proxy actions would be required to work with the resource. And these proxy actions work equally well with the *_id properties.

2) This will introduce a significant number of circular Python import dependencies (e.g. see network LBaaS v2 resources).

3) Handling cross service resources, such as tenant_id, requires extra care since the version of the resource is unknown.

4) Logically, we'd want to extend this bug to properties that are a list of *_id properties. However, doing so introduces additional circular Python import dependencies.

I'm looking for opinions on how to proceed. I'm personally leaning towards leaving the *_id properties as-is since I've yet to see the user benefit from the change to a resource.

[Brian Curtin (brian.curtin)]

After talking with Richard and Everett on IRC, I think we should probably drop this effort to create type=Resource for ID-based properties and revive https://bugs.launchpad.net/python-openstacksdk/+bug/1417175 to make sure we just have consistent naming for those props which are IDs.

I ran into the circular import issue in block_store (which is tiny), Richard ran into it with network (which is huge), and we're going to run into it with compute and identity for sure. It's going to prove very hard to maintain the approaches we've had to undertake to work around those circular dependencies, and because of that I think we run the risk of becoming inconsistent when it comes to these properties.

I think we had a good idea with this issue, but we probably can't ever realize its full value as what's currently provided is merely a wrapper around the string ID but providing no extra information. As our APIs already take either a Resource or an ID, nothing is currently gained by it outside of a slightly nicer string representation in the interpreter when you're toying around, but it's not a deal breaker as that is explained and documented elsewhere. Additionally, to realize the full potential of nested resources, we'd have to be able to make extra GET calls on those IDs, which we're not currently setup for (and would be quite messy to do) and also not likely to be worth the performance penalty of extra calls for marginally more useful information.

At this point, the way forward is probably to revert the existing changes that introduce the nested resources idea, and then ensure that we're consistent on using foo_id/foo_ids.

[OpenStack Infra (hudson-openstack)]

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

[Qiming Teng (tengqim)]

I don't think I understand the problem well because we never encountered circular reference problems so far. An example would be very helpful.

Dealing with nested resources automatically in SDK would be messy. I get this point.

Maybe we should just expose resource properties as is instead of making further assumptions? For properties that look like resource collections or resource instances, we are supposed to document them faithfully. Again, I don't really get the idea of "inventing" some property names that were not found from the service API response. For example, renaming 'pools' property of a healthmonitor to 'pool_ids' doesn't seem buying us anything?

[Richard Theis (rtheis)]

To address the specific concerns around the LBaaS v2 resources, we could leave the list of dicts properites as-is. Today these dicts only contain ids, but I think this could be expanded in the future since they are dicts. So maybe we only use _ids for a list of IDs. This will significantly reduce the changes made under https://review.openstack.org/270900. Thoughts?

[Richard Theis (rtheis)]

Here's an example of how Brian dealt with a specific circular resource problem: https://github.com/openstack/python-openstacksdk/commit/8c7ca1e62e5a834fc5e04a95225ecb6ae7025e60

[Brian Curtin (brian.curtin)]

Qiming: from the last points in your last message, while your one particular example isn't greatly improved within the scope of that particular resource, bringing consistency across the board to all other resources and services is what we really need to be mindful of. We currently have a mix of things like `foo = resource.prop("foo", type=foo.Foo)`, `foo = resource.prop("foo_id")`, and `foo_id = resource.prop("foo")` -- with what we've seen of the type=foo.Foo not being workable, we need to converge on one way to call properties that contain identifiers.

I pretty strongly believe that at this point, anything that contains IDs should be named as the following:
- A single ID value should be the singular name of the property with an "_id" suffix.
- Any sequence or collection of ID values should be the singular name of the property with an "_ids" suffix. This form includes lists, dicts, lists of dicts, dicts of lists, etc.

Adding _id/_ids suffixes to the names should bring a more readable and understandable set of properties without having to resort to the documentation. Of course it's no excuse for under-documenting things, but now that we're not going to have the properties be Resources themselves, I think there is now value to identifying those properties more explicitly.

[Richard Theis (rtheis)]

No changes are required for database, key_manager, metric and object_store.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/270900
Committed: https://git.openstack.org/cgit/openstack/python-openstacksdk/commit/?id=f71f6f0f1342cff143a63b6344746664a52a0591
Submitter: Jenkins
Branch: master

commit f71f6f0f1342cff143a63b6344746664a52a0591
Author: Richard Theis <email address hidden>
Date: Thu Jan 21 10:13:59 2016 -0600

    Basic resource.prop for ID attributes (network)

    This patch set updates all network resource objects to use basic
    properties for ID attributes. In particular, the following changes
    were made:
      - Use basic *_id resource.prop for ID attributes
      - Use list *_ids resource.prop for attributes containing a list of IDs
      - Clarify documentation for ID attributes

    Change-Id: I725095bb07da55148e29c511e7e8506bc7dbb7ca
    Partial-Bug: #1461200

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/278625
Committed: https://git.openstack.org/cgit/openstack/python-openstacksdk/commit/?id=7c9e4c3acc625de64f32cb2460eabafcf58a2535
Submitter: Jenkins
Branch: master

commit 7c9e4c3acc625de64f32cb2460eabafcf58a2535
Author: Richard Theis <email address hidden>
Date: Wed Feb 10 14:48:10 2016 -0600

    Basic resource.prop for ID attributes (telemetry)

    This patch set updates all telemetry resource objects to use basic
    properties for ID attributes. In particular, the following changes
    were made:
      - Use basic *_id resource.prop for ID attributes
      - Clarify documentation for ID attributes

    Change-Id: I8d78e5bd7aa94667501e0037ee84a1fea4d2c62a
    Partial-Bug: #1461200

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/279089
Committed: https://git.openstack.org/cgit/openstack/python-openstacksdk/commit/?id=a5a383fdd49ac834f7c9532756b3d81009bef58a
Submitter: Jenkins
Branch: master

commit a5a383fdd49ac834f7c9532756b3d81009bef58a
Author: Richard Theis <email address hidden>
Date: Thu Feb 11 07:59:10 2016 -0600

    Basic resource.prop for ID attributes (identity)

    This patch set updates all identity resource objects to use basic
    properties for ID attributes. In particular, the following changes
    were made:
     - Clarify documentation for ID attributes

    Change-Id: Ibd04e5b78274a60ea0cf4faea4783116c343ff37
    Partial-Bug: #1461200

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/279618
Committed: https://git.openstack.org/cgit/openstack/python-openstacksdk/commit/?id=8268a2eba4e8351177f53d2d7974061c92f5c08c
Submitter: Jenkins
Branch: master

commit 8268a2eba4e8351177f53d2d7974061c92f5c08c
Author: Richard Theis <email address hidden>
Date: Fri Feb 12 09:34:10 2016 -0600

    Basic resource.prop for ID attributes (compute)

    This patch set updates all compute resource objects to use basic
    properties for ID attributes. In particular, the following changes
    were made:
      - Use basic *_id resource.prop for ID attributes
      - Clarify documentation for ID attributes

    Change-Id: I48662698e9a272a9e42a46f6108ed79f5107d8fe
    Partial-Bug: #1461200

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/279581
Committed: https://git.openstack.org/cgit/openstack/python-openstacksdk/commit/?id=e201e460dfa3fbc9a3f0b8b17d11a98ff754f68e
Submitter: Jenkins
Branch: master

commit e201e460dfa3fbc9a3f0b8b17d11a98ff754f68e
Author: Richard Theis <email address hidden>
Date: Fri Feb 12 08:27:27 2016 -0600

    Basic resource.prop for ID attributes (image)

    This patch set updates all image resource objects to use basic
    properties for ID attributes. In particular, the following changes
    were made:
      - Use basic *_id resource.prop for ID attributes
      - Clarify documentation for ID attributes

    Change-Id: Ia2f65f566e6b5a03e865ce26122cd567712a08f1
    Partial-Bug: #1461200

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/280280
Committed: https://git.openstack.org/cgit/openstack/python-openstacksdk/commit/?id=b4b9104d5cd2cedf12c11360d77f36e6efd6ebbd
Submitter: Jenkins
Branch: master

commit b4b9104d5cd2cedf12c11360d77f36e6efd6ebbd
Author: Richard Theis <email address hidden>
Date: Mon Feb 15 09:12:57 2016 -0600

    Fix compute tests for resource.prop ID attributes

    Fix unit, functional and examples tests for [1]. Also
    fix the documentation for the server image_id and flavor_id
    properties.

    [1] https://review.openstack.org/#/c/279618/

    Change-Id: Ia37ecc7c9dccb90fabf7bf73601a6ad519212556
    Partial-Bug: #1461200

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/280409
Committed: https://git.openstack.org/cgit/openstack/python-openstacksdk/commit/?id=2cc41980a8e8b5d31a94ed70755bc1da6735e2ad
Submitter: Jenkins
Branch: master

commit 2cc41980a8e8b5d31a94ed70755bc1da6735e2ad
Author: Richard Theis <email address hidden>
Date: Mon Feb 15 13:54:04 2016 -0600

    Basic resource.prop for ID attributes (orchestration)

    This patch set updates all orchestration resource objects to use
    basic properties for ID attributes. In particular, the following
    changes were made:
      - Clarify documentation for ID attributes

    Change-Id: Idcf1d0215fbd3103794a9c7d07881f0fc591c993
    Partial-Bug: #1461200

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/280748
Committed: https://git.openstack.org/cgit/openstack/python-openstacksdk/commit/?id=38442dd3c06d9e11977b6509ca0e6b9d186d00d2
Submitter: Jenkins
Branch: master

commit 38442dd3c06d9e11977b6509ca0e6b9d186d00d2
Author: Brian Curtin <email address hidden>
Date: Tue Feb 16 10:02:28 2016 -0500

    Basic resource.prop for ID attributes (cluster)

    This patch set updates all cluster objects to use basic
    properties for ID attributes. In particular, the following changes
    were made:
      - Use basic *_id resource.prop for ID attributes

    Change-Id: I9b4a99fee5a32856f6a5023424e6f12c70618b29
    Partial-Bug: #1461200

[OpenStack Infra (hudson-openstack)]

Change abandoned by Terry Howe (<email address hidden>) on branch: master
Review: https://review.openstack.org/216237

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/281903
Committed: https://git.openstack.org/cgit/openstack/python-openstacksdk/commit/?id=f938857a4db930a10cfeda2b2ad0d86a758449b0
Submitter: Jenkins
Branch: master

commit f938857a4db930a10cfeda2b2ad0d86a758449b0
Author: Richard Theis <email address hidden>
Date: Thu Feb 18 09:12:16 2016 -0600

    Basic resource.prop for ID attributes (message)

    This patch set updates all message objects to use basic
    properties for ID attributes. In particular, the following changes
    were made:
      - Use basic *_id properties for ID attributes
      - Clarify documentation for ID attributes

    Change-Id: I8edfdf57e0411f01c41145f58182be9a8e5a47b0
    Partial-Bug: #1461200

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/280723
Committed: https://git.openstack.org/cgit/openstack/python-openstacksdk/commit/?id=ca4956635d7eac5485d2d713e2917394f95c713f
Submitter: Jenkins
Branch: master

commit ca4956635d7eac5485d2d713e2917394f95c713f
Author: Richard Theis <email address hidden>
Date: Mon Feb 15 15:12:33 2016 -0600

    Basic resource.prop for ID attributes (block store)

    This patch set updates all block store objects to use basic
    properties for ID attributes. In particular, the following changes
    were made:
      - Use basic *_id resource.prop for ID attributes
      - Clarify documentation for ID attributes
      - Revert https://github.com/openstack/python-openstacksdk/commit/8c7ca1e62e5a834fc5e04a95225ecb6ae7025e60

    In addition, functional tests were added for the snapshot,
    type and volume proxy interfaces.

    Change-Id: Ica597e65a59fc054a8544cd6e5888528bf1d2a70
    Partial-Bug: #1461200

[Richard Theis (rtheis)]

All fixes have been released.