)]}'
{"openstack_dashboard/api/base.py":[{"author":{"_account_id":2455,"name":"Tihomir Trifonov","email":"t.trifonov@gmail.com","username":"ttrifonov"},"change_message_id":"8078c605d58ada09e6c41fc76fdb84bb830488a4","unresolved":false,"context_lines":[{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def __getattribute__(self, attr):"},{"line_number":82,"context_line":"        try:"},{"line_number":83,"context_line":"            return object.__getattribute__(self, attr)"},{"line_number":84,"context_line":"        except AttributeError:"},{"line_number":85,"context_line":"            if attr not in self._attrs:"},{"line_number":86,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F9YWc%3D","line":83,"updated":"2014-02-07 10:47:46.000000000","message":"Being a wrapper, the APIResourceWrapper is intended to wrap _apiresource and usually the called attribute is one from the self._attrs rather than the class itself. Thus it is more likely that this code will most of the time raise AttributeError here?\n \nSince __getattr__ is invoked after a lookup in the class attributes is performed, so in the original code we know that the class has no such attribute and we need to do lookup into self._apiresource instead.\n\nWhat if we look in self._attrs first, and if doesn\u0027t find it - then perform the object.__getattribute__ ? Actually we do not need the try-except in that case.","commit_id":"f42799f2e1fdea9a5ea0e4baa72ccafa06351bf2"},{"author":{"_account_id":8648,"name":"Radomir Dopieralski","email":"openstack@dopieralski.pl","username":"thesheep"},"change_message_id":"4389c9a4d2333ee9f299c2c6d6f566efb5428dd6","unresolved":false,"context_lines":[{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def __getattribute__(self, attr):"},{"line_number":82,"context_line":"        try:"},{"line_number":83,"context_line":"            return object.__getattribute__(self, attr)"},{"line_number":84,"context_line":"        except AttributeError:"},{"line_number":85,"context_line":"            if attr not in self._attrs:"},{"line_number":86,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUX%2F%2F33c%3D","line":83,"in_reply_to":"AAAAUH%2F9YWc%3D","updated":"2014-02-10 08:55:40.000000000","message":"I actually tried that, but it changes the behavior of this wrapper and makes the tests fail in subtle ways. My guess is that someone is monkey-patching the wrapper to shadow the attributes from _attrs somewhere, so the current code relies on this behavior.","commit_id":"f42799f2e1fdea9a5ea0e4baa72ccafa06351bf2"},{"author":{"_account_id":2455,"name":"Tihomir Trifonov","email":"t.trifonov@gmail.com","username":"ttrifonov"},"change_message_id":"7d44276ea7f369b1e39b2ca2e987c4f36ccb00bf","unresolved":false,"context_lines":[{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def __getattribute__(self, attr):"},{"line_number":82,"context_line":"        try:"},{"line_number":83,"context_line":"            return object.__getattribute__(self, attr)"},{"line_number":84,"context_line":"        except AttributeError:"},{"line_number":85,"context_line":"            if attr not in self._attrs:"},{"line_number":86,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUX%2F%2Ftic%3D","line":83,"in_reply_to":"AAAAUX%2F%2F33c%3D","updated":"2014-02-11 09:51:25.000000000","message":"I think the current behavior is that the wrapper class is used to override some _attrs properties, so it\u0027s attrs should have priority over self._attrs. Can you try to implement a simple metaclass that will actually merge the attributes from _attrs with the provided to the wrapper class keeping the order - first class-defined, then _attrs? Then there will be no need of this complex method implementation.\n\nJust a simple example how it might look like:\n\n    class Meta(type):\n        def __new__(cls, name, bases, attrs):\n            attrs_list \u003d list(attrs)\n            for _attr in attrs.get(\u0027_attrs\u0027, []):                                                                                                            \n                if _attr not in attrs: attrs[_attr] \u003d property(lambda self: object.__getattribute__(self._apiresource, _attr))                             \n            return type.__new__(cls, name, bases, attrs)\n\nIn this case we will have possible overloading of _atttrs in wrapper class and the _attrs itself will look like regular attributes, thus invoking an attr will raise AttributeError as expected. What do you think of this?","commit_id":"f42799f2e1fdea9a5ea0e4baa72ccafa06351bf2"},{"author":{"_account_id":2455,"name":"Tihomir Trifonov","email":"t.trifonov@gmail.com","username":"ttrifonov"},"change_message_id":"e8638ec28c70a5d0a96333c22784ec3b326eb77e","unresolved":false,"context_lines":[{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def __getattribute__(self, attr):"},{"line_number":82,"context_line":"        try:"},{"line_number":83,"context_line":"            return object.__getattribute__(self, attr)"},{"line_number":84,"context_line":"        except AttributeError:"},{"line_number":85,"context_line":"            if attr not in self._attrs:"},{"line_number":86,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUX%2F%2FKO0%3D","line":83,"in_reply_to":"AAAAUX%2F%2FP%2FA%3D","updated":"2014-02-15 17:01:04.000000000","message":"Although some Python developers are kind of biased when using try-except, I prefer to keep to the Design and FAQ guidelines: http://docs.python.org/2/faq/design.html#how-fast-are-exceptions\n\nAnyway, there is another solution which might solve the problem as described in the bug description:\n\n    def __getattr__(self, attr):\n        if attr in self._attrs:\n            return getattr(self._apiresource, attr)\n        else:\n            return object.__getattribute__(self, attr)\n\nIn the case when the attr is not found in the parrent class and doesn\u0027t appear in self._attrs, then it is ok to invoke the original exception as simply calling the attribute on self. As long as it is a really exceptional state, for me it is acceptable to do it that way. Otherwise the code will always invoke and store the exception in case it might be needed if attr is not found in self._attrs. But it wouldn\u0027t be needed in most cases.","commit_id":"f42799f2e1fdea9a5ea0e4baa72ccafa06351bf2"},{"author":{"_account_id":2455,"name":"Tihomir Trifonov","email":"t.trifonov@gmail.com","username":"ttrifonov"},"change_message_id":"8db11e05cc3bb225882eead0f64b868cf48e83ec","unresolved":false,"context_lines":[{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def __getattribute__(self, attr):"},{"line_number":82,"context_line":"        try:"},{"line_number":83,"context_line":"            return object.__getattribute__(self, attr)"},{"line_number":84,"context_line":"        except AttributeError:"},{"line_number":85,"context_line":"            if attr not in self._attrs:"},{"line_number":86,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUX%2F%2FP%2FA%3D","line":83,"in_reply_to":"AAAAUX%2F%2Fsqk%3D","updated":"2014-02-14 14:55:24.000000000","message":"Performance.\nhttp://pastebin.com/A6KEd5Rt\n\n6x slower and growing when _attrs props are growing. try-except is slow.\n\nThis is one of the most-used classes:\n\n\n    class Server(base.APIResourceWrapper):\n\n        _attrs \u003d [\u0027addresses\u0027, \u0027attrs\u0027, \u0027id\u0027, \u0027image\u0027, \u0027links\u0027,\n             \u0027metadata\u0027, \u0027name\u0027, \u0027private_ip\u0027, \u0027public_ip\u0027, \u0027status\u0027, \u0027uuid\u0027,\n             \u0027image_name\u0027, \u0027VirtualInterfaces\u0027, \u0027flavor\u0027, \u0027key_name\u0027, \u0027fault\u0027,\n             \u0027tenant_id\u0027, \u0027user_id\u0027, \u0027created\u0027, \u0027OS-EXT-STS:power_state\u0027,\n             \u0027OS-EXT-STS:task_state\u0027, \u0027OS-EXT-SRV-ATTR:instance_name\u0027,\n             \u0027OS-EXT-SRV-ATTR:host\u0027, \u0027OS-EXT-AZ:availability_zone\u0027]","commit_id":"f42799f2e1fdea9a5ea0e4baa72ccafa06351bf2"},{"author":{"_account_id":8648,"name":"Radomir Dopieralski","email":"openstack@dopieralski.pl","username":"thesheep"},"change_message_id":"bdaf352ef4a76a834fe55129fdd7204787b61c49","unresolved":false,"context_lines":[{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def __getattribute__(self, attr):"},{"line_number":82,"context_line":"        try:"},{"line_number":83,"context_line":"            return object.__getattribute__(self, attr)"},{"line_number":84,"context_line":"        except AttributeError:"},{"line_number":85,"context_line":"            if attr not in self._attrs:"},{"line_number":86,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUX%2F%2Fsqk%3D","line":83,"in_reply_to":"AAAAUX%2F%2Fst4%3D","updated":"2014-02-11 12:10:35.000000000","message":"What\u0027s wrong with relying on try-except?","commit_id":"f42799f2e1fdea9a5ea0e4baa72ccafa06351bf2"},{"author":{"_account_id":2455,"name":"Tihomir Trifonov","email":"t.trifonov@gmail.com","username":"ttrifonov"},"change_message_id":"6ecbe6f46f5246d1538abbfa9d5c3cac47f05e47","unresolved":false,"context_lines":[{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def __getattribute__(self, attr):"},{"line_number":82,"context_line":"        try:"},{"line_number":83,"context_line":"            return object.__getattribute__(self, attr)"},{"line_number":84,"context_line":"        except AttributeError:"},{"line_number":85,"context_line":"            if attr not in self._attrs:"},{"line_number":86,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUX%2F%2Fst4%3D","line":83,"in_reply_to":"AAAAUX%2F%2FtBY%3D","updated":"2014-02-11 11:58:49.000000000","message":"The currently proposed implementation raises and catches AttributeError every time a call to _attrs[] property is made. Also, the wrapper simply maps external attributes from self._attrs to be easy to be invoked with direct class.attr calls. For me this is the ideal case of using a meta class, since we have a class that has its\u0027 properties externally defined, and metaclasses are useful to wrap non-standard attribute definitions into standard class structure. As you said - the two implementations here do the same job, except that the one doesn\u0027t rely on try-except for each call.","commit_id":"f42799f2e1fdea9a5ea0e4baa72ccafa06351bf2"},{"author":{"_account_id":8648,"name":"Radomir Dopieralski","email":"openstack@dopieralski.pl","username":"thesheep"},"change_message_id":"bdde8da7a795a34f1aa9b16ad2702eb072aff992","unresolved":false,"context_lines":[{"line_number":80,"context_line":""},{"line_number":81,"context_line":"    def __getattribute__(self, attr):"},{"line_number":82,"context_line":"        try:"},{"line_number":83,"context_line":"            return object.__getattribute__(self, attr)"},{"line_number":84,"context_line":"        except AttributeError:"},{"line_number":85,"context_line":"            if attr not in self._attrs:"},{"line_number":86,"context_line":"                raise"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUX%2F%2FtBY%3D","line":83,"in_reply_to":"AAAAUX%2F%2Ftic%3D","updated":"2014-02-11 11:06:47.000000000","message":"I don\u0027t think we actually need that. I think that my implementation is pretty simple and straightforward, and, what\u0027s more important, easy to understand and debug. Mucking around with metaclasses for no reason, just because they are cool is a mistake in my opinion. We gain nothing, and incur a considerable maintenance cost. What you are implementing there with a metaclass is what Python actually does with __getattribute__, so I see no reason to do it.","commit_id":"f42799f2e1fdea9a5ea0e4baa72ccafa06351bf2"}]}