)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"897afa29bd10cca7b714c8373645ddc8ccdb928c","unresolved":false,"context_lines":[{"line_number":11,"context_line":"be provided to the user. This patch will add a check to see that the"},{"line_number":12,"context_line":"service is running before issuing the rpcapi cast to delete the backup,"},{"line_number":13,"context_line":"and inform the user that the service is down if the check fails."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: Ia9914bfaae8ed9b6972953fc1e78c4471fa1661a"},{"line_number":16,"context_line":"Closes-bug: #1321519"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9a8ffd7b_e1e876b5","line":14,"updated":"2015-12-02 20:01:21.000000000","message":"This should really note that there\u0027s still a race here where if the backup service goes down after the check, the same problem occurs.\n\nThe patch is fine, but we don\u0027t want to pretend it completely fixes this issue.","commit_id":"ca7ef77c97c7323da69e1d5d4b3840f3d9811754"},{"author":{"_account_id":17275,"name":"Nate Potter","email":"nathaniel.potter@intel.com","username":"npotter"},"change_message_id":"3f233dac70b027fba7e68d5f51ef3599ab918d73","unresolved":false,"context_lines":[{"line_number":11,"context_line":"be provided to the user. This patch will add a check to see that the"},{"line_number":12,"context_line":"service is running before issuing the rpcapi cast to delete the backup,"},{"line_number":13,"context_line":"and inform the user that the service is down if the check fails."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change-Id: Ia9914bfaae8ed9b6972953fc1e78c4471fa1661a"},{"line_number":16,"context_line":"Closes-bug: #1321519"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"9a8ffd7b_c19bf215","line":14,"in_reply_to":"9a8ffd7b_e1e876b5","updated":"2015-12-02 20:07:16.000000000","message":"Yes that\u0027s a good point, I\u0027ve added that in now.","commit_id":"ca7ef77c97c7323da69e1d5d4b3840f3d9811754"}],"cinder/backup/api.py":[{"author":{"_account_id":14274,"name":"XinXiaohui","email":"xiaohui.xin@intel.com","username":"XinXiaohui"},"change_message_id":"640be65dfa088bc5022a49b98138c748fef0589a","unresolved":false,"context_lines":[{"line_number":88,"context_line":"            msg \u003d _(\u0027force delete\u0027)"},{"line_number":89,"context_line":"            raise exception.NotSupportedOperation(operation\u003dmsg)"},{"line_number":90,"context_line":"        if not self._is_backup_service_enabled(backup[\u0027availability_zone\u0027],"},{"line_number":91,"context_line":"                                               backup.host):"},{"line_number":92,"context_line":"            raise exception.ServiceNotFound(service_id\u003d\u0027cinder-backup\u0027)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        # Don\u0027t allow backup to be deleted if there are incremental"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba8a016a_7af06dc9","line":91,"updated":"2015-11-19 19:56:31.000000000","message":"nits: It might be better to be consistent to access the member as backup.host. backup[\u0027availability\u0027] -\u003e backup.availability_zone.","commit_id":"56f28a40a033961d07fa4aa82a4f892cb156b6cd"},{"author":{"_account_id":17275,"name":"Nate Potter","email":"nathaniel.potter@intel.com","username":"npotter"},"change_message_id":"e2a07bf86c0c19c5412cef279faea8b19935de79","unresolved":false,"context_lines":[{"line_number":88,"context_line":"            msg \u003d _(\u0027force delete\u0027)"},{"line_number":89,"context_line":"            raise exception.NotSupportedOperation(operation\u003dmsg)"},{"line_number":90,"context_line":"        if not self._is_backup_service_enabled(backup[\u0027availability_zone\u0027],"},{"line_number":91,"context_line":"                                               backup.host):"},{"line_number":92,"context_line":"            raise exception.ServiceNotFound(service_id\u003d\u0027cinder-backup\u0027)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        # Don\u0027t allow backup to be deleted if there are incremental"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a8ffd7b_e9e89d12","line":91,"in_reply_to":"ba8a016a_7af06dc9","updated":"2015-11-25 18:58:14.000000000","message":"Yes thanks, I\u0027ll address that in my next patch set","commit_id":"56f28a40a033961d07fa4aa82a4f892cb156b6cd"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"c6a6cde58895dd31ba448ed757e08dea312d5978","unresolved":false,"context_lines":[{"line_number":89,"context_line":"            raise exception.NotSupportedOperation(operation\u003dmsg)"},{"line_number":90,"context_line":"        if not self._is_backup_service_enabled(backup[\u0027availability_zone\u0027],"},{"line_number":91,"context_line":"                                               backup.host):"},{"line_number":92,"context_line":"            raise exception.ServiceNotFound(service_id\u003d\u0027cinder-backup\u0027)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        # Don\u0027t allow backup to be deleted if there are incremental"},{"line_number":95,"context_line":"        # backups dependent on it."}],"source_content_type":"text/x-python","patch_set":1,"id":"ba8a016a_a7bba0e4","line":92,"updated":"2015-11-17 19:29:38.000000000","message":"It might be better to use ServiceUnavailable?","commit_id":"56f28a40a033961d07fa4aa82a4f892cb156b6cd"},{"author":{"_account_id":8247,"name":"Thang Pham","email":"thang.g.pham@gmail.com","username":"thang.pham"},"change_message_id":"1ffb6314e6fcb43ce1298f6f66cfc21ad5b40119","unresolved":false,"context_lines":[{"line_number":89,"context_line":"            raise exception.NotSupportedOperation(operation\u003dmsg)"},{"line_number":90,"context_line":"        if not self._is_backup_service_enabled(backup[\u0027availability_zone\u0027],"},{"line_number":91,"context_line":"                                               backup.host):"},{"line_number":92,"context_line":"            raise exception.ServiceNotFound(service_id\u003d\u0027cinder-backup\u0027)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        # Don\u0027t allow backup to be deleted if there are incremental"},{"line_number":95,"context_line":"        # backups dependent on it."}],"source_content_type":"text/x-python","patch_set":1,"id":"ba8a016a_a719608a","line":92,"in_reply_to":"ba8a016a_67361854","updated":"2015-11-17 19:38:09.000000000","message":"I did not notice it in the create method.  It is best to just be consistent and use ServiceNotFound.","commit_id":"56f28a40a033961d07fa4aa82a4f892cb156b6cd"},{"author":{"_account_id":17275,"name":"Nate Potter","email":"nathaniel.potter@intel.com","username":"npotter"},"change_message_id":"8edad80a14e3db06255d491c81a3889de524abc3","unresolved":false,"context_lines":[{"line_number":89,"context_line":"            raise exception.NotSupportedOperation(operation\u003dmsg)"},{"line_number":90,"context_line":"        if not self._is_backup_service_enabled(backup[\u0027availability_zone\u0027],"},{"line_number":91,"context_line":"                                               backup.host):"},{"line_number":92,"context_line":"            raise exception.ServiceNotFound(service_id\u003d\u0027cinder-backup\u0027)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        # Don\u0027t allow backup to be deleted if there are incremental"},{"line_number":95,"context_line":"        # backups dependent on it."}],"source_content_type":"text/x-python","patch_set":1,"id":"ba8a016a_67361854","line":92,"in_reply_to":"ba8a016a_a7bba0e4","updated":"2015-11-17 19:34:03.000000000","message":"This is the exception raised by the create method if _is_backup_service_enabled returns false, do you think that would be a better exception to raise in both of these cases?","commit_id":"56f28a40a033961d07fa4aa82a4f892cb156b6cd"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"a6722948ed49389e11aed4229f05ad893fcc10dc","unresolved":false,"context_lines":[{"line_number":125,"context_line":""},{"line_number":126,"context_line":"        return backups"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"    def _is_backup_service_enabled(self, availability_zone, host):"},{"line_number":129,"context_line":"        \"\"\"Check if there is a backup service available.\"\"\""},{"line_number":130,"context_line":"        topic \u003d CONF.backup_topic"},{"line_number":131,"context_line":"        ctxt \u003d context.get_admin_context()"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a740942_fabe2b63","line":128,"updated":"2015-12-10 15:43:59.000000000","message":"Since we changing method signature, it seems to be better to pass only volume object here","commit_id":"860a40f299134fac895a5a68bd171a401c540acb"},{"author":{"_account_id":17275,"name":"Nate Potter","email":"nathaniel.potter@intel.com","username":"npotter"},"change_message_id":"2dc5c2e06298c362b51284067a57418de080b320","unresolved":false,"context_lines":[{"line_number":125,"context_line":""},{"line_number":126,"context_line":"        return backups"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"    def _is_backup_service_enabled(self, availability_zone, host):"},{"line_number":129,"context_line":"        \"\"\"Check if there is a backup service available.\"\"\""},{"line_number":130,"context_line":"        topic \u003d CONF.backup_topic"},{"line_number":131,"context_line":"        ctxt \u003d context.get_admin_context()"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a740942_bd709da5","line":128,"in_reply_to":"7a740942_fabe2b63","updated":"2015-12-10 15:48:00.000000000","message":"I noticed that this method was only being called once, and I had to pass a backup object rather than a volume object to it in the delete method. It seemed easier to just have it pass the availability zone directly since that was all the volume was being used for.","commit_id":"860a40f299134fac895a5a68bd171a401c540acb"}],"cinder/tests/unit/api/contrib/test_backups.py":[{"author":{"_account_id":14084,"name":"Einst Crazy","email":"yu.changcai@99cloud.net","username":"Einst"},"change_message_id":"04408736d9859f5fdbedff6fd6823370a4d1f7e9","unresolved":false,"context_lines":[{"line_number":925,"context_line":"        volume \u003d self.volume_api.get(context.get_admin_context(), volume_id)"},{"line_number":926,"context_line":""},{"line_number":927,"context_line":"        # test empty service"},{"line_number":928,"context_line":"        self.assertEqual(False,"},{"line_number":929,"context_line":"                         self.backup_api._is_backup_service_enabled("},{"line_number":930,"context_line":"                             volume[\u0027availability_zone\u0027],"},{"line_number":931,"context_line":"                             test_host))"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a740942_db30f803","line":928,"updated":"2015-12-07 03:40:43.000000000","message":"I think should change to assertFalse here","commit_id":"cb7e443fdd4a3bcbca4fd13a8e40dd885c706344"},{"author":{"_account_id":17275,"name":"Nate Potter","email":"nathaniel.potter@intel.com","username":"npotter"},"change_message_id":"a0143afde5fa46487486c220aa3989180743b701","unresolved":false,"context_lines":[{"line_number":925,"context_line":"        volume \u003d self.volume_api.get(context.get_admin_context(), volume_id)"},{"line_number":926,"context_line":""},{"line_number":927,"context_line":"        # test empty service"},{"line_number":928,"context_line":"        self.assertEqual(False,"},{"line_number":929,"context_line":"                         self.backup_api._is_backup_service_enabled("},{"line_number":930,"context_line":"                             volume[\u0027availability_zone\u0027],"},{"line_number":931,"context_line":"                             test_host))"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a740942_b454fdfe","line":928,"in_reply_to":"7a740942_db30f803","updated":"2015-12-07 15:19:54.000000000","message":"Actually I do think these should be left as assertEqual, they were going to be changed to assertFalse in this patch https://review.openstack.org/#/c/246554/1..3/cinder/tests/unit/api/contrib/test_backups.py but it ended up being decided that they should be left, you can see their conversation about it in the comments.","commit_id":"cb7e443fdd4a3bcbca4fd13a8e40dd885c706344"}]}