)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6577,"name":"Sergey Kraynev","email":"sergejyit@gmail.com","username":"skraynev"},"change_message_id":"57cab1a436682202b4e3e73fc0fefb2a862f0559","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     cbjchen@cn.ibm.com \u003ccbjchen@cn.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2013-12-17 16:24:12 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Enalbe parallel while deleting neutron resources"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The confirm delete task was run immediately after creation in"},{"line_number":10,"context_line":"handle_delete, which blocked the whole DependencyTaskGroup."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"AAAATn%2F9C2s%3D","line":7,"updated":"2013-12-17 13:53:36.000000000","message":"s/Enalbe/Enable","commit_id":"e0caa68279d5e7759504b4074aa4cf1ac71f390e"},{"author":{"_account_id":7135,"name":"Liang Chen","email":"liangchen.linux@gmail.com","username":"Liang"},"change_message_id":"4b950b62b75c8680aefc453f6216597f7a7fe6b7","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     cbjchen@cn.ibm.com \u003ccbjchen@cn.ibm.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2013-12-17 16:24:12 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Enalbe parallel while deleting neutron resources"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The confirm delete task was run immediately after creation in"},{"line_number":10,"context_line":"handle_delete, which blocked the whole DependencyTaskGroup."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"AAAATn%2F9B9k%3D","line":7,"in_reply_to":"AAAATn%2F9C2s%3D","updated":"2013-12-17 14:19:04.000000000","message":"Done","commit_id":"e0caa68279d5e7759504b4074aa4cf1ac71f390e"}],"heat/engine/resources/neutron/firewall.py":[{"author":{"_account_id":6917,"name":"Bartosz Górski","email":"bartosz@codilime.com","username":"bartosz-gorski"},"change_message_id":"a5f0d95fbb3ea8a7f9f3d19ac002f1059cce54a4","unresolved":false,"context_lines":[{"line_number":82,"context_line":"            return delete_task"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    def check_delete_complete(self, delete_task):"},{"line_number":85,"context_line":"        # if the resource was already deleted, delete_task will be None"},{"line_number":86,"context_line":"        if delete_task is None:"},{"line_number":87,"context_line":"            return True"},{"line_number":88,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F8%2BOE%3D","line":85,"updated":"2013-12-18 09:03:38.000000000","message":"This code is repeating in many neutron resources. We should create private helper function in NeutronResource class and just execute it in appropriate resources","commit_id":"5819b7318b7ef787110c91e5d8564a59129c37af"},{"author":{"_account_id":7135,"name":"Liang Chen","email":"liangchen.linux@gmail.com","username":"Liang"},"change_message_id":"b90e70367c90122523392c10204a58c3d0bf233b","unresolved":false,"context_lines":[{"line_number":82,"context_line":"            return delete_task"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    def check_delete_complete(self, delete_task):"},{"line_number":85,"context_line":"        # if the resource was already deleted, delete_task will be None"},{"line_number":86,"context_line":"        if delete_task is None:"},{"line_number":87,"context_line":"            return True"},{"line_number":88,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F837k%3D","line":85,"in_reply_to":"AAAATn%2F8%2BOE%3D","updated":"2013-12-19 10:01:56.000000000","message":"There will be only one line left in next patch ...","commit_id":"5819b7318b7ef787110c91e5d8564a59129c37af"},{"author":{"_account_id":6917,"name":"Bartosz Górski","email":"bartosz@codilime.com","username":"bartosz-gorski"},"change_message_id":"a5f0d95fbb3ea8a7f9f3d19ac002f1059cce54a4","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        # if the resource was already deleted, delete_task will be None"},{"line_number":86,"context_line":"        if delete_task is None:"},{"line_number":87,"context_line":"            return True"},{"line_number":88,"context_line":"        else:"},{"line_number":89,"context_line":"            return delete_task.step()"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F8%2BNg%3D","line":88,"updated":"2013-12-18 09:03:38.000000000","message":"I think you can omit else and leave only return delete_task.step()","commit_id":"5819b7318b7ef787110c91e5d8564a59129c37af"},{"author":{"_account_id":7135,"name":"Liang Chen","email":"liangchen.linux@gmail.com","username":"Liang"},"change_message_id":"b90e70367c90122523392c10204a58c3d0bf233b","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        # if the resource was already deleted, delete_task will be None"},{"line_number":86,"context_line":"        if delete_task is None:"},{"line_number":87,"context_line":"            return True"},{"line_number":88,"context_line":"        else:"},{"line_number":89,"context_line":"            return delete_task.step()"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F8370%3D","line":88,"in_reply_to":"AAAATn%2F8%2BNg%3D","updated":"2013-12-19 10:01:56.000000000","message":"Agree. This can be simplified.","commit_id":"5819b7318b7ef787110c91e5d8564a59129c37af"},{"author":{"_account_id":6917,"name":"Bartosz Górski","email":"bartosz@codilime.com","username":"bartosz-gorski"},"change_message_id":"a5f0d95fbb3ea8a7f9f3d19ac002f1059cce54a4","unresolved":false,"context_lines":[{"line_number":136,"context_line":"            self.neutron().update_firewall_policy("},{"line_number":137,"context_line":"                self.resource_id, {\u0027firewall_policy\u0027: prop_diff})"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"    def handle_delete(self):"},{"line_number":140,"context_line":"        client \u003d self.neutron()"},{"line_number":141,"context_line":"        try:"},{"line_number":142,"context_line":"            client.delete_firewall_policy(self.resource_id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F8%2BLU%3D","line":139,"updated":"2013-12-18 09:03:38.000000000","message":"I think we should also refactor handle_delete functions in neutron resources. Create helper function in NeutronResources which will take function to execute (client call).","commit_id":"5819b7318b7ef787110c91e5d8564a59129c37af"},{"author":{"_account_id":7135,"name":"Liang Chen","email":"liangchen.linux@gmail.com","username":"Liang"},"change_message_id":"b90e70367c90122523392c10204a58c3d0bf233b","unresolved":false,"context_lines":[{"line_number":136,"context_line":"            self.neutron().update_firewall_policy("},{"line_number":137,"context_line":"                self.resource_id, {\u0027firewall_policy\u0027: prop_diff})"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"    def handle_delete(self):"},{"line_number":140,"context_line":"        client \u003d self.neutron()"},{"line_number":141,"context_line":"        try:"},{"line_number":142,"context_line":"            client.delete_firewall_policy(self.resource_id)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAATn%2F837U%3D","line":139,"in_reply_to":"AAAATn%2F8%2BLU%3D","updated":"2013-12-19 10:01:56.000000000","message":"Sounds good. I will give a try for that in a separate patch.","commit_id":"5819b7318b7ef787110c91e5d8564a59129c37af"},{"author":{"_account_id":4328,"name":"Steven Hardy","email":"steven.hardy@suse.com","username":"shardy"},"change_message_id":"b5913b86e9027c65d126a0f7f68998e2ecbc1b65","unresolved":false,"context_lines":[{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    def check_delete_complete(self, delete_task):"},{"line_number":85,"context_line":"        # if the resource was already deleted, delete_task will be None"},{"line_number":86,"context_line":"        return delete_task is None or delete_task.step()"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"class FirewallPolicy(neutron.NeutronResource):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F83tg%3D","line":86,"updated":"2013-12-19 11:08:51.000000000","message":"These two bits of code are repeated in every resource, so can we refactor so it goes into the NeutronResource base class?  e.g have a common check_delete_complete, which could be overridden if needed (e.g in the Pool resource), and have the delete task do:\n\n  else:\n    return self._delete_task()\n\nor something?\n\nThis would also fit the existing pattern where _confirm_delete is defined in the base class.","commit_id":"456ee1eaa1fb3a909ba1fba9e490444489da83cc"},{"author":{"_account_id":7135,"name":"Liang Chen","email":"liangchen.linux@gmail.com","username":"Liang"},"change_message_id":"dbe0ebb9032791a9f7705dcea8f8ebd1e9e1a65f","unresolved":false,"context_lines":[{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    def check_delete_complete(self, delete_task):"},{"line_number":85,"context_line":"        # if the resource was already deleted, delete_task will be None"},{"line_number":86,"context_line":"        return delete_task is None or delete_task.step()"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"class FirewallPolicy(neutron.NeutronResource):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F82%2BU%3D","line":86,"in_reply_to":"AAAATn%2F83tg%3D","updated":"2013-12-19 14:08:31.000000000","message":"Thanks for the detailed suggestion. It will be implemented in the next patch.\n\nI wasn\u0027t feeling so safe to supply a check_delete_complete in the base class. But that can reduce lots of duplicate code, and giving a None delete_task will not harm.","commit_id":"456ee1eaa1fb3a909ba1fba9e490444489da83cc"}]}