__repr__ added to ApiResourceWrapper breaks e.g. Server list

[justinsb]

The recent patch "Neutron Security Group native support" added a __repr__ function to APIResourceWrapper.

But this now throws an error if any attributes in _attrs are not defined, which happens for me when I browse to the server list screen in horizon (note that I'm using a "special" version of nova which doesn't necessarily have the exact same JSON data). In particular, for servers, __repr__ now requires "attrs" and "private_ip", both of which I think are not in the OpenStack API spec (that I could find).

+ def __repr__(self):
+ return "<%s: %s>" % (self.__class__.__name__,
+ dict((attr,
+ getattr(self, attr))
+ for attr in self._attrs))
+

I'm not entirely sure why __repr__ was included in this patch, so I'm not comfortable just proposing a patch to rip it out.

[Akihiro Motoki (amotoki)]

Regarding __repr__ in APIResourceWrapper, I added it just from the debugging purpose.
openstack_dashboard API defines several wrapper class and it is not a good practice to define similar methods in each class.

However this method searches all attributes in _attrs list, so it descreases robustness of unknown attributes
as you reported. I agree we need some update to handle non-existing attributes in APIResourceWrapper.
I think there are two options avialable:
(1) To handle AttributeError in __repr__ and make it display existing attributes only.
(2) To remove __repr__() from APIResoruceWrapper. (The methods for debugging should be removed?)

I prefer the option (1). What do you think?

In addition, basically non-existing attrs should not be defined in _attrs.
I am not sure the reason why private_ip and attrs is defined in Server class.
If private_ip and attrs are not actually used, they should be removed.

[justinsb (justin-fathomdb)]

I prefer option 1; I think having better debugging is great (so thank you for writing it). But printing an object should never throw an exception. So, I would suggest, as you loop over, catch all exceptions; if it's an AttributeError then just skip the property silently; but if it is another error then maybe put a string representation of the error. Hopefully we won't have too many of that second case!

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/38628
Committed: http://github.com/openstack/horizon/commit/a06e263b60779739654ebf1020da59f231ca5da2
Submitter: Jenkins
Branch: master

commit a06e263b60779739654ebf1020da59f231ca5da2
Author: Akihiro MOTOKI <email address hidden>
Date: Thu Jul 25 21:38:31 2013 +0900

    Ignore non-existing attr in APIResourceWrapper __repr__

    Fixes bug 1202415

    __repr__ was added to APIResourceWrapper in the recent commit,
    but it searches all attributes in _attrs list. When an attribute
    defined in _attrs actually does not exist, __repr__() fails.
    This commit changes __repr__ to ignore non-existing attributes.

    Change-Id: Iebaeae78f3763d87f3993ba5c4bbed4c23e84c45