)]}'
{"ceilometer/nova_client.py":[{"author":{"_account_id":4491,"name":"Lianhao Lu","email":"llh_misc@outlook.com","username":"lianhao-lu"},"change_message_id":"a5487a9392a76a66dc80237d7feb5303e86d3d9f","unresolved":false,"context_lines":[{"line_number":69,"context_line":"            return"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        for attr, default in [(\u0027name\u0027, \u0027unknown-id-%s\u0027 % fid),"},{"line_number":72,"context_line":"                              (\u0027vcpus\u0027, 0), (\u0027ram\u0027, 0), (\u0027disk\u0027, 0)]:"},{"line_number":73,"context_line":"            instance.flavor[attr] \u003d getattr(flavor, attr, default)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def _with_image(self, instance):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAP3%2F%2Fz4g%3D","line":72,"updated":"2013-07-11 03:38:31.000000000","message":"how about put the list in a separate variable, so it would be easy to add new required attribute into the list if needed?","commit_id":"63715acac37f8894a874a31a65453d6f23e17a18"},{"author":{"_account_id":4277,"name":"John Tran","email":"jhtran@att.com","username":"jtran"},"change_message_id":"853d6533ceb6eac1e41ac8415d8cd35b317dfca1","unresolved":false,"context_lines":[{"line_number":69,"context_line":"            return"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        for attr, default in [(\u0027name\u0027, \u0027unknown-id-%s\u0027 % fid),"},{"line_number":72,"context_line":"                              (\u0027vcpus\u0027, 0), (\u0027ram\u0027, 0), (\u0027disk\u0027, 0)]:"},{"line_number":73,"context_line":"            instance.flavor[attr] \u003d getattr(flavor, attr, default)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def _with_image(self, instance):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAP3%2F%2FsPs%3D","line":72,"in_reply_to":"AAAAP3%2F%2Fz4g%3D","updated":"2013-07-12 22:19:56.000000000","message":"Done","commit_id":"63715acac37f8894a874a31a65453d6f23e17a18"},{"author":{"_account_id":2472,"name":"Doug Hellmann","email":"dhellmann@redhat.com","username":"doug-hellmann"},"change_message_id":"f18a294ddbe6a26263338d2dd7590035b4da8d6c","unresolved":false,"context_lines":[{"line_number":72,"context_line":"                              (\u0027vcpus\u0027, 0), (\u0027ram\u0027, 0), (\u0027disk\u0027, 0)]:"},{"line_number":73,"context_line":"            instance.flavor[attr] \u003d getattr(flavor, attr, default)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def _with_image(self, instance):"},{"line_number":76,"context_line":"        iid \u003d instance.image[\u0027id\u0027]"},{"line_number":77,"context_line":"        try:"},{"line_number":78,"context_line":"            image \u003d self.nova_client.images.get(iid)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAP3%2F%2Fsmw%3D","line":75,"updated":"2013-07-12 20:39:45.000000000","message":"I think with the changes you\u0027ve made, you can streamline or eliminate some of the error handling in this method. I\u0027ve tried to point out a couple of specific cases, but there may be others.","commit_id":"63715acac37f8894a874a31a65453d6f23e17a18"},{"author":{"_account_id":4277,"name":"John Tran","email":"jhtran@att.com","username":"jtran"},"change_message_id":"853d6533ceb6eac1e41ac8415d8cd35b317dfca1","unresolved":false,"context_lines":[{"line_number":72,"context_line":"                              (\u0027vcpus\u0027, 0), (\u0027ram\u0027, 0), (\u0027disk\u0027, 0)]:"},{"line_number":73,"context_line":"            instance.flavor[attr] \u003d getattr(flavor, attr, default)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def _with_image(self, instance):"},{"line_number":76,"context_line":"        iid \u003d instance.image[\u0027id\u0027]"},{"line_number":77,"context_line":"        try:"},{"line_number":78,"context_line":"            image \u003d self.nova_client.images.get(iid)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAP3%2F%2FsP8%3D","line":75,"in_reply_to":"AAAAP3%2F%2Fsmw%3D","updated":"2013-07-12 22:19:56.000000000","message":"Done , sorry I missed these the last go around.","commit_id":"63715acac37f8894a874a31a65453d6f23e17a18"},{"author":{"_account_id":2472,"name":"Doug Hellmann","email":"dhellmann@redhat.com","username":"doug-hellmann"},"change_message_id":"f18a294ddbe6a26263338d2dd7590035b4da8d6c","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        instance.image[\u0027name\u0027] \u003d getattr(image, \u0027name\u0027)"},{"line_number":86,"context_line":"        try:"},{"line_number":87,"context_line":"            image_metadata \u003d getattr(image, \u0027metadata\u0027)"},{"line_number":88,"context_line":"        except TypeError:"},{"line_number":89,"context_line":"            return"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        for attr in [\u0027kernel_id\u0027, \u0027ramdisk_id\u0027]:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAP3%2F%2Fsnk%3D","line":88,"updated":"2013-07-12 20:39:45.000000000","message":"Why would getattr() raise a TypeError?","commit_id":"63715acac37f8894a874a31a65453d6f23e17a18"},{"author":{"_account_id":4277,"name":"John Tran","email":"jhtran@att.com","username":"jtran"},"change_message_id":"853d6533ceb6eac1e41ac8415d8cd35b317dfca1","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        instance.image[\u0027name\u0027] \u003d getattr(image, \u0027name\u0027)"},{"line_number":86,"context_line":"        try:"},{"line_number":87,"context_line":"            image_metadata \u003d getattr(image, \u0027metadata\u0027)"},{"line_number":88,"context_line":"        except TypeError:"},{"line_number":89,"context_line":"            return"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"        for attr in [\u0027kernel_id\u0027, \u0027ramdisk_id\u0027]:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAP3%2F%2FsP0%3D","line":88,"in_reply_to":"AAAAP3%2F%2Fsnk%3D","updated":"2013-07-12 22:19:56.000000000","message":"Done","commit_id":"63715acac37f8894a874a31a65453d6f23e17a18"},{"author":{"_account_id":2472,"name":"Doug Hellmann","email":"dhellmann@redhat.com","username":"doug-hellmann"},"change_message_id":"f18a294ddbe6a26263338d2dd7590035b4da8d6c","unresolved":false,"context_lines":[{"line_number":91,"context_line":"        for attr in [\u0027kernel_id\u0027, \u0027ramdisk_id\u0027]:"},{"line_number":92,"context_line":"            try:"},{"line_number":93,"context_line":"                setattr(instance, attr, image_metadata.get(attr))"},{"line_number":94,"context_line":"            except (KeyError):"},{"line_number":95,"context_line":"                setattr(instance, attr, None)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    @logged"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAP3%2F%2Fsm8%3D","line":94,"updated":"2013-07-12 20:39:45.000000000","message":"When will you get a KeyError from the setattr() or .get() calls? I think you can remove the exception handling from the inside of this loop entirely.","commit_id":"63715acac37f8894a874a31a65453d6f23e17a18"},{"author":{"_account_id":4277,"name":"John Tran","email":"jhtran@att.com","username":"jtran"},"change_message_id":"853d6533ceb6eac1e41ac8415d8cd35b317dfca1","unresolved":false,"context_lines":[{"line_number":91,"context_line":"        for attr in [\u0027kernel_id\u0027, \u0027ramdisk_id\u0027]:"},{"line_number":92,"context_line":"            try:"},{"line_number":93,"context_line":"                setattr(instance, attr, image_metadata.get(attr))"},{"line_number":94,"context_line":"            except (KeyError):"},{"line_number":95,"context_line":"                setattr(instance, attr, None)"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    @logged"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAP3%2F%2FsPw%3D","line":94,"in_reply_to":"AAAAP3%2F%2Fsm8%3D","updated":"2013-07-12 22:19:56.000000000","message":"Done","commit_id":"63715acac37f8894a874a31a65453d6f23e17a18"},{"author":{"_account_id":2813,"name":"Mehdi Abaakouk","email":"sileht@sileht.net","username":"sileht"},"change_message_id":"a34c39924eb720a3bd84713077aa5db0da819d8c","unresolved":false,"context_lines":[{"line_number":87,"context_line":"        image_metadata \u003d getattr(image, \u0027metadata\u0027, None)"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"        for attr in [\u0027kernel_id\u0027, \u0027ramdisk_id\u0027]:"},{"line_number":90,"context_line":"            setattr(instance, attr, image_metadata.get(attr))"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    @logged"},{"line_number":93,"context_line":"    def instance_get_all_by_host(self, hostname):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAP3%2F%2FrpQ%3D","line":90,"updated":"2013-07-13 07:11:14.000000000","message":"image_metadata can be None\n\nimage_metadata.get( need to set a default value to ensure kernel_id and ramdisk_id to not raise a exception\n\nAnd you need to add some tests in case of image_metadata is None or if image_metadata don\u0027t have kernel_id or ramdisk_id.","commit_id":"e67f7cf21e33175143d3a74c0543fdf27d0ef0c1"},{"author":{"_account_id":2472,"name":"Doug Hellmann","email":"dhellmann@redhat.com","username":"doug-hellmann"},"change_message_id":"43a5ce0e97e98653fcc8741c2c212ffd197f3a83","unresolved":false,"context_lines":[{"line_number":69,"context_line":"            instance.flavor[\u0027name\u0027] \u003d \u0027unknown-id-%s\u0027 % fid"},{"line_number":70,"context_line":"            return"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        attr_defaults \u003d [(\u0027name\u0027, \u0027unknown-id-%s\u0027 % fid),"},{"line_number":73,"context_line":"                         (\u0027vcpus\u0027, 0), (\u0027ram\u0027, 0), (\u0027disk\u0027, 0)]"},{"line_number":74,"context_line":"        for attr, default in attr_defaults:"},{"line_number":75,"context_line":"            instance.flavor[attr] \u003d getattr(flavor, attr, default)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2FkUg%3D","line":72,"updated":"2013-07-16 13:52:52.000000000","message":"I like this defaulting logic. Why not apply it all the time, even when the flavor can\u0027t be found? That way all of the extra flavor-related attributes are there all the time.","commit_id":"75bcf7d0f08a47652d5222fe963150d4ccf72adc"},{"author":{"_account_id":4277,"name":"John Tran","email":"jhtran@att.com","username":"jtran"},"change_message_id":"d8b9a6c62b64b3cb69c360a6cb640021d6e9fa63","unresolved":false,"context_lines":[{"line_number":69,"context_line":"            instance.flavor[\u0027name\u0027] \u003d \u0027unknown-id-%s\u0027 % fid"},{"line_number":70,"context_line":"            return"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        attr_defaults \u003d [(\u0027name\u0027, \u0027unknown-id-%s\u0027 % fid),"},{"line_number":73,"context_line":"                         (\u0027vcpus\u0027, 0), (\u0027ram\u0027, 0), (\u0027disk\u0027, 0)]"},{"line_number":74,"context_line":"        for attr, default in attr_defaults:"},{"line_number":75,"context_line":"            instance.flavor[attr] \u003d getattr(flavor, attr, default)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAP3%2F%2FiW8%3D","line":72,"in_reply_to":"AAAAP3%2F%2FkUg%3D","updated":"2013-07-16 21:01:00.000000000","message":"Done","commit_id":"75bcf7d0f08a47652d5222fe963150d4ccf72adc"},{"author":{"_account_id":7975,"name":"Pavel Kirpichyov","email":"pavel.kirpichyov@gmail.com","username":"Cheshire"},"change_message_id":"2c6f30cecaa2b0c6d62c34c3b253ff9f8b56ae00","unresolved":false,"context_lines":[{"line_number":85,"context_line":"            instance.kernel_id \u003d None"},{"line_number":86,"context_line":"            instance.ramdisk_id \u003d None"},{"line_number":87,"context_line":"            return"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"        instance.image[\u0027name\u0027] \u003d getattr(image, \u0027name\u0027)"},{"line_number":90,"context_line":"        image_metadata \u003d getattr(image, \u0027metadata\u0027, None)"},{"line_number":91,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAP3%2F%2Ffuk%3D","line":88,"updated":"2013-07-17 12:30:28.000000000","message":"You can move below code into \"try\" block and remove \"return\" upper. It will be much cleaner","commit_id":"4260317932f01fc2cbdeefb5bf0d745295c86d22"}],"tests/test_novaclient.py":[{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"5bafd71b6b6e0b8bb6146b15793619ac4fed7147","unresolved":false,"context_lines":[{"line_number":70,"context_line":"            a.name \u003d \u0027rhel-6-x64\u0027"},{"line_number":71,"context_line":"            a.metadata \u003d {\u0027ramdisk_id\u0027: 21}"},{"line_number":72,"context_line":"        else:"},{"line_number":73,"context_line":"            raise novaclient.exceptions.NotFound(\u0027foobar\u0027)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"        return a"},{"line_number":76,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAP3%2F%2FezA%3D","line":73,"updated":"2013-07-17 16:15:47.000000000","message":"This multi-clause if is getting out of hand.\n\nHow about replacing with a more pythonic dict lookup?\n\ne.g.\n\n  image_details \u003d dict(\n      1\u003d(\u0027ubuntu-12.04-x86\u0027, dict(kernel_id\u003d11, ramdisk_id\u003d21)),\n      2\u003d(\u0027centos-5.4-x64\u0027, dict(kernel_id\u003d12, ramdisk_id\u003d22)),\n      ...\n  )\n  if a.id in image_details:\n      a.name \u003d image_details[a.id][0]\n      a.metadata \u003d image_details[a.id][1]\n  else:\n      raise novaclient.exceptions.NotFound(\u0027foobar\u0027)","commit_id":"4260317932f01fc2cbdeefb5bf0d745295c86d22"}]}