)]}'
{"aodh/notifier/zaqar.py":[{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"2d1b55ebffde928b3470192bc976d36f989efc98","unresolved":false,"context_lines":[{"line_number":93,"context_line":"            }"},{"line_number":94,"context_line":"        }"},{"line_number":95,"context_line":"        try:"},{"line_number":96,"context_line":"            from zaqarclient.queues import client as zaqar_client"},{"line_number":97,"context_line":"            return (zaqar_client.Client(self._get_endpoint(),"},{"line_number":98,"context_line":"                                        version\u003d2, conf\u003dparams),"},{"line_number":99,"context_line":"                    queue_info.get(\u0027queue_name\u0027))"}],"source_content_type":"text/x-python","patch_set":2,"id":"1aa78d24_508a2a1c","line":96,"range":{"start_line":96,"start_character":12,"end_line":96,"end_character":65},"updated":"2016-07-06 02:51:52.000000000","message":"can we avoid re-initializing zaqar client in every alarm notifying ? at least, avoid re-importing zaqarclient ?\n\nmay we can merge this method and the above one. just keep the import statement.","commit_id":"10461c300b1f1113a99294d7fbdb514b1f517ae9"},{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"036bc318652d9d39d96430611e3ae1a6fec00a03","unresolved":false,"context_lines":[{"line_number":93,"context_line":"            }"},{"line_number":94,"context_line":"        }"},{"line_number":95,"context_line":"        try:"},{"line_number":96,"context_line":"            from zaqarclient.queues import client as zaqar_client"},{"line_number":97,"context_line":"            return (zaqar_client.Client(self._get_endpoint(),"},{"line_number":98,"context_line":"                                        version\u003d2, conf\u003dparams),"},{"line_number":99,"context_line":"                    queue_info.get(\u0027queue_name\u0027))"}],"source_content_type":"text/x-python","patch_set":2,"id":"1aa78d24_fd44cab7","line":96,"range":{"start_line":96,"start_character":12,"end_line":96,"end_character":65},"in_reply_to":"1aa78d24_4406ef1d","updated":"2016-07-06 06:02:39.000000000","message":"after searching some docs about python, this sounds reasonable to me, thanks for the explanation!","commit_id":"10461c300b1f1113a99294d7fbdb514b1f517ae9"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"99875e18601d66462d55fa9f2681d5ceae68a58f","unresolved":false,"context_lines":[{"line_number":93,"context_line":"            }"},{"line_number":94,"context_line":"        }"},{"line_number":95,"context_line":"        try:"},{"line_number":96,"context_line":"            from zaqarclient.queues import client as zaqar_client"},{"line_number":97,"context_line":"            return (zaqar_client.Client(self._get_endpoint(),"},{"line_number":98,"context_line":"                                        version\u003d2, conf\u003dparams),"},{"line_number":99,"context_line":"                    queue_info.get(\u0027queue_name\u0027))"}],"source_content_type":"text/x-python","patch_set":2,"id":"1aa78d24_4406ef1d","line":96,"range":{"start_line":96,"start_character":12,"end_line":96,"end_character":65},"in_reply_to":"1aa78d24_508a2a1c","updated":"2016-07-06 03:17:36.000000000","message":"We have to initialize zaqar client for different alarm since they may talk to different tenants\u0027 queues. But don\u0027t worry about the performance, since when you initialize the client, Zaqar client won\u0027t do any auth, see https://github.com/openstack/python-zaqarclient/blob/master/zaqarclient/queues/v2/client.py#L44 So that said, it\u0027s basically just like you create a normal variable IMHO.\n\nBecause aodh is not adding zaqarclient as a requirement either in requirements.txt or test-requirements.txt, so we may have to import it each time. And IMHO, even merge this method and above, from coding view, the duplicated line is removed, but actually, it still be called each time. Besides, based on my understanding of Python VM, the VM maintains a list of modules which have already been imported, and any subsequent attempts to import that module result in a quick dict lookup in sys.modules and nothing else. So there shouldn\u0027t be much extra performance cost.\n\nI\u0027m happy to see comments from Python experts about this :)","commit_id":"10461c300b1f1113a99294d7fbdb514b1f517ae9"},{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"14b54e87b5659a05640b35ca70e12e1f289f6638","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        }"},{"line_number":99,"context_line":"        try:"},{"line_number":100,"context_line":"            from zaqarclient.queues import client as zaqar_client"},{"line_number":101,"context_line":"            return (zaqar_client.Client(self._get_endpoint(),"},{"line_number":102,"context_line":"                                        version\u003d2, conf\u003dparams),"},{"line_number":103,"context_line":"                    queue_name)"},{"line_number":104,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_d27855fb","line":101,"range":{"start_line":101,"start_character":40,"end_line":101,"end_character":60},"updated":"2016-07-18 02:26:11.000000000","message":"it is better to avoid retriving endpoint every notifying to avoid interactiving with keystone frequenetly, may can add a property \"zaqar_endpoint\" like following \"client\" definition.","commit_id":"28045d3df0a719d72c2752d22e0169433d003107"},{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"14b54e87b5659a05640b35ca70e12e1f289f6638","unresolved":false,"context_lines":[{"line_number":160,"context_line":"                    # NOTE(flwang): Zaqar will throw 409 error if there is"},{"line_number":161,"context_line":"                    # duplicated subscriber"},{"line_number":162,"context_line":"                    if getattr(e, \u0027code\u0027) \u003d\u003d 409:"},{"line_number":163,"context_line":"                        pass"},{"line_number":164,"context_line":"                    raise e"},{"line_number":165,"context_line":"            # post the message to the queue"},{"line_number":166,"context_line":"            queue.post(message)"}],"source_content_type":"text/x-python","patch_set":3,"id":"dada55a8_526d8531","line":163,"range":{"start_line":163,"start_character":26,"end_line":163,"end_character":27},"updated":"2016-07-18 02:26:11.000000000","message":"need a log statement ?","commit_id":"28045d3df0a719d72c2752d22e0169433d003107"},{"author":{"_account_id":18462,"name":"Rahul U Nair","email":"rahulunair@gmail.com","username":"rahulunair"},"change_message_id":"332d1fdbda73a796db670b5e597bf0083e6cc291","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        self._zendpoint \u003d None"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def _get_endpoint(self):"},{"line_number":46,"context_line":"        try:"},{"line_number":47,"context_line":"            if self._zendpoint is None:"},{"line_number":48,"context_line":"                ks_client \u003d keystone_client.get_client(self.conf)"},{"line_number":49,"context_line":"                endpoint_type \u003d self.conf.service_credentials.os_endpoint_type"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_a5027e09","line":46,"range":{"start_line":46,"start_character":8,"end_line":46,"end_character":11},"updated":"2016-07-19 06:09:56.000000000","message":"Feels like a wide try/except block, it would be better to break it into multiple and specific blocks?","commit_id":"1e7dc7472e4efd65a3c0ff66c457723f203e8069"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"25f69edf84d6bd20ac6ffc59fbb8c71ec606f531","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        self._zendpoint \u003d None"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def _get_endpoint(self):"},{"line_number":46,"context_line":"        try:"},{"line_number":47,"context_line":"            if self._zendpoint is None:"},{"line_number":48,"context_line":"                ks_client \u003d keystone_client.get_client(self.conf)"},{"line_number":49,"context_line":"                endpoint_type \u003d self.conf.service_credentials.os_endpoint_type"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_67eb37b1","line":46,"range":{"start_line":46,"start_character":8,"end_line":46,"end_character":11},"in_reply_to":"dada55a8_a5027e09","updated":"2016-07-19 23:40:08.000000000","message":"break into several blocks? you mean put the line ks_client \u003d keystone_client.get_client(self.conf) into a separated block? That\u0027s a little bit over-engineering, IMHO.\n\nAfter dig into the code, keystone_client.get_client[1] just call keystone v3 client[2] I don\u0027t think there is a good specific exception can be used at here. If you can figure out one, I\u0027m happy to fix it.\n\n[1] https://github.com/openstack/aodh/blob/master/aodh/keystone_client.py#L37\n\n[2] https://github.com/openstack/python-keystoneclient/blob/master/keystoneclient/v3/client.py#L193","commit_id":"1e7dc7472e4efd65a3c0ff66c457723f203e8069"},{"author":{"_account_id":18462,"name":"Rahul U Nair","email":"rahulunair@gmail.com","username":"rahulunair"},"change_message_id":"332d1fdbda73a796db670b5e597bf0083e6cc291","unresolved":false,"context_lines":[{"line_number":51,"context_line":"                    service_type\u003dself.conf.service_types.zaqar,"},{"line_number":52,"context_line":"                    endpoint_type\u003dendpoint_type)"},{"line_number":53,"context_line":"            return self._zendpoint"},{"line_number":54,"context_line":"        except Exception:"},{"line_number":55,"context_line":"            LOG.error(_LE(\"Aodh was configured to use zaqar:// action,\""},{"line_number":56,"context_line":"                          \" but Zaqar endpoint could not be found in Keystone\""},{"line_number":57,"context_line":"                          \" service catalog.\"))"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_6036b4e8","line":54,"range":{"start_line":54,"start_character":15,"end_line":54,"end_character":24},"updated":"2016-07-19 06:09:56.000000000","message":"It\u0027s better to use the specific exception, where ever possible, in this case it can be difficult as there are multiple location from which an exception can be raised, so as I have said in my previous comment, if the try except block was bit more specified, this can be avoided.","commit_id":"1e7dc7472e4efd65a3c0ff66c457723f203e8069"},{"author":{"_account_id":18462,"name":"Rahul U Nair","email":"rahulunair@gmail.com","username":"rahulunair"},"change_message_id":"80d18ee3170e551cd1d9b5f73801fbcd13543c22","unresolved":false,"context_lines":[{"line_number":51,"context_line":"                    service_type\u003dself.conf.service_types.zaqar,"},{"line_number":52,"context_line":"                    endpoint_type\u003dendpoint_type)"},{"line_number":53,"context_line":"            return self._zendpoint"},{"line_number":54,"context_line":"        except Exception:"},{"line_number":55,"context_line":"            LOG.error(_LE(\"Aodh was configured to use zaqar:// action,\""},{"line_number":56,"context_line":"                          \" but Zaqar endpoint could not be found in Keystone\""},{"line_number":57,"context_line":"                          \" service catalog.\"))"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_67692ef9","line":54,"range":{"start_line":54,"start_character":15,"end_line":54,"end_character":24},"in_reply_to":"dada55a8_47d4331f","updated":"2016-07-21 07:36:24.000000000","message":"Makes sense, if keystone authentication failure can cause an exception here, then updating that as a possibility in the log message would be nice.","commit_id":"1e7dc7472e4efd65a3c0ff66c457723f203e8069"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"25f69edf84d6bd20ac6ffc59fbb8c71ec606f531","unresolved":false,"context_lines":[{"line_number":51,"context_line":"                    service_type\u003dself.conf.service_types.zaqar,"},{"line_number":52,"context_line":"                    endpoint_type\u003dendpoint_type)"},{"line_number":53,"context_line":"            return self._zendpoint"},{"line_number":54,"context_line":"        except Exception:"},{"line_number":55,"context_line":"            LOG.error(_LE(\"Aodh was configured to use zaqar:// action,\""},{"line_number":56,"context_line":"                          \" but Zaqar endpoint could not be found in Keystone\""},{"line_number":57,"context_line":"                          \" service catalog.\"))"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_47d4331f","line":54,"range":{"start_line":54,"start_character":15,"end_line":54,"end_character":24},"in_reply_to":"dada55a8_6036b4e8","updated":"2016-07-19 23:40:08.000000000","message":"I would say it\u0027s out of the scope of this patch, but I don\u0027t mind fixing it at here. I can add a specific exception based on [1], but I think we still need to keep the generic exception at here.\n\n[1] https://github.com/openstack/python-keystoneclient/blob/master/keystoneclient/service_catalog.py#L297","commit_id":"1e7dc7472e4efd65a3c0ff66c457723f203e8069"},{"author":{"_account_id":1669,"name":"Julien Danjou","display_name":"jd","email":"julien@danjou.info","username":"jdanjou"},"change_message_id":"6716fafffda5a7f9c6e7f51dca8476c2c11b081d","unresolved":false,"context_lines":[{"line_number":51,"context_line":"                    service_type\u003dself.conf.service_types.zaqar,"},{"line_number":52,"context_line":"                    endpoint_type\u003dendpoint_type)"},{"line_number":53,"context_line":"            return self._zendpoint"},{"line_number":54,"context_line":"        except Exception:"},{"line_number":55,"context_line":"            LOG.error(_LE(\"Aodh was configured to use zaqar:// action,\""},{"line_number":56,"context_line":"                          \" but Zaqar endpoint could not be found in Keystone\""},{"line_number":57,"context_line":"                          \" service catalog.\"))"}],"source_content_type":"text/x-python","patch_set":4,"id":"bacf61ea_ae727c7a","line":54,"range":{"start_line":54,"start_character":15,"end_line":54,"end_character":24},"in_reply_to":"dada55a8_67692ef9","updated":"2016-07-28 07:45:24.000000000","message":"I agree this is bad, but it\u0027s not tied to this patch.","commit_id":"1e7dc7472e4efd65a3c0ff66c457723f203e8069"},{"author":{"_account_id":18462,"name":"Rahul U Nair","email":"rahulunair@gmail.com","username":"rahulunair"},"change_message_id":"332d1fdbda73a796db670b5e597bf0083e6cc291","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            from zaqarclient.queues import client as zaqar_client"},{"line_number":75,"context_line":"            return zaqar_client.Client(self._get_endpoint(),"},{"line_number":76,"context_line":"                                       version\u003d2, conf\u003dparams)"},{"line_number":77,"context_line":"        except Exception:"},{"line_number":78,"context_line":"            LOG.error(_LE(\"Failed to connect to Zaqar service \"),"},{"line_number":79,"context_line":"                      exc_info\u003dTrue)"},{"line_number":80,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_205b1c30","line":77,"range":{"start_line":77,"start_character":15,"end_line":77,"end_character":24},"updated":"2016-07-19 06:09:56.000000000","message":"Generic Exception?","commit_id":"1e7dc7472e4efd65a3c0ff66c457723f203e8069"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"25f69edf84d6bd20ac6ffc59fbb8c71ec606f531","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            from zaqarclient.queues import client as zaqar_client"},{"line_number":75,"context_line":"            return zaqar_client.Client(self._get_endpoint(),"},{"line_number":76,"context_line":"                                       version\u003d2, conf\u003dparams)"},{"line_number":77,"context_line":"        except Exception:"},{"line_number":78,"context_line":"            LOG.error(_LE(\"Failed to connect to Zaqar service \"),"},{"line_number":79,"context_line":"                      exc_info\u003dTrue)"},{"line_number":80,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_c7fae3bc","line":77,"range":{"start_line":77,"start_character":15,"end_line":77,"end_character":24},"in_reply_to":"dada55a8_205b1c30","updated":"2016-07-19 23:40:08.000000000","message":"Specific exception means you have to import zaqar client\u0027s exceptions, which is not the plugin want to do since it will cause adding a new hard dependency.","commit_id":"1e7dc7472e4efd65a3c0ff66c457723f203e8069"},{"author":{"_account_id":18462,"name":"Rahul U Nair","email":"rahulunair@gmail.com","username":"rahulunair"},"change_message_id":"80d18ee3170e551cd1d9b5f73801fbcd13543c22","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            from zaqarclient.queues import client as zaqar_client"},{"line_number":75,"context_line":"            return zaqar_client.Client(self._get_endpoint(),"},{"line_number":76,"context_line":"                                       version\u003d2, conf\u003dparams)"},{"line_number":77,"context_line":"        except Exception:"},{"line_number":78,"context_line":"            LOG.error(_LE(\"Failed to connect to Zaqar service \"),"},{"line_number":79,"context_line":"                      exc_info\u003dTrue)"},{"line_number":80,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_ca561f05","line":77,"range":{"start_line":77,"start_character":15,"end_line":77,"end_character":24},"in_reply_to":"dada55a8_c7fae3bc","updated":"2016-07-21 07:36:24.000000000","message":"Yup, I agree, thanks for clearing that.","commit_id":"1e7dc7472e4efd65a3c0ff66c457723f203e8069"},{"author":{"_account_id":18462,"name":"Rahul U Nair","email":"rahulunair@gmail.com","username":"rahulunair"},"change_message_id":"332d1fdbda73a796db670b5e597bf0083e6cc291","unresolved":false,"context_lines":[{"line_number":105,"context_line":"            return (zaqar_client.Client(self._get_endpoint(),"},{"line_number":106,"context_line":"                                        version\u003d2, conf\u003dparams),"},{"line_number":107,"context_line":"                    queue_name)"},{"line_number":108,"context_line":"        except Exception:"},{"line_number":109,"context_line":"            LOG.error(_LE(\"Failed to connect to Zaqar service \"),"},{"line_number":110,"context_line":"                      exc_info\u003dTrue)"},{"line_number":111,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_20f79cf6","line":108,"range":{"start_line":108,"start_character":15,"end_line":108,"end_character":24},"updated":"2016-07-19 06:09:56.000000000","message":"Generic Exception, can it be refactored and a more specific exception be raised?","commit_id":"1e7dc7472e4efd65a3c0ff66c457723f203e8069"},{"author":{"_account_id":18462,"name":"Rahul U Nair","email":"rahulunair@gmail.com","username":"rahulunair"},"change_message_id":"80d18ee3170e551cd1d9b5f73801fbcd13543c22","unresolved":false,"context_lines":[{"line_number":105,"context_line":"            return (zaqar_client.Client(self._get_endpoint(),"},{"line_number":106,"context_line":"                                        version\u003d2, conf\u003dparams),"},{"line_number":107,"context_line":"                    queue_name)"},{"line_number":108,"context_line":"        except Exception:"},{"line_number":109,"context_line":"            LOG.error(_LE(\"Failed to connect to Zaqar service \"),"},{"line_number":110,"context_line":"                      exc_info\u003dTrue)"},{"line_number":111,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_0a45d744","line":108,"range":{"start_line":108,"start_character":15,"end_line":108,"end_character":24},"in_reply_to":"dada55a8_20f79cf6","updated":"2016-07-21 07:36:24.000000000","message":"+1","commit_id":"1e7dc7472e4efd65a3c0ff66c457723f203e8069"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"25f69edf84d6bd20ac6ffc59fbb8c71ec606f531","unresolved":false,"context_lines":[{"line_number":105,"context_line":"            return (zaqar_client.Client(self._get_endpoint(),"},{"line_number":106,"context_line":"                                        version\u003d2, conf\u003dparams),"},{"line_number":107,"context_line":"                    queue_name)"},{"line_number":108,"context_line":"        except Exception:"},{"line_number":109,"context_line":"            LOG.error(_LE(\"Failed to connect to Zaqar service \"),"},{"line_number":110,"context_line":"                      exc_info\u003dTrue)"},{"line_number":111,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_472d5345","line":108,"range":{"start_line":108,"start_character":15,"end_line":108,"end_character":24},"in_reply_to":"dada55a8_20f79cf6","updated":"2016-07-19 23:40:08.000000000","message":"ditto","commit_id":"1e7dc7472e4efd65a3c0ff66c457723f203e8069"},{"author":{"_account_id":18462,"name":"Rahul U Nair","email":"rahulunair@gmail.com","username":"rahulunair"},"change_message_id":"332d1fdbda73a796db670b5e597bf0083e6cc291","unresolved":false,"context_lines":[{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    def notify_zaqar(self, action, message):"},{"line_number":138,"context_line":"        queue_info \u003d urlparse.parse_qs(action.query)"},{"line_number":139,"context_line":"        try:"},{"line_number":140,"context_line":"            # NOTE(flwang): Try to get build a pre-signed client if user has"},{"line_number":141,"context_line":"            # provide enough information about that. Otherwise, go to build"},{"line_number":142,"context_line":"            # a client with service account and queue name for this alarm."}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_40429011","line":139,"range":{"start_line":139,"start_character":8,"end_line":139,"end_character":11},"updated":"2016-07-19 06:09:56.000000000","message":"A better approach would be to split the try/except so that, it can be easily understood, what kind of exception can be raised by which part.","commit_id":"1e7dc7472e4efd65a3c0ff66c457723f203e8069"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"25f69edf84d6bd20ac6ffc59fbb8c71ec606f531","unresolved":false,"context_lines":[{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    def notify_zaqar(self, action, message):"},{"line_number":138,"context_line":"        queue_info \u003d urlparse.parse_qs(action.query)"},{"line_number":139,"context_line":"        try:"},{"line_number":140,"context_line":"            # NOTE(flwang): Try to get build a pre-signed client if user has"},{"line_number":141,"context_line":"            # provide enough information about that. Otherwise, go to build"},{"line_number":142,"context_line":"            # a client with service account and queue name for this alarm."}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_87d2abd0","line":139,"range":{"start_line":139,"start_character":8,"end_line":139,"end_character":11},"in_reply_to":"dada55a8_40429011","updated":"2016-07-19 23:40:08.000000000","message":"Don\u0027t be scared by the lines, except the comments and try/catch, there are only few lines about the real logic and all of them have been covered. Can you let me know which part should be put in a separated block?","commit_id":"1e7dc7472e4efd65a3c0ff66c457723f203e8069"},{"author":{"_account_id":18462,"name":"Rahul U Nair","email":"rahulunair@gmail.com","username":"rahulunair"},"change_message_id":"80d18ee3170e551cd1d9b5f73801fbcd13543c22","unresolved":false,"context_lines":[{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    def notify_zaqar(self, action, message):"},{"line_number":138,"context_line":"        queue_info \u003d urlparse.parse_qs(action.query)"},{"line_number":139,"context_line":"        try:"},{"line_number":140,"context_line":"            # NOTE(flwang): Try to get build a pre-signed client if user has"},{"line_number":141,"context_line":"            # provide enough information about that. Otherwise, go to build"},{"line_number":142,"context_line":"            # a client with service account and queue name for this alarm."}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_4ae7cf3f","line":139,"range":{"start_line":139,"start_character":8,"end_line":139,"end_character":11},"in_reply_to":"dada55a8_87d2abd0","updated":"2016-07-21 07:36:24.000000000","message":"This is on me, I missed the except lines for IndexError, yes  all of them have been covered.","commit_id":"1e7dc7472e4efd65a3c0ff66c457723f203e8069"},{"author":{"_account_id":1669,"name":"Julien Danjou","display_name":"jd","email":"julien@danjou.info","username":"jdanjou"},"change_message_id":"6716fafffda5a7f9c6e7f51dca8476c2c11b081d","unresolved":false,"context_lines":[{"line_number":164,"context_line":"                    # NOTE(flwang): Zaqar will throw 409 error if there is"},{"line_number":165,"context_line":"                    # duplicated subscriber, this will be changed after"},{"line_number":166,"context_line":"                    # Zaqar support subscription confirmation"},{"line_number":167,"context_line":"                    if getattr(e, \u0027code\u0027) \u003d\u003d 409:"},{"line_number":168,"context_line":"                        LOG.debug(\u0027The subscription for: %s has been created \u0027"},{"line_number":169,"context_line":"                                  \u0027already.\u0027 % subscriber)"},{"line_number":170,"context_line":"                        pass"}],"source_content_type":"text/x-python","patch_set":4,"id":"bacf61ea_ee03d4b6","line":167,"range":{"start_line":167,"start_character":23,"end_line":167,"end_character":41},"updated":"2016-07-28 07:45:24.000000000","message":"This will raise AttributeError if code is not an attribute.\n\nIf you want to check that, catch something else than Exception.","commit_id":"1e7dc7472e4efd65a3c0ff66c457723f203e8069"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"235d476cbe117f08f613d7309894a27db6247b47","unresolved":false,"context_lines":[{"line_number":164,"context_line":"                    # NOTE(flwang): Zaqar will throw 409 error if there is"},{"line_number":165,"context_line":"                    # duplicated subscriber, this will be changed after"},{"line_number":166,"context_line":"                    # Zaqar support subscription confirmation"},{"line_number":167,"context_line":"                    if getattr(e, \u0027code\u0027) \u003d\u003d 409:"},{"line_number":168,"context_line":"                        LOG.debug(\u0027The subscription for: %s has been created \u0027"},{"line_number":169,"context_line":"                                  \u0027already.\u0027 % subscriber)"},{"line_number":170,"context_line":"                        pass"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a89bdaa_abc9997a","line":167,"range":{"start_line":167,"start_character":23,"end_line":167,"end_character":41},"in_reply_to":"bacf61ea_6e3b64ec","updated":"2016-09-05 19:07:50.000000000","message":"We don\u0027t need this catch, because zaqar recently changed this behaviour. It will be successful if there is a duplicated subscription instead of failing with 409.","commit_id":"1e7dc7472e4efd65a3c0ff66c457723f203e8069"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"98142beaac87b843c57712152635c8d32940f12e","unresolved":false,"context_lines":[{"line_number":164,"context_line":"                    # NOTE(flwang): Zaqar will throw 409 error if there is"},{"line_number":165,"context_line":"                    # duplicated subscriber, this will be changed after"},{"line_number":166,"context_line":"                    # Zaqar support subscription confirmation"},{"line_number":167,"context_line":"                    if getattr(e, \u0027code\u0027) \u003d\u003d 409:"},{"line_number":168,"context_line":"                        LOG.debug(\u0027The subscription for: %s has been created \u0027"},{"line_number":169,"context_line":"                                  \u0027already.\u0027 % subscriber)"},{"line_number":170,"context_line":"                        pass"}],"source_content_type":"text/x-python","patch_set":4,"id":"bacf61ea_6e3b64ec","line":167,"range":{"start_line":167,"start_character":23,"end_line":167,"end_character":41},"in_reply_to":"bacf61ea_ee03d4b6","updated":"2016-07-28 08:10:35.000000000","message":"Nice catch, but if I catch the specific error, that means I have to import the error. So I will give it a default value anyway. Thanks.","commit_id":"1e7dc7472e4efd65a3c0ff66c457723f203e8069"},{"author":{"_account_id":1669,"name":"Julien Danjou","display_name":"jd","email":"julien@danjou.info","username":"jdanjou"},"change_message_id":"4d449fba3ebddf030eae63584227a748a21c487c","unresolved":false,"context_lines":[{"line_number":160,"context_line":"                try:"},{"line_number":161,"context_line":"                    zaqar_client.subscription(queue_name,"},{"line_number":162,"context_line":"                                              **subscription_data)"},{"line_number":163,"context_line":"                except Exception as e:"},{"line_number":164,"context_line":"                    # NOTE(flwang): Zaqar will throw 409 error if there is"},{"line_number":165,"context_line":"                    # duplicated subscriber, this will be changed after"},{"line_number":166,"context_line":"                    # Zaqar support subscription confirmation"},{"line_number":167,"context_line":"                    if getattr(e, \u0027code\u0027, None) \u003d\u003d 409:"},{"line_number":168,"context_line":"                        LOG.debug(\u0027The subscription for: %s has been created \u0027"},{"line_number":169,"context_line":"                                  \u0027already.\u0027 % subscriber)"},{"line_number":170,"context_line":"                        pass"}],"source_content_type":"text/x-python","patch_set":5,"id":"bacf61ea_81aca9ee","line":167,"range":{"start_line":163,"start_character":15,"end_line":167,"end_character":55},"updated":"2016-07-28 08:10:29.000000000","message":"Any chance to really check .code if it\u0027s the right type?","commit_id":"17b2d6a6dfe200a4e0ac1a0668743b086b47a5f5"},{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"8cfd5189a631b42210b023e0d3804c4f3da7250d","unresolved":false,"context_lines":[{"line_number":160,"context_line":"                try:"},{"line_number":161,"context_line":"                    zaqar_client.subscription(queue_name,"},{"line_number":162,"context_line":"                                              **subscription_data)"},{"line_number":163,"context_line":"                except Exception as e:"},{"line_number":164,"context_line":"                    # NOTE(flwang): Zaqar will throw 409 error if there is"},{"line_number":165,"context_line":"                    # duplicated subscriber, this will be changed after"},{"line_number":166,"context_line":"                    # Zaqar support subscription confirmation"},{"line_number":167,"context_line":"                    if getattr(e, \u0027code\u0027, None) \u003d\u003d 409:"},{"line_number":168,"context_line":"                        LOG.debug(\u0027The subscription for: %s has been created \u0027"},{"line_number":169,"context_line":"                                  \u0027already.\u0027 % subscriber)"},{"line_number":170,"context_line":"                        pass"}],"source_content_type":"text/x-python","patch_set":5,"id":"bacf61ea_e103e5b6","line":167,"range":{"start_line":163,"start_character":15,"end_line":167,"end_character":55},"in_reply_to":"bacf61ea_81aca9ee","updated":"2016-07-28 08:15:15.000000000","message":"When you say \u0027right type\u0027, do you mean the error raised by Zaqar client, if so, as I mentioned before, that means we have to import zaqar client, and that will cause a hard dependency. Is that ok?\n\nAnd as I noted above, this will be changed soon(in this release I\u0027m sure), I will propose another patch to remove this ugly check.","commit_id":"17b2d6a6dfe200a4e0ac1a0668743b086b47a5f5"},{"author":{"_account_id":1669,"name":"Julien Danjou","display_name":"jd","email":"julien@danjou.info","username":"jdanjou"},"change_message_id":"365ffa2e2ce53bcdd52af7d52c8d6961f2c28e15","unresolved":false,"context_lines":[{"line_number":160,"context_line":"                try:"},{"line_number":161,"context_line":"                    zaqar_client.subscription(queue_name,"},{"line_number":162,"context_line":"                                              **subscription_data)"},{"line_number":163,"context_line":"                except Exception as e:"},{"line_number":164,"context_line":"                    # NOTE(flwang): Zaqar will throw 409 error if there is"},{"line_number":165,"context_line":"                    # duplicated subscriber, this will be changed after"},{"line_number":166,"context_line":"                    # Zaqar support subscription confirmation"},{"line_number":167,"context_line":"                    if getattr(e, \u0027code\u0027, None) \u003d\u003d 409:"},{"line_number":168,"context_line":"                        LOG.debug(\u0027The subscription for: %s has been created \u0027"},{"line_number":169,"context_line":"                                  \u0027already.\u0027 % subscriber)"},{"line_number":170,"context_line":"                        pass"}],"source_content_type":"text/x-python","patch_set":5,"id":"bacf61ea_0109d995","line":167,"range":{"start_line":163,"start_character":15,"end_line":167,"end_character":55},"in_reply_to":"bacf61ea_e103e5b6","updated":"2016-07-28 08:21:24.000000000","message":"Yes, I don\u0027t care about the requirement to be hard. I realize it\u0027s currently not listed, but I think it\u0027d be better to list it and not run around the code like  that…","commit_id":"17b2d6a6dfe200a4e0ac1a0668743b086b47a5f5"},{"author":{"_account_id":1669,"name":"Julien Danjou","display_name":"jd","email":"julien@danjou.info","username":"jdanjou"},"change_message_id":"f43a26024f9813bf1d602946bb96bd1788adc3ca","unresolved":false,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def _get_endpoint(self):"},{"line_number":46,"context_line":"        try:"},{"line_number":47,"context_line":"            if self._zendpoint is None:"},{"line_number":48,"context_line":"                ks_client \u003d keystone_client.get_client(self.conf)"},{"line_number":49,"context_line":"                endpoint_type \u003d self.conf.service_credentials.os_endpoint_type"},{"line_number":50,"context_line":"                self._zendpoint \u003d ks_client.service_catalog.url_for("}],"source_content_type":"text/x-python","patch_set":6,"id":"7a8ec9b2_c74b5b58","line":47,"updated":"2016-09-14 08:17:49.000000000","message":"nor the if and return can raise an Exception so might be better to leave them out the try/except","commit_id":"fb22a3ef6f96a9358e476a0b6e7cff591e7f45c3"},{"author":{"_account_id":1669,"name":"Julien Danjou","display_name":"jd","email":"julien@danjou.info","username":"jdanjou"},"change_message_id":"f43a26024f9813bf1d602946bb96bd1788adc3ca","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            from zaqarclient.queues import client as zaqar_client"},{"line_number":75,"context_line":"            return zaqar_client.Client(self._get_endpoint(),"},{"line_number":76,"context_line":"                                       version\u003d2, conf\u003dparams)"},{"line_number":77,"context_line":"        except Exception:"},{"line_number":78,"context_line":"            LOG.error(_LE(\"Failed to connect to Zaqar service \"),"},{"line_number":79,"context_line":"                      exc_info\u003dTrue)"},{"line_number":80,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7a8ec9b2_079b83ca","line":77,"range":{"start_line":77,"start_character":15,"end_line":77,"end_character":24},"updated":"2016-09-14 08:17:49.000000000","message":"This is very wild… if the import fail, the error raised will not help an operator to realize the client is not installed for example","commit_id":"fb22a3ef6f96a9358e476a0b6e7cff591e7f45c3"},{"author":{"_account_id":1669,"name":"Julien Danjou","display_name":"jd","email":"julien@danjou.info","username":"jdanjou"},"change_message_id":"f43a26024f9813bf1d602946bb96bd1788adc3ca","unresolved":false,"context_lines":[{"line_number":105,"context_line":"            return (zaqar_client.Client(self._get_endpoint(),"},{"line_number":106,"context_line":"                                        version\u003d2, conf\u003dparams),"},{"line_number":107,"context_line":"                    queue_name)"},{"line_number":108,"context_line":"        except Exception:"},{"line_number":109,"context_line":"            LOG.error(_LE(\"Failed to connect to Zaqar service \"),"},{"line_number":110,"context_line":"                      exc_info\u003dTrue)"},{"line_number":111,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7a8ec9b2_c7b99b30","line":108,"updated":"2016-09-14 08:17:49.000000000","message":"ditto","commit_id":"fb22a3ef6f96a9358e476a0b6e7cff591e7f45c3"}],"aodh/tests/unit/test_notifier.py":[{"author":{"_account_id":6484,"name":"Feilong Wang","email":"hustemb@gmail.com","username":"flwang"},"change_message_id":"4bab6b90ecb9a040b55b37105619f980386ba1f4","unresolved":false,"context_lines":[{"line_number":399,"context_line":"        self._msg_notifier.sample({}, \u0027alarm.update\u0027,"},{"line_number":400,"context_line":"                                  self._notification(action))"},{"line_number":401,"context_line":"        time.sleep(1)"},{"line_number":402,"context_line":"        self.assertEqual(self.zaqar,"},{"line_number":403,"context_line":"                         self.service.notifiers[\u0027zaqar\u0027].obj.client)"},{"line_number":404,"context_line":""},{"line_number":405,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3aaa91ec_484d8098","line":402,"updated":"2016-06-29 04:19:13.000000000","message":"I think the unit test is not correct. Need some suggestion here. Thanks in advance.","commit_id":"e19ff9378eaf8012ed09404e7f8d6cc6135c56ca"},{"author":{"_account_id":6924,"name":"Pradeep Kilambi","email":"pkilambi@redhat.com","username":"pkilambi"},"change_message_id":"647ec4c8b77ada99f010eaf70e182030e1d7d3b1","unresolved":false,"context_lines":[{"line_number":399,"context_line":"        self._msg_notifier.sample({}, \u0027alarm.update\u0027,"},{"line_number":400,"context_line":"                                  self._notification(action))"},{"line_number":401,"context_line":"        time.sleep(1)"},{"line_number":402,"context_line":"        self.assertEqual(self.zaqar,"},{"line_number":403,"context_line":"                         self.service.notifiers[\u0027zaqar\u0027].obj.client)"},{"line_number":404,"context_line":""},{"line_number":405,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3aaa91ec_1df15402","line":402,"in_reply_to":"3aaa91ec_484d8098","updated":"2016-07-01 17:33:41.000000000","message":"Not sure i understand your question, I would first make sure get_presigned_client is mocked and tested here.","commit_id":"e19ff9378eaf8012ed09404e7f8d6cc6135c56ca"}],"requirements.txt":[{"author":{"_account_id":6537,"name":"gordon chung","email":"gord@live.ca","username":"chungg"},"change_message_id":"f2b1e9e277a97e38798281bc6b2d4276ec50b789","unresolved":false,"context_lines":[{"line_number":25,"context_line":"oslo.utils\u003e\u003d3.5.0 # Apache-2.0"},{"line_number":26,"context_line":"python-ceilometerclient\u003e\u003d1.5.0"},{"line_number":27,"context_line":"python-keystoneclient\u003e\u003d1.6.0"},{"line_number":28,"context_line":"python-zaqarclient\u003e\u003d1.2.0"},{"line_number":29,"context_line":"pytz\u003e\u003d2013.6"},{"line_number":30,"context_line":"requests\u003e\u003d2.5.2"},{"line_number":31,"context_line":"six\u003e\u003d1.9.0"}],"source_content_type":"text/plain","patch_set":8,"id":"7a8ec9b2_dff814c6","line":28,"updated":"2016-09-15 20:25:29.000000000","message":"this should be a test-requirement?","commit_id":"7c288bfe6c7bfb982a205cab357be373cf6744b9"}]}