keystoneclient unit tests fails on update tenants and users

[Adam Young]

  File "/opt/stack/keystone/tests/test_keystoneclient.py", line 270, in test_tenant_create_update_and_delete
    self.assertEquals(tenant.description, tenant_description)
  File "/opt/stack/keystone/vendor/python-keystoneclient-master/keystoneclient/base.py", line 287, in __getattr__
    raise AttributeError(k)

Same error on test_user_create_update_and_delete

The line that is triggering the error is marked with >>:

tenant = client.tenants.update(tenant_id=tenant.id,
                                       tenant_name=tenant_name,
                                       enabled=tenant_enabled,
                                       description=tenant_description)
>> self.assertEquals(tenant.name, tenant_name)nt.description, tenant_description)

Looking in the debugger, the tenant object looks like:
Tenant: <Tenant {u'id': u'5b4785e72ee34fff9b078ee53281f08f', u'name': u'updated_tenant', u'extra': {u'enabled': False, u'description': u'Updated tenant!'}}>

So the description is in the wrong location.

[Guang Yee (guang-yee)]

https://github.com/openstack/python-keystoneclient/blob/master/keystoneclient/base.py:line 91

def _create(self, url, body, response_key, return_raw=False):
        resp, body = self.api.post(url, body=body)
        if return_raw:
            return body[response_key]
        return self.resource_class(self, body[response_key], loaded=True)

This one seem to have something to do with lazy-loading. "loaded" is set to "True", which means never bother to fetch new.

https://github.com/openstack/python-keystoneclient/blob/master/keystoneclient/base.py:line 280-289

def __getattr__(self, k):
        if k not in self.__dict__:
            #NOTE(bcwaldon): disallow lazy-loading if already loaded once
            if not self.is_loaded():
                self.get()
                return self.__getattr__(k)

            raise AttributeError(k)
        else:
            return self.__dict__[k]

Question is why is "loaded" set to "True" in the latest code?

btw, this "lazy-loading" logic is convoluted. We may need to document its purpose.

[Joseph Heck (heckj)]

Guang -

the "loaded" is set to true because the entire resource has been retrieved in that code path, and the intention of the "loaded" attribute was to signify that maybe only a partial resource was created (it's possible to create a resource with just an ID) - at which point the first time any other attribute is used, it gets loaded in the background.

It's the common (albiet confusing and obscure) pattern behind all the manager/base resource mechanisms in the openstack clients.

[Guang Yee (guang-yee)]

Sounds good.

If I am reading the code correctly, on tenant update, the "description" is stashed into "extra" dict. On tenant create or get, "description" is part of "tenant" dict. Therefore, when "loaded" is set to "False", we do a tenant get when "description" is referenced and absent.

Is that by design, that "extra" is return on update only, but not on create or get?

If that's the case, the fix may be as simple as this?

def __getattr__(self, k):
        if k not in self.__dict__:
            #NOTE(bcwaldon): disallow lazy-loading if already loaded once
            if not self.is_loaded():
                self.get()
                return self.__getattr__(k)

            # check extra
            if 'extra' in self.__dict__ and \
                type(self.__dict__['extra']) is types.DictType and \
                k in self.__dict__['extra']:
                return self.__dict__['extra'].get(k)

            raise AttributeError(k)
        else:
            return self.__dict__[k]

[Dolph Mathews (dolph)]

description shouldn't be in 'extra' ... I don't believe extra was ever intended to be exposed by keystone, as it's really a sql backend implementation detail for non-indexed attributes. So, if the issue is that 'extra' is being returned by keystone, it's a server-side issue.

[Dolph Mathews (dolph)]

Potential fix was already in review in the v3 feature branch: https://review.openstack.org/#/c/12106/6/keystone/common/sql/core.py

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/15513
Committed: http://github.com/openstack/keystone/commit/df148a09fc1c7d44f2134a2dc6566ef1dbe772df
Submitter: Jenkins
Branch: master

commit df148a09fc1c7d44f2134a2dc6566ef1dbe772df
Author: Dolph Mathews <email address hidden>
Date: Tue Nov 6 17:01:59 2012 +0000

    Return non-indexed attrs, not 'extra' (bug 1075376)

    (most of this is pulled from the v3 branch)

    Change-Id: Id1118e7a2b245fb7ec95e41ec297c87036953db2