)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":2243,"name":"John Griffith","email":"john.griffith8@gmail.com","username":"john-griffith"},"change_message_id":"f6d1aa1b91d4c4d462a7f549de609fa70b3ab5de","unresolved":false,"context_lines":[{"line_number":10,"context_line":"have added the workers table to keep track of which node is working with"},{"line_number":11,"context_line":"each specific resource."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This patch makes c-vol service work with this new table by adding"},{"line_number":14,"context_line":"entries on cleanable operations and removing them once these operations"},{"line_number":15,"context_line":"have completed."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Service cleanup on initialization has also been changed to use this new"},{"line_number":18,"context_line":"table so nodes will cleanup only resources from operations they left on"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"5a18252c_f2706aa6","line":15,"range":{"start_line":13,"start_character":0,"end_line":15,"end_character":15},"updated":"2016-04-14 14:08:26.000000000","message":"So after all of the churn and thousands of lines of patches the last 6 months; we just come back to a db table entry?  We could\u0027ve just added the table and been done.","commit_id":"c31be18629e9327c749214497811b901bdd60fbf"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e9d186e0f7ee68f42f42abd9891029299d737734","unresolved":false,"context_lines":[{"line_number":10,"context_line":"have added the workers table to keep track of which node is working with"},{"line_number":11,"context_line":"each specific resource."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This patch makes c-vol service work with this new table by adding"},{"line_number":14,"context_line":"entries on cleanable operations and removing them once these operations"},{"line_number":15,"context_line":"have completed."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Service cleanup on initialization has also been changed to use this new"},{"line_number":18,"context_line":"table so nodes will cleanup only resources from operations they left on"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"5a18252c_c38e3e2d","line":15,"range":{"start_line":13,"start_character":0,"end_line":15,"end_character":15},"in_reply_to":"5a18252c_f2706aa6","updated":"2016-04-14 14:23:19.000000000","message":"This was always the plan as discussed in the summit [1] and as explained in the specs [2], so it shouldn\u0027t come as a surprise.  :-)\n\n [1]: https://etherpad.openstack.org/p/mitaka-cinder-cvol-aa\n [2]: https://review.openstack.org/236977","commit_id":"c31be18629e9327c749214497811b901bdd60fbf"}],"cinder/coordination.py":[{"author":{"_account_id":7173,"name":"Scott DAngelo","email":"scott.dangelo@gmail.com","username":"scottda"},"change_message_id":"1f6cf5e22d6aa42f30ea5e17521eee231b82ede6","unresolved":false,"context_lines":[{"line_number":245,"context_line":"            the value is used as a timeout value and if lock is not acquired"},{"line_number":246,"context_line":"            after this number of seconds exception is raised."},{"line_number":247,"context_line":"    :param coordinator: Coordinator class to use when creating lock."},{"line_number":248,"context_line":"        Defaults to the global coordinator."},{"line_number":249,"context_line":"    :raises tooz.coordination.LockAcquireFailed: if lock is not acquired"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"    Decorating a method like so::"}],"source_content_type":"text/x-python","patch_set":20,"id":"dada55a8_a519037c","line":248,"updated":"2016-07-22 16:05:49.000000000","message":"param for set_workers?","commit_id":"9eb09904b906a292e18ba90925b51fc4876113de"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"3a799c3458a328d08283bb16b6d9b20f8d25e06d","unresolved":false,"context_lines":[{"line_number":245,"context_line":"            the value is used as a timeout value and if lock is not acquired"},{"line_number":246,"context_line":"            after this number of seconds exception is raised."},{"line_number":247,"context_line":"    :param coordinator: Coordinator class to use when creating lock."},{"line_number":248,"context_line":"        Defaults to the global coordinator."},{"line_number":249,"context_line":"    :raises tooz.coordination.LockAcquireFailed: if lock is not acquired"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"    Decorating a method like so::"}],"source_content_type":"text/x-python","patch_set":20,"id":"dada55a8_c83ecb28","line":248,"in_reply_to":"dada55a8_a519037c","updated":"2016-07-22 16:44:52.000000000","message":"Done","commit_id":"9eb09904b906a292e18ba90925b51fc4876113de"},{"author":{"_account_id":7173,"name":"Scott DAngelo","email":"scott.dangelo@gmail.com","username":"scottda"},"change_message_id":"81a91d208b92c0dff87f4b8f598503e89e6de2ec","unresolved":false,"context_lines":[{"line_number":288,"context_line":"            with lock(blocking):"},{"line_number":289,"context_line":"                if set_workers:"},{"line_number":290,"context_line":"                    # NOTE(geguileo): We could have used any other cleanable"},{"line_number":291,"context_line":"                    # object that doesn\u0027t overwrite defaul set_workers"},{"line_number":292,"context_line":"                    # behavior.  We cannot import cleanable directly because"},{"line_number":293,"context_line":"                    # then we would have a circular import"},{"line_number":294,"context_line":"                    func \u003d objects.Volume.set_workers(f)"}],"source_content_type":"text/x-python","patch_set":24,"id":"9ad45d7e_d7242d61","line":291,"updated":"2016-08-10 20:15:30.000000000","message":"defaul -\u003e default","commit_id":"83dee033053269511d7158e4dbaefdd9fea54b88"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4ffd5939244b0a0e78e10af782cc0fd33b7c42ff","unresolved":false,"context_lines":[{"line_number":288,"context_line":"            with lock(blocking):"},{"line_number":289,"context_line":"                if set_workers:"},{"line_number":290,"context_line":"                    # NOTE(geguileo): We could have used any other cleanable"},{"line_number":291,"context_line":"                    # object that doesn\u0027t overwrite defaul set_workers"},{"line_number":292,"context_line":"                    # behavior.  We cannot import cleanable directly because"},{"line_number":293,"context_line":"                    # then we would have a circular import"},{"line_number":294,"context_line":"                    func \u003d objects.Volume.set_workers(f)"}],"source_content_type":"text/x-python","patch_set":24,"id":"9ad45d7e_2471fda8","line":291,"in_reply_to":"9ad45d7e_d7242d61","updated":"2016-08-11 08:38:48.000000000","message":"Done","commit_id":"83dee033053269511d7158e4dbaefdd9fea54b88"}],"cinder/manager.py":[{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"5ad3e0c4fc0dee3b74644200a5947f43dc50d225","unresolved":false,"context_lines":[{"line_number":251,"context_line":"                        # of it"},{"line_number":252,"context_line":"                        keep_entry \u003d self._do_cleanup(context, vo)"},{"line_number":253,"context_line":"                    except Exception:"},{"line_number":254,"context_line":"                        LOG.exception(_LE(\u0027Could not perform cleanup\u0027))"},{"line_number":255,"context_line":"                        # Return the work to the origin service"},{"line_number":256,"context_line":"                        db.worker_update(context, clean.id,"},{"line_number":257,"context_line":"                                         service_id\u003doriginal_service_id,"}],"source_content_type":"text/x-python","patch_set":20,"id":"dada55a8_e592db4a","line":254,"range":{"start_line":254,"start_character":44,"end_line":254,"end_character":68},"updated":"2016-07-22 16:15:59.000000000","message":"Would be nice to have consistency in the messages here.  Some have \u0027.\u0027 at the end, some don\u0027t.  Would like them to all have \u0027.\u0027 if possible.","commit_id":"9eb09904b906a292e18ba90925b51fc4876113de"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"3a799c3458a328d08283bb16b6d9b20f8d25e06d","unresolved":false,"context_lines":[{"line_number":251,"context_line":"                        # of it"},{"line_number":252,"context_line":"                        keep_entry \u003d self._do_cleanup(context, vo)"},{"line_number":253,"context_line":"                    except Exception:"},{"line_number":254,"context_line":"                        LOG.exception(_LE(\u0027Could not perform cleanup\u0027))"},{"line_number":255,"context_line":"                        # Return the work to the origin service"},{"line_number":256,"context_line":"                        db.worker_update(context, clean.id,"},{"line_number":257,"context_line":"                                         service_id\u003doriginal_service_id,"}],"source_content_type":"text/x-python","patch_set":20,"id":"dada55a8_0820e3f6","line":254,"range":{"start_line":254,"start_character":44,"end_line":254,"end_character":68},"in_reply_to":"dada55a8_e592db4a","updated":"2016-07-22 16:44:52.000000000","message":"Done","commit_id":"9eb09904b906a292e18ba90925b51fc4876113de"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"5ad3e0c4fc0dee3b74644200a5947f43dc50d225","unresolved":false,"context_lines":[{"line_number":263,"context_line":"            # method doesn\u0027t want to keep the entry (for example for delayed"},{"line_number":264,"context_line":"            # deletion)."},{"line_number":265,"context_line":"            if not keep_entry and not db.worker_destroy(context, id\u003dclean.id):"},{"line_number":266,"context_line":"                LOG.error(_LE(\u0027Could not remove worker entry %s\u0027), clean.id)"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"        LOG.info(_LI(\u0027Service %s cleanup completed.\u0027),"},{"line_number":269,"context_line":"                 cleanup_request.service_id)"}],"source_content_type":"text/x-python","patch_set":20,"id":"dada55a8_8594bf44","line":266,"range":{"start_line":266,"start_character":16,"end_line":266,"end_character":25},"updated":"2016-07-22 16:15:59.000000000","message":"Is this really an error?  Seems like warning is more appropriate given that on line 252 you say that you could have keep_entry because the clean-up is asynchronous.","commit_id":"9eb09904b906a292e18ba90925b51fc4876113de"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"3a799c3458a328d08283bb16b6d9b20f8d25e06d","unresolved":false,"context_lines":[{"line_number":263,"context_line":"            # method doesn\u0027t want to keep the entry (for example for delayed"},{"line_number":264,"context_line":"            # deletion)."},{"line_number":265,"context_line":"            if not keep_entry and not db.worker_destroy(context, id\u003dclean.id):"},{"line_number":266,"context_line":"                LOG.error(_LE(\u0027Could not remove worker entry %s\u0027), clean.id)"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"        LOG.info(_LI(\u0027Service %s cleanup completed.\u0027),"},{"line_number":269,"context_line":"                 cleanup_request.service_id)"}],"source_content_type":"text/x-python","patch_set":20,"id":"dada55a8_287de7e7","line":266,"range":{"start_line":266,"start_character":16,"end_line":266,"end_character":25},"in_reply_to":"dada55a8_8594bf44","updated":"2016-07-22 16:44:52.000000000","message":"I agree, this should be a warning.","commit_id":"9eb09904b906a292e18ba90925b51fc4876113de"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"e197b49396b33e348d4a752c7ab1f26557461e7f","unresolved":false,"context_lines":[{"line_number":263,"context_line":"            # method doesn\u0027t want to keep the entry (for example for delayed"},{"line_number":264,"context_line":"            # deletion)."},{"line_number":265,"context_line":"            if not keep_entry and not db.worker_destroy(context, id\u003dclean.id):"},{"line_number":266,"context_line":"                LOG.error(_LW(\u0027Could not remove worker entry %s.\u0027), clean.id)"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"        LOG.info(_LI(\u0027Service %s cleanup completed.\u0027),"},{"line_number":269,"context_line":"                 cleanup_request.service_id)"}],"source_content_type":"text/x-python","patch_set":21,"id":"dada55a8_194957d1","line":266,"range":{"start_line":266,"start_character":16,"end_line":266,"end_character":25},"updated":"2016-07-22 17:39:32.000000000","message":"Didn\u0027t actually change the LOG.error to LOG.warning.","commit_id":"ceff784e039b6be3786c238bb0a2952b16d69b02"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c0dc577133d9a1561b4ba2630fea425854dd144b","unresolved":false,"context_lines":[{"line_number":263,"context_line":"            # method doesn\u0027t want to keep the entry (for example for delayed"},{"line_number":264,"context_line":"            # deletion)."},{"line_number":265,"context_line":"            if not keep_entry and not db.worker_destroy(context, id\u003dclean.id):"},{"line_number":266,"context_line":"                LOG.error(_LW(\u0027Could not remove worker entry %s.\u0027), clean.id)"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"        LOG.info(_LI(\u0027Service %s cleanup completed.\u0027),"},{"line_number":269,"context_line":"                 cleanup_request.service_id)"}],"source_content_type":"text/x-python","patch_set":21,"id":"bacf61ea_b5b18ae2","line":266,"range":{"start_line":266,"start_character":16,"end_line":266,"end_character":25},"in_reply_to":"dada55a8_194957d1","updated":"2016-08-05 15:45:09.000000000","message":"LOL, what whas I thinking!","commit_id":"ceff784e039b6be3786c238bb0a2952b16d69b02"},{"author":{"_account_id":7173,"name":"Scott DAngelo","email":"scott.dangelo@gmail.com","username":"scottda"},"change_message_id":"81a91d208b92c0dff87f4b8f598503e89e6de2ec","unresolved":false,"context_lines":[{"line_number":188,"context_line":"        LOG.info(_LI(\u0027Initiating service %s cleanup\u0027),"},{"line_number":189,"context_line":"                 cleanup_request.service_id)"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        # If we have not received until when we want to do the cleanup we"},{"line_number":192,"context_line":"        # assume it\u0027s until just now"},{"line_number":193,"context_line":"        until \u003d cleanup_request.until or timeutils.utcnow()"},{"line_number":194,"context_line":"        keep_entry \u003d False"}],"source_content_type":"text/x-python","patch_set":24,"id":"9ad45d7e_57dafd17","line":191,"updated":"2016-08-10 20:15:30.000000000","message":"This confused me...\nMaybe clearer if something like:\n# If we have not received \u0027until\u0027 when we want to do the cleanup we\n# assume it\u0027s \u0027now\u0027\n\nOr maybe I still don\u0027t understand it...","commit_id":"83dee033053269511d7158e4dbaefdd9fea54b88"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4ffd5939244b0a0e78e10af782cc0fd33b7c42ff","unresolved":false,"context_lines":[{"line_number":188,"context_line":"        LOG.info(_LI(\u0027Initiating service %s cleanup\u0027),"},{"line_number":189,"context_line":"                 cleanup_request.service_id)"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        # If we have not received until when we want to do the cleanup we"},{"line_number":192,"context_line":"        # assume it\u0027s until just now"},{"line_number":193,"context_line":"        until \u003d cleanup_request.until or timeutils.utcnow()"},{"line_number":194,"context_line":"        keep_entry \u003d False"}],"source_content_type":"text/x-python","patch_set":24,"id":"9ad45d7e_64ae55e7","line":191,"in_reply_to":"9ad45d7e_57dafd17","updated":"2016-08-11 08:38:48.000000000","message":"I\u0027ll try to rephrase it in a clearer way.","commit_id":"83dee033053269511d7158e4dbaefdd9fea54b88"},{"author":{"_account_id":7173,"name":"Scott DAngelo","email":"scott.dangelo@gmail.com","username":"scottda"},"change_message_id":"81a91d208b92c0dff87f4b8f598503e89e6de2ec","unresolved":false,"context_lines":[{"line_number":252,"context_line":"                        keep_entry \u003d self._do_cleanup(context, vo)"},{"line_number":253,"context_line":"                    except Exception:"},{"line_number":254,"context_line":"                        LOG.exception(_LE(\u0027Could not perform cleanup.\u0027))"},{"line_number":255,"context_line":"                        # Return the work to the origin service"},{"line_number":256,"context_line":"                        db.worker_update(context, clean.id,"},{"line_number":257,"context_line":"                                         service_id\u003doriginal_service_id,"},{"line_number":258,"context_line":"                                         updated_at\u003doriginal_time)"}],"source_content_type":"text/x-python","patch_set":24,"id":"9ad45d7e_971475a0","line":255,"updated":"2016-08-10 20:15:30.000000000","message":"work -\u003e worker\norigin -\u003e original","commit_id":"83dee033053269511d7158e4dbaefdd9fea54b88"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4ffd5939244b0a0e78e10af782cc0fd33b7c42ff","unresolved":false,"context_lines":[{"line_number":252,"context_line":"                        keep_entry \u003d self._do_cleanup(context, vo)"},{"line_number":253,"context_line":"                    except Exception:"},{"line_number":254,"context_line":"                        LOG.exception(_LE(\u0027Could not perform cleanup.\u0027))"},{"line_number":255,"context_line":"                        # Return the work to the origin service"},{"line_number":256,"context_line":"                        db.worker_update(context, clean.id,"},{"line_number":257,"context_line":"                                         service_id\u003doriginal_service_id,"},{"line_number":258,"context_line":"                                         updated_at\u003doriginal_time)"}],"source_content_type":"text/x-python","patch_set":24,"id":"9ad45d7e_04d5d91a","line":255,"in_reply_to":"9ad45d7e_971475a0","updated":"2016-08-11 08:38:48.000000000","message":"Done","commit_id":"83dee033053269511d7158e4dbaefdd9fea54b88"}],"cinder/objects/volume.py":[{"author":{"_account_id":7173,"name":"Scott DAngelo","email":"scott.dangelo@gmail.com","username":"scottda"},"change_message_id":"81a91d208b92c0dff87f4b8f598503e89e6de2ec","unresolved":false,"context_lines":[{"line_number":51,"context_line":"@base.CinderObjectRegistry.register"},{"line_number":52,"context_line":"class Volume(cleanable.CinderCleanableObject, base.CinderObject,"},{"line_number":53,"context_line":"             base.CinderObjectDictCompat, base.CinderComparableObject,"},{"line_number":54,"context_line":"             base.ClusteredObject, ):"},{"line_number":55,"context_line":"    # Version 1.0: Initial version"},{"line_number":56,"context_line":"    # Version 1.1: Added metadata, admin_metadata, volume_attachment, and"},{"line_number":57,"context_line":"    #              volume_type"}],"source_content_type":"text/x-python","patch_set":24,"id":"9ad45d7e_1324185e","line":54,"updated":"2016-08-10 20:15:30.000000000","message":"extra comma \u0027,\u0027","commit_id":"83dee033053269511d7158e4dbaefdd9fea54b88"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4ffd5939244b0a0e78e10af782cc0fd33b7c42ff","unresolved":false,"context_lines":[{"line_number":51,"context_line":"@base.CinderObjectRegistry.register"},{"line_number":52,"context_line":"class Volume(cleanable.CinderCleanableObject, base.CinderObject,"},{"line_number":53,"context_line":"             base.CinderObjectDictCompat, base.CinderComparableObject,"},{"line_number":54,"context_line":"             base.ClusteredObject, ):"},{"line_number":55,"context_line":"    # Version 1.0: Initial version"},{"line_number":56,"context_line":"    # Version 1.1: Added metadata, admin_metadata, volume_attachment, and"},{"line_number":57,"context_line":"    #              volume_type"}],"source_content_type":"text/x-python","patch_set":24,"id":"9ad45d7e_04a319b3","line":54,"in_reply_to":"9ad45d7e_1324185e","updated":"2016-08-11 08:38:48.000000000","message":"Done","commit_id":"83dee033053269511d7158e4dbaefdd9fea54b88"}],"cinder/test.py":[{"author":{"_account_id":7173,"name":"Scott DAngelo","email":"scott.dangelo@gmail.com","username":"scottda"},"change_message_id":"b8e6e4c8b4f36b0b31aa30aa723211e3efc0b90d","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        notifier.notifications \u003d self.notifier.notifications"},{"line_number":96,"context_line":"        return notifier"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def setUp(self, mock_worker\u003dTrue):"},{"line_number":99,"context_line":"        \"\"\"Run before each test method to initialize test environment.\"\"\""},{"line_number":100,"context_line":"        super(TestCase, self).setUp()"},{"line_number":101,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"1a95cdbc_2f3f9126","line":98,"updated":"2016-10-04 15:02:53.000000000","message":"pylint doesn\u0027t like this:\ncinder/test.py:98: [E1002, TestCase.setUp] Use of super on an old style class","commit_id":"33a0a9b7e5494e921d0aa094d670b129ebb05200"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"7ffe23287f599c1f706dba00735c4c0cfc8cc23f","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        notifier.notifications \u003d self.notifier.notifications"},{"line_number":96,"context_line":"        return notifier"},{"line_number":97,"context_line":""},{"line_number":98,"context_line":"    def setUp(self, mock_worker\u003dTrue):"},{"line_number":99,"context_line":"        \"\"\"Run before each test method to initialize test environment.\"\"\""},{"line_number":100,"context_line":"        super(TestCase, self).setUp()"},{"line_number":101,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"1a95cdbc_ba8a1462","line":98,"in_reply_to":"1a95cdbc_2f3f9126","updated":"2016-10-10 14:57:27.000000000","message":"Running it locally I don\u0027t seem to have a problem though.","commit_id":"33a0a9b7e5494e921d0aa094d670b129ebb05200"}],"cinder/tests/unit/test_volume.py":[{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"3fa5d0ea7ec8b9c6138dad1aedab5db0eb4d14b7","unresolved":false,"context_lines":[{"line_number":4509,"context_line":"                              snap)"},{"line_number":4510,"context_line":""},{"line_number":4511,"context_line":""},{"line_number":4512,"context_line":"class VolumeCleanupTestCase(BaseVolumeTestCase):"},{"line_number":4513,"context_line":""},{"line_number":4514,"context_line":"    def setUp(self):"},{"line_number":4515,"context_line":"        super(VolumeCleanupTestCase, self).setUp(mock_worker\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":22,"id":"dada55a8_4776383f","line":4512,"updated":"2016-07-23 03:26:04.000000000","message":"Should this class be moved to a separate test file called test_volume_cleanup.py?  We discussed at the meetup that this test_volume.py is getting too big and it\u0027s better to split tests out to different test files if possible.","commit_id":"329ad4a062e8921801e4419bcf123cad823373fb"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b6b9d587f8bdbbf7e9a974acc0bd3060d3146cc5","unresolved":false,"context_lines":[{"line_number":4509,"context_line":"                              snap)"},{"line_number":4510,"context_line":""},{"line_number":4511,"context_line":""},{"line_number":4512,"context_line":"class VolumeCleanupTestCase(BaseVolumeTestCase):"},{"line_number":4513,"context_line":""},{"line_number":4514,"context_line":"    def setUp(self):"},{"line_number":4515,"context_line":"        super(VolumeCleanupTestCase, self).setUp(mock_worker\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":22,"id":"bacf61ea_867b86f4","line":4512,"in_reply_to":"dada55a8_4776383f","updated":"2016-08-05 15:45:21.000000000","message":"Yes it\u0027s the perfect opportunity to take this code out of this file, thanks for pointing it out.","commit_id":"329ad4a062e8921801e4419bcf123cad823373fb"}],"cinder/volume/manager.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4ffd5939244b0a0e78e10af782cc0fd33b7c42ff","unresolved":false,"context_lines":[{"line_number":325,"context_line":""},{"line_number":326,"context_line":"    def _sync_provider_info(self, ctxt, volumes, snapshots):"},{"line_number":327,"context_line":"        # NOTE(jdg): For now this just updates provider_id, we can add more"},{"line_number":328,"context_line":"        # items to the update if they\u0027re releveant but we need to be safe in"},{"line_number":329,"context_line":"        # what we allow and add a list of allowed keys things that make sense"},{"line_number":330,"context_line":"        # are provider_*, replication_status etc"},{"line_number":331,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"9ad45d7e_e48ec52c","line":328,"range":{"start_line":328,"start_character":41,"end_line":328,"end_character":50},"updated":"2016-08-11 08:38:48.000000000","message":"relevant","commit_id":"83dee033053269511d7158e4dbaefdd9fea54b88"},{"author":{"_account_id":7173,"name":"Scott DAngelo","email":"scott.dangelo@gmail.com","username":"scottda"},"change_message_id":"81a91d208b92c0dff87f4b8f598503e89e6de2ec","unresolved":false,"context_lines":[{"line_number":326,"context_line":"    def _sync_provider_info(self, ctxt, volumes, snapshots):"},{"line_number":327,"context_line":"        # NOTE(jdg): For now this just updates provider_id, we can add more"},{"line_number":328,"context_line":"        # items to the update if they\u0027re releveant but we need to be safe in"},{"line_number":329,"context_line":"        # what we allow and add a list of allowed keys things that make sense"},{"line_number":330,"context_line":"        # are provider_*, replication_status etc"},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"        updates, snapshot_updates \u003d self.driver.update_provider_info("}],"source_content_type":"text/x-python","patch_set":24,"id":"9ad45d7e_d92d7f47","line":329,"updated":"2016-08-10 20:15:30.000000000","message":"of allowed keys. Things that make sense are...","commit_id":"83dee033053269511d7158e4dbaefdd9fea54b88"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4ffd5939244b0a0e78e10af782cc0fd33b7c42ff","unresolved":false,"context_lines":[{"line_number":326,"context_line":"    def _sync_provider_info(self, ctxt, volumes, snapshots):"},{"line_number":327,"context_line":"        # NOTE(jdg): For now this just updates provider_id, we can add more"},{"line_number":328,"context_line":"        # items to the update if they\u0027re releveant but we need to be safe in"},{"line_number":329,"context_line":"        # what we allow and add a list of allowed keys things that make sense"},{"line_number":330,"context_line":"        # are provider_*, replication_status etc"},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"        updates, snapshot_updates \u003d self.driver.update_provider_info("}],"source_content_type":"text/x-python","patch_set":24,"id":"9ad45d7e_24ea9dac","line":329,"in_reply_to":"9ad45d7e_d92d7f47","updated":"2016-08-11 08:38:48.000000000","message":"In my defense, all I did here was remove a duplicated \"add more\"  ;-)","commit_id":"83dee033053269511d7158e4dbaefdd9fea54b88"},{"author":{"_account_id":7173,"name":"Scott DAngelo","email":"scott.dangelo@gmail.com","username":"scottda"},"change_message_id":"81a91d208b92c0dff87f4b8f598503e89e6de2ec","unresolved":false,"context_lines":[{"line_number":623,"context_line":"        LOG.info(_LI(\"Created volume successfully.\"), resource\u003dvolume)"},{"line_number":624,"context_line":"        return volume.id"},{"line_number":625,"context_line":""},{"line_number":626,"context_line":"    # FIXME(bluex): replace volume_id with volume.id when volume_id is removed"},{"line_number":627,"context_line":"    @coordination.synchronized(\u0027{volume_id}-{f_name}\u0027, set_workers\u003dTrue)"},{"line_number":628,"context_line":"    def delete_volume(self, context, volume_id,"},{"line_number":629,"context_line":"                      unmanage_only\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":24,"id":"9ad45d7e_19aef780","line":626,"updated":"2016-08-10 20:15:30.000000000","message":"Can\u0027t we change to volume.id now?","commit_id":"83dee033053269511d7158e4dbaefdd9fea54b88"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"4ffd5939244b0a0e78e10af782cc0fd33b7c42ff","unresolved":false,"context_lines":[{"line_number":623,"context_line":"        LOG.info(_LI(\"Created volume successfully.\"), resource\u003dvolume)"},{"line_number":624,"context_line":"        return volume.id"},{"line_number":625,"context_line":""},{"line_number":626,"context_line":"    # FIXME(bluex): replace volume_id with volume.id when volume_id is removed"},{"line_number":627,"context_line":"    @coordination.synchronized(\u0027{volume_id}-{f_name}\u0027, set_workers\u003dTrue)"},{"line_number":628,"context_line":"    def delete_volume(self, context, volume_id,"},{"line_number":629,"context_line":"                      unmanage_only\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":24,"id":"9ad45d7e_240f3d89","line":626,"in_reply_to":"9ad45d7e_19aef780","updated":"2016-08-11 08:38:48.000000000","message":"In theory we can, but I\u0027d rather wait until we remove the volume_id from the rpc calls.","commit_id":"83dee033053269511d7158e4dbaefdd9fea54b88"},{"author":{"_account_id":12825,"name":"Szymon Wróblewski","email":"szymon.wroblewski@ovhcloud.com","username":"bluex"},"change_message_id":"ac591468e07f3d56f9ffc525bb3ebf704d93dc4d","unresolved":false,"context_lines":[{"line_number":387,"context_line":"                 {\u0027num_vols\u0027: num_vols, \u0027num_cgs\u0027: num_cgs,"},{"line_number":388,"context_line":"                  \u0027host\u0027: self.host, \u0027cluster\u0027: self.cluster})"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"    def init_host(self, added_to_cluster\u003dNone, **kwargs):"},{"line_number":391,"context_line":"        \"\"\"Perform any required initialization.\"\"\""},{"line_number":392,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":393,"context_line":"        if not self.driver.supported:"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a98d1f4_9e338ef5","line":390,"range":{"start_line":390,"start_character":24,"end_line":390,"end_character":55},"updated":"2016-10-03 23:48:02.000000000","message":"Should be service_id\u003dNone, added_to_cluster\u003dNone, see comment below","commit_id":"3879169b1f2a9636601b419354477ce98255f365"},{"author":{"_account_id":12825,"name":"Szymon Wróblewski","email":"szymon.wroblewski@ovhcloud.com","username":"bluex"},"change_message_id":"ac591468e07f3d56f9ffc525bb3ebf704d93dc4d","unresolved":false,"context_lines":[{"line_number":469,"context_line":"                           \u0027id\u0027: self.driver.__class__.__name__})"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":"        # Make sure to call CleanableManager to do the cleanup"},{"line_number":472,"context_line":"        super(VolumeManager, self).init_host(added_to_cluster\u003dadded_to_cluster,"},{"line_number":473,"context_line":"                                             **kwargs)"},{"line_number":474,"context_line":""},{"line_number":475,"context_line":"    def init_host_with_rpc(self):"},{"line_number":476,"context_line":"        LOG.info(_LI(\"Initializing RPC dependent components of volume \""}],"source_content_type":"text/x-python","patch_set":28,"id":"3a98d1f4_5eb2e64c","line":473,"range":{"start_line":472,"start_character":45,"end_line":473,"end_character":53},"updated":"2016-10-03 23:48:02.000000000","message":"What is the point in sending added_to_cluster parameter when CleanableManager.init_host doesn\u0027t care. However it does care about service_id parameter and will fail without it, so I think this method signature should be changed as mentioned in previous comment :)","commit_id":"3879169b1f2a9636601b419354477ce98255f365"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"3eb50e499d3a52c042ec78454c2fd7e7e9697794","unresolved":false,"context_lines":[{"line_number":469,"context_line":"                           \u0027id\u0027: self.driver.__class__.__name__})"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":"        # Make sure to call CleanableManager to do the cleanup"},{"line_number":472,"context_line":"        super(VolumeManager, self).init_host(added_to_cluster\u003dadded_to_cluster,"},{"line_number":473,"context_line":"                                             **kwargs)"},{"line_number":474,"context_line":""},{"line_number":475,"context_line":"    def init_host_with_rpc(self):"},{"line_number":476,"context_line":"        LOG.info(_LI(\"Initializing RPC dependent components of volume \""}],"source_content_type":"text/x-python","patch_set":28,"id":"3a98d1f4_8665f51e","line":473,"range":{"start_line":472,"start_character":45,"end_line":473,"end_character":53},"in_reply_to":"3a98d1f4_5eb2e64c","updated":"2016-10-04 13:13:19.000000000","message":"I don\u0027t understand what you mean... This will not fail.  The init_host method in this class doesn\u0027t care about service_id parameter, so it doesn\u0027t specify it in the signature, but since it accepts keyword arguments that passes along to the base class init_host method.\n\nIf the init_host method of the base class cares about service_id, and this is passed to this method, then it will be received byt the base class.\n\nThe idea is to simplify the signature and leave only things we care about.  If one wants to see the full signature all that\u0027s needed is to go to the method cinder.manager.Manager:init_host where all keyword arguments are defined.","commit_id":"3879169b1f2a9636601b419354477ce98255f365"},{"author":{"_account_id":12825,"name":"Szymon Wróblewski","email":"szymon.wroblewski@ovhcloud.com","username":"bluex"},"change_message_id":"4f8a4d801824526556f5ef44eb1b65ba0c4e18b8","unresolved":false,"context_lines":[{"line_number":469,"context_line":"                           \u0027id\u0027: self.driver.__class__.__name__})"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":"        # Make sure to call CleanableManager to do the cleanup"},{"line_number":472,"context_line":"        super(VolumeManager, self).init_host(added_to_cluster\u003dadded_to_cluster,"},{"line_number":473,"context_line":"                                             **kwargs)"},{"line_number":474,"context_line":""},{"line_number":475,"context_line":"    def init_host_with_rpc(self):"},{"line_number":476,"context_line":"        LOG.info(_LI(\"Initializing RPC dependent components of volume \""}],"source_content_type":"text/x-python","patch_set":28,"id":"fa6399be_1c3f1cd7","line":473,"range":{"start_line":472,"start_character":45,"end_line":473,"end_character":53},"in_reply_to":"3a98d1f4_8665f51e","updated":"2016-10-12 16:10:39.000000000","message":"What I was trying to say is that this method could be called without any parameters - VolumeManager.init_host() (which is valid syntax according to signature) and will raise exception because CleanableManager.init_host expects service_id parameter. Even cinder.manager.Manager:init_host defines service_id as optional so it\u0027s not clear that in this case it\u0027s required parameter.\n\nOf course it\u0027s really minor, as init_host is always called with service_id parameter, but I though it would be better to avoid potential error.","commit_id":"3879169b1f2a9636601b419354477ce98255f365"},{"author":{"_account_id":12825,"name":"Szymon Wróblewski","email":"szymon.wroblewski@ovhcloud.com","username":"bluex"},"change_message_id":"ac591468e07f3d56f9ffc525bb3ebf704d93dc4d","unresolved":false,"context_lines":[{"line_number":521,"context_line":"            vo_resource.save()"},{"line_number":522,"context_line":""},{"line_number":523,"context_line":"        if isinstance(vo_resource, objects.Volume):"},{"line_number":524,"context_line":"            if vo_resource.status \u003d\u003d \u0027downloading\u0027:"},{"line_number":525,"context_line":"                self.driver.clear_download(ctxt, vo_resource)"},{"line_number":526,"context_line":""},{"line_number":527,"context_line":"            elif vo_resource.status \u003d\u003d \u0027uploading\u0027:"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a98d1f4_c348e1b5","line":524,"range":{"start_line":524,"start_character":12,"end_line":524,"end_character":51},"updated":"2016-10-03 23:48:02.000000000","message":"I may be missing something, but it seems that this case will never occur, because in line 520 you change status to \u0027error\u0027 if it was \u0027downloading\u0027.","commit_id":"3879169b1f2a9636601b419354477ce98255f365"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"3eb50e499d3a52c042ec78454c2fd7e7e9697794","unresolved":false,"context_lines":[{"line_number":521,"context_line":"            vo_resource.save()"},{"line_number":522,"context_line":""},{"line_number":523,"context_line":"        if isinstance(vo_resource, objects.Volume):"},{"line_number":524,"context_line":"            if vo_resource.status \u003d\u003d \u0027downloading\u0027:"},{"line_number":525,"context_line":"                self.driver.clear_download(ctxt, vo_resource)"},{"line_number":526,"context_line":""},{"line_number":527,"context_line":"            elif vo_resource.status \u003d\u003d \u0027uploading\u0027:"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a98d1f4_3c0fd12f","line":524,"range":{"start_line":524,"start_character":12,"end_line":524,"end_character":51},"in_reply_to":"3a98d1f4_c348e1b5","updated":"2016-10-04 13:13:19.000000000","message":"You are totally right!!  It may not a big deal since no driver implements that method, but it\u0027s definitely wrong, so I\u0027ll fix this and update tests to catch it.","commit_id":"3879169b1f2a9636601b419354477ce98255f365"},{"author":{"_account_id":12825,"name":"Szymon Wróblewski","email":"szymon.wroblewski@ovhcloud.com","username":"bluex"},"change_message_id":"ac591468e07f3d56f9ffc525bb3ebf704d93dc4d","unresolved":false,"context_lines":[{"line_number":659,"context_line":"        return volume.id"},{"line_number":660,"context_line":""},{"line_number":661,"context_line":"    # FIXME(bluex): replace volume_id with volume.id when volume_id is removed"},{"line_number":662,"context_line":"    @coordination.synchronized(\u0027{volume_id}-{f_name}\u0027, set_workers\u003dTrue)"},{"line_number":663,"context_line":"    def delete_volume(self, context, volume_id,"},{"line_number":664,"context_line":"                      unmanage_only\u003dFalse,"},{"line_number":665,"context_line":"                      volume\u003dNone,"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a98d1f4_83db397b","line":662,"range":{"start_line":662,"start_character":53,"end_line":662,"end_character":71},"updated":"2016-10-03 23:48:02.000000000","message":"Wouldn\u0027t it make more sense to leave it as another decorator?\n\n @coordination.synchronized(\u0027{volume_id}-{f_name}\u0027)\n @objects.Volume.set_workers\n def delete_volume(...)\n\nIn that order synchronized locks delete_volume already decorated by set_workers, in other words:\n\n delete_volume \u003d synchronized(set_workers(delete_volume))","commit_id":"3879169b1f2a9636601b419354477ce98255f365"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"3eb50e499d3a52c042ec78454c2fd7e7e9697794","unresolved":false,"context_lines":[{"line_number":659,"context_line":"        return volume.id"},{"line_number":660,"context_line":""},{"line_number":661,"context_line":"    # FIXME(bluex): replace volume_id with volume.id when volume_id is removed"},{"line_number":662,"context_line":"    @coordination.synchronized(\u0027{volume_id}-{f_name}\u0027, set_workers\u003dTrue)"},{"line_number":663,"context_line":"    def delete_volume(self, context, volume_id,"},{"line_number":664,"context_line":"                      unmanage_only\u003dFalse,"},{"line_number":665,"context_line":"                      volume\u003dNone,"}],"source_content_type":"text/x-python","patch_set":28,"id":"3a98d1f4_a2481a8c","line":662,"range":{"start_line":662,"start_character":53,"end_line":662,"end_character":71},"in_reply_to":"3a98d1f4_83db397b","updated":"2016-10-04 13:13:19.000000000","message":"Unfortunately that won\u0027t work, because both decorator,s set_workers and synchronized, do introspection on decorated method, and the synchronized decorator when decorating the set_workers decorator will get ``args`` and ``kwargs`` arguments instead of the ones from ``delete_volume`` so it won\u0027t find volume_id.  :-(","commit_id":"3879169b1f2a9636601b419354477ce98255f365"},{"author":{"_account_id":12825,"name":"Szymon Wróblewski","email":"szymon.wroblewski@ovhcloud.com","username":"bluex"},"change_message_id":"4f8a4d801824526556f5ef44eb1b65ba0c4e18b8","unresolved":false,"context_lines":[{"line_number":659,"context_line":"        return volume.id"},{"line_number":660,"context_line":""},{"line_number":661,"context_line":"    # FIXME(bluex): replace volume_id with volume.id when volume_id is removed"},{"line_number":662,"context_line":"    @coordination.synchronized(\u0027{volume_id}-{f_name}\u0027, set_workers\u003dTrue)"},{"line_number":663,"context_line":"    def delete_volume(self, context, volume_id,"},{"line_number":664,"context_line":"                      unmanage_only\u003dFalse,"},{"line_number":665,"context_line":"                      volume\u003dNone,"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa6399be_2124b7ad","line":662,"range":{"start_line":662,"start_character":53,"end_line":662,"end_character":71},"in_reply_to":"3a98d1f4_a2481a8c","updated":"2016-10-12 16:10:39.000000000","message":"Searching for solution I found decorator module which should allow decorated functions to preserve signature (and it\u0027s already in requrements.txt) see change #385524 with example.","commit_id":"3879169b1f2a9636601b419354477ce98255f365"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"1455772311902a80808aaf95f10003183499666d","unresolved":false,"context_lines":[{"line_number":659,"context_line":"        return volume.id"},{"line_number":660,"context_line":""},{"line_number":661,"context_line":"    # FIXME(bluex): replace volume_id with volume.id when volume_id is removed"},{"line_number":662,"context_line":"    @coordination.synchronized(\u0027{volume_id}-{f_name}\u0027, set_workers\u003dTrue)"},{"line_number":663,"context_line":"    def delete_volume(self, context, volume_id,"},{"line_number":664,"context_line":"                      unmanage_only\u003dFalse,"},{"line_number":665,"context_line":"                      volume\u003dNone,"}],"source_content_type":"text/x-python","patch_set":28,"id":"fa6399be_73aafe71","line":662,"range":{"start_line":662,"start_character":53,"end_line":662,"end_character":71},"in_reply_to":"fa6399be_2124b7ad","updated":"2016-10-12 18:55:39.000000000","message":"I\u0027m always afraid that people will mes up the order of the decorators...\n\nAlthough that specific method doesn\u0027t work in this case, since the decorator can receive optional arguments, the module has another that does, thanks!","commit_id":"3879169b1f2a9636601b419354477ce98255f365"}]}