)]}'
{"cinder/tests/unit/test_volume.py":[{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"049f8f69ca6c8f7d6075f1be47e6eefe7edabf87","unresolved":false,"context_lines":[{"line_number":5004,"context_line":"                                                       mock_reschedule):"},{"line_number":5005,"context_line":"        # create source volume"},{"line_number":5006,"context_line":"        mock_execute.side_effect \u003d exception.DriverNotInitialized()"},{"line_number":5007,"context_line":"        mock_reschedule.return_value \u003d None"},{"line_number":5008,"context_line":"        test_vol \u003d tests_utils.create_volume(self.context,"},{"line_number":5009,"context_line":"                                             **self.volume_params)"},{"line_number":5010,"context_line":"        test_vol_id \u003d test_vol[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"5afe65bd_0a686986","line":5007,"updated":"2015-06-03 10:18:03.000000000","message":"How is this going to cover the new method _volume_is_rescheduling?","commit_id":"2bd9f087906800be74185679b507ea0e4dd6e36c"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"967e40c8337cad5c520a19fbcce8e7ccba979b2c","unresolved":false,"context_lines":[{"line_number":5004,"context_line":"                                                       mock_reschedule):"},{"line_number":5005,"context_line":"        # create source volume"},{"line_number":5006,"context_line":"        mock_execute.side_effect \u003d exception.DriverNotInitialized()"},{"line_number":5007,"context_line":"        mock_reschedule.return_value \u003d None"},{"line_number":5008,"context_line":"        test_vol \u003d tests_utils.create_volume(self.context,"},{"line_number":5009,"context_line":"                                             **self.volume_params)"},{"line_number":5010,"context_line":"        test_vol_id \u003d test_vol[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"5afe65bd_3e134668","line":5007,"in_reply_to":"5afe65bd_0a686986","updated":"2015-06-04 02:35:46.000000000","message":"This mock just let the _reschedule no need to call cinder.scheduler in OnFailureRescheduleTask, and the whole revert process will continue to process. The revert method in ExtractVolumeRefTask also will be called.So it will cover the _volume_is_rescheduling.","commit_id":"2bd9f087906800be74185679b507ea0e4dd6e36c"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"020f50fa421027984ccef621b528bf0041e198e8","unresolved":false,"context_lines":[{"line_number":5004,"context_line":"                                                       mock_reschedule):"},{"line_number":5005,"context_line":"        # create source volume"},{"line_number":5006,"context_line":"        mock_execute.side_effect \u003d exception.DriverNotInitialized()"},{"line_number":5007,"context_line":"        mock_reschedule.return_value \u003d None"},{"line_number":5008,"context_line":"        test_vol \u003d tests_utils.create_volume(self.context,"},{"line_number":5009,"context_line":"                                             **self.volume_params)"},{"line_number":5010,"context_line":"        test_vol_id \u003d test_vol[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":4,"id":"5afe65bd_8f62f93c","line":5007,"in_reply_to":"5afe65bd_3e134668","updated":"2015-06-04 10:19:43.000000000","message":"Thanks for clarifying.","commit_id":"2bd9f087906800be74185679b507ea0e4dd6e36c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"97a967026c1651949422682f933ce75ae61c0677","unresolved":false,"context_lines":[{"line_number":5016,"context_line":"        volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5017,"context_line":"        self.assertEqual(\u0027creating\u0027, volume[\u0027status\u0027])"},{"line_number":5018,"context_line":"        # cleanup resource"},{"line_number":5019,"context_line":"        db.volume_destroy(self.context, test_vol_id)"},{"line_number":5020,"context_line":""},{"line_number":5021,"context_line":"    @mock.patch(\u0027cinder.volume.flows.manager.create_volume.\u0027"},{"line_number":5022,"context_line":"                \u0027CreateVolumeFromSpecTask.execute\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3afb71cf_6e8de730","line":5019,"updated":"2015-06-05 14:57:04.000000000","message":"There should be another test when it fails on the last retry to confirm it changes to error, right?","commit_id":"34a5b3db80693ddd0d3d849a6cc8ad04fd9f84fb"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"2839861bf757458844f15683e369ca759d64213f","unresolved":false,"context_lines":[{"line_number":5016,"context_line":"        volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5017,"context_line":"        self.assertEqual(\u0027creating\u0027, volume[\u0027status\u0027])"},{"line_number":5018,"context_line":"        # cleanup resource"},{"line_number":5019,"context_line":"        db.volume_destroy(self.context, test_vol_id)"},{"line_number":5020,"context_line":""},{"line_number":5021,"context_line":"    @mock.patch(\u0027cinder.volume.flows.manager.create_volume.\u0027"},{"line_number":5022,"context_line":"                \u0027CreateVolumeFromSpecTask.execute\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3afb71cf_e190b3ee","line":5019,"in_reply_to":"3afb71cf_6e8de730","updated":"2015-06-06 09:27:53.000000000","message":"I think it\u0027s unnecessarily. Because if the retry number \u003e max_attempts, it will raise exception NoValidHost in cinder-scheduler(https://github.com/openstack/cinder/blob/master/cinder/scheduler/filter_scheduler.py#L223), and the scheduler will set the volume\u0027s status to error in scheduler flow. (https://github.com/openstack/cinder/blob/master/cinder/scheduler/flows/create_volume.py#L139). That has been working there, no need to add another test.","commit_id":"34a5b3db80693ddd0d3d849a6cc8ad04fd9f84fb"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"54a4607f7c92538b28a4686608ad955cd0e80a79","unresolved":false,"context_lines":[{"line_number":5016,"context_line":"        volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5017,"context_line":"        self.assertEqual(\u0027creating\u0027, volume[\u0027status\u0027])"},{"line_number":5018,"context_line":"        # cleanup resource"},{"line_number":5019,"context_line":"        db.volume_destroy(self.context, test_vol_id)"},{"line_number":5020,"context_line":""},{"line_number":5021,"context_line":"    @mock.patch(\u0027cinder.volume.flows.manager.create_volume.\u0027"},{"line_number":5022,"context_line":"                \u0027CreateVolumeFromSpecTask.execute\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3afb71cf_82720b48","line":5019,"in_reply_to":"3afb71cf_e190b3ee","updated":"2015-06-08 07:57:16.000000000","message":"I know that\u0027s been working, but is there a test to confirm that it\u0027s working now and that in the future there\u0027ll be no regressions? Because I think there isn\u0027t one.","commit_id":"34a5b3db80693ddd0d3d849a6cc8ad04fd9f84fb"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"2c08339c67c78c0cc18534c27e6c1dda1a0455f1","unresolved":false,"context_lines":[{"line_number":5025,"context_line":"        volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5026,"context_line":"        self.assertEqual(\u0027creating\u0027, volume[\u0027status\u0027])"},{"line_number":5027,"context_line":"        # cleanup resource"},{"line_number":5028,"context_line":"        db.volume_destroy(self.context, test_vol_id)"},{"line_number":5029,"context_line":""},{"line_number":5030,"context_line":"    def mock_scheduler(self, *args, **kwargs):"},{"line_number":5031,"context_line":"        def scheduler_volume():"}],"source_content_type":"text/x-python","patch_set":8,"id":"3afb71cf_41b95c14","line":5028,"updated":"2015-06-09 18:46:06.000000000","message":"Please, move all clean-ups to tearDown method","commit_id":"5f5a4647d721b02da450dd0525f30d288f117155"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"c1e00253b8b20c4e17cc9d1182ae55e79f31afa1","unresolved":false,"context_lines":[{"line_number":5025,"context_line":"        volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5026,"context_line":"        self.assertEqual(\u0027creating\u0027, volume[\u0027status\u0027])"},{"line_number":5027,"context_line":"        # cleanup resource"},{"line_number":5028,"context_line":"        db.volume_destroy(self.context, test_vol_id)"},{"line_number":5029,"context_line":""},{"line_number":5030,"context_line":"    def mock_scheduler(self, *args, **kwargs):"},{"line_number":5031,"context_line":"        def scheduler_volume():"}],"source_content_type":"text/x-python","patch_set":8,"id":"3afb71cf_056c77ce","line":5028,"in_reply_to":"3afb71cf_41b95c14","updated":"2015-06-10 07:53:20.000000000","message":"The second argument is test_vol_id. I\u0027m not sure how we can move this to tearDown?","commit_id":"5f5a4647d721b02da450dd0525f30d288f117155"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"3fd337040290945463fdc74e22edc6457903a35b","unresolved":false,"context_lines":[{"line_number":5051,"context_line":"                          {\u0027volume_properties\u0027: self.volume_params},"},{"line_number":5052,"context_line":"                          {\u0027retry\u0027: {\u0027num_attempts\u0027: 3, \u0027host\u0027: []}})"},{"line_number":5053,"context_line":"        # Wait 1 second to ensure the volume status has been changed to error"},{"line_number":5054,"context_line":"        eventlet.sleep(1)"},{"line_number":5055,"context_line":"        volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5056,"context_line":"        self.assertEqual(\u0027error\u0027, volume[\u0027status\u0027])"},{"line_number":5057,"context_line":"        # Cleanup resource"}],"source_content_type":"text/x-python","patch_set":8,"id":"3afb71cf_7b940742","line":5054,"updated":"2015-06-09 18:04:47.000000000","message":"I\u0027m not a friend of time dependent tests, as they tend to give spurious failures.","commit_id":"5f5a4647d721b02da450dd0525f30d288f117155"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"2c08339c67c78c0cc18534c27e6c1dda1a0455f1","unresolved":false,"context_lines":[{"line_number":5051,"context_line":"                          {\u0027volume_properties\u0027: self.volume_params},"},{"line_number":5052,"context_line":"                          {\u0027retry\u0027: {\u0027num_attempts\u0027: 3, \u0027host\u0027: []}})"},{"line_number":5053,"context_line":"        # Wait 1 second to ensure the volume status has been changed to error"},{"line_number":5054,"context_line":"        eventlet.sleep(1)"},{"line_number":5055,"context_line":"        volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5056,"context_line":"        self.assertEqual(\u0027error\u0027, volume[\u0027status\u0027])"},{"line_number":5057,"context_line":"        # Cleanup resource"}],"source_content_type":"text/x-python","patch_set":8,"id":"3afb71cf_e13e088a","line":5054,"in_reply_to":"3afb71cf_7b940742","updated":"2015-06-09 18:46:06.000000000","message":"Please, remove sleep from unit tests","commit_id":"5f5a4647d721b02da450dd0525f30d288f117155"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"59eb86866b7db0673fb11767fd8142862c41e3e5","unresolved":false,"context_lines":[{"line_number":5025,"context_line":"        volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5026,"context_line":"        self.assertEqual(\u0027creating\u0027, volume[\u0027status\u0027])"},{"line_number":5027,"context_line":"        # cleanup resource"},{"line_number":5028,"context_line":"        db.volume_destroy(self.context, test_vol_id)"},{"line_number":5029,"context_line":""},{"line_number":5030,"context_line":"    def mock_scheduler(self, *args, **kwargs):"},{"line_number":5031,"context_line":"        def scheduler_volume():"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_4aa39eb2","line":5028,"updated":"2015-06-10 07:37:24.000000000","message":"There\u0027s no need for the cleanup.","commit_id":"8afe3559bcf2791d16e5c70baaeee7ea538be5e1"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"1a740a8f7369fa68bfb33418ecf8831fb63ec6af","unresolved":false,"context_lines":[{"line_number":5025,"context_line":"        volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5026,"context_line":"        self.assertEqual(\u0027creating\u0027, volume[\u0027status\u0027])"},{"line_number":5027,"context_line":"        # cleanup resource"},{"line_number":5028,"context_line":"        db.volume_destroy(self.context, test_vol_id)"},{"line_number":5029,"context_line":""},{"line_number":5030,"context_line":"    def mock_scheduler(self, *args, **kwargs):"},{"line_number":5031,"context_line":"        def scheduler_volume():"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_c54d3f3f","line":5028,"in_reply_to":"3afb71cf_4aa39eb2","updated":"2015-06-10 08:10:36.000000000","message":"Sure, but just want to know is that set to clean up in TestCase?","commit_id":"8afe3559bcf2791d16e5c70baaeee7ea538be5e1"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f320c4dccf9ed0464c38cbcf5c1461fb8676d52d","unresolved":false,"context_lines":[{"line_number":5025,"context_line":"        volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5026,"context_line":"        self.assertEqual(\u0027creating\u0027, volume[\u0027status\u0027])"},{"line_number":5027,"context_line":"        # cleanup resource"},{"line_number":5028,"context_line":"        db.volume_destroy(self.context, test_vol_id)"},{"line_number":5029,"context_line":""},{"line_number":5030,"context_line":"    def mock_scheduler(self, *args, **kwargs):"},{"line_number":5031,"context_line":"        def scheduler_volume():"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_1b897675","line":5028,"in_reply_to":"3afb71cf_c54d3f3f","updated":"2015-06-10 09:17:00.000000000","message":"My bad, this was removed because I thought that in this test class we were cleaning the DB between tests.\n\n@wanghao, sorry","commit_id":"8afe3559bcf2791d16e5c70baaeee7ea538be5e1"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"9d88144dc3644259efc7b31ed06cb73eb6af9644","unresolved":false,"context_lines":[{"line_number":5025,"context_line":"        volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5026,"context_line":"        self.assertEqual(\u0027creating\u0027, volume[\u0027status\u0027])"},{"line_number":5027,"context_line":"        # cleanup resource"},{"line_number":5028,"context_line":"        db.volume_destroy(self.context, test_vol_id)"},{"line_number":5029,"context_line":""},{"line_number":5030,"context_line":"    def mock_scheduler(self, *args, **kwargs):"},{"line_number":5031,"context_line":"        def scheduler_volume():"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_d6df3338","line":5028,"in_reply_to":"3afb71cf_c54d3f3f","updated":"2015-06-10 08:58:17.000000000","message":"Why this is removed? See line 412.","commit_id":"8afe3559bcf2791d16e5c70baaeee7ea538be5e1"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"59eb86866b7db0673fb11767fd8142862c41e3e5","unresolved":false,"context_lines":[{"line_number":5052,"context_line":"                          {\u0027retry\u0027: {\u0027num_attempts\u0027: 3, \u0027host\u0027: []}})"},{"line_number":5053,"context_line":"        # Need to ensure the volume status has been changed to error"},{"line_number":5054,"context_line":"        # So query it for 3 times."},{"line_number":5055,"context_line":"        for i in range(1, 3):"},{"line_number":5056,"context_line":"            volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5057,"context_line":"            if volume[\u0027status\u0027] \u003d\u003d \u0027error\u0027:"},{"line_number":5058,"context_line":"                db.volume_destroy(self.context, test_vol_id)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_4ab83ec8","line":5055,"updated":"2015-06-10 07:37:24.000000000","message":"This will query only twice, not three times.","commit_id":"8afe3559bcf2791d16e5c70baaeee7ea538be5e1"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"7b8fa08fc6da84408f17c4c9be343b3d8dd12c07","unresolved":false,"context_lines":[{"line_number":5052,"context_line":"                          {\u0027retry\u0027: {\u0027num_attempts\u0027: 3, \u0027host\u0027: []}})"},{"line_number":5053,"context_line":"        # Need to ensure the volume status has been changed to error"},{"line_number":5054,"context_line":"        # So query it for 3 times."},{"line_number":5055,"context_line":"        for i in range(1, 3):"},{"line_number":5056,"context_line":"            volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5057,"context_line":"            if volume[\u0027status\u0027] \u003d\u003d \u0027error\u0027:"},{"line_number":5058,"context_line":"                db.volume_destroy(self.context, test_vol_id)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_cab2ee93","line":5055,"updated":"2015-06-10 07:39:46.000000000","message":"Why do we need this loop? If create_volume raises DriverNotInitialized, it also set the volume status to error, right?","commit_id":"8afe3559bcf2791d16e5c70baaeee7ea538be5e1"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"1a740a8f7369fa68bfb33418ecf8831fb63ec6af","unresolved":false,"context_lines":[{"line_number":5052,"context_line":"                          {\u0027retry\u0027: {\u0027num_attempts\u0027: 3, \u0027host\u0027: []}})"},{"line_number":5053,"context_line":"        # Need to ensure the volume status has been changed to error"},{"line_number":5054,"context_line":"        # So query it for 3 times."},{"line_number":5055,"context_line":"        for i in range(1, 3):"},{"line_number":5056,"context_line":"            volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5057,"context_line":"            if volume[\u0027status\u0027] \u003d\u003d \u0027error\u0027:"},{"line_number":5058,"context_line":"                db.volume_destroy(self.context, test_vol_id)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_45b82fc8","line":5055,"in_reply_to":"3afb71cf_cab2ee93","updated":"2015-06-10 08:10:36.000000000","message":"If create_volume raises DriverNotinitialized, it will begin to reschedule the request, during this the volume\u0027status is still \u0027creating\u0027 and if the retry times \u003e 3, the cinder-scheduler will set the status to \u0027error\u0027. So we need a little time to wait the scheduler doing this thing and then confirm \u0027error\u0027 status in finally.","commit_id":"8afe3559bcf2791d16e5c70baaeee7ea538be5e1"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"59eb86866b7db0673fb11767fd8142862c41e3e5","unresolved":false,"context_lines":[{"line_number":5055,"context_line":"        for i in range(1, 3):"},{"line_number":5056,"context_line":"            volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5057,"context_line":"            if volume[\u0027status\u0027] \u003d\u003d \u0027error\u0027:"},{"line_number":5058,"context_line":"                db.volume_destroy(self.context, test_vol_id)"},{"line_number":5059,"context_line":"                return"},{"line_number":5060,"context_line":"        self.assertTrue(False)"},{"line_number":5061,"context_line":"        # Cleanup resource"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_8a9d1668","line":5058,"updated":"2015-06-10 07:37:24.000000000","message":"There\u0027s no need for the cleanup.","commit_id":"8afe3559bcf2791d16e5c70baaeee7ea538be5e1"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"1a740a8f7369fa68bfb33418ecf8831fb63ec6af","unresolved":false,"context_lines":[{"line_number":5055,"context_line":"        for i in range(1, 3):"},{"line_number":5056,"context_line":"            volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5057,"context_line":"            if volume[\u0027status\u0027] \u003d\u003d \u0027error\u0027:"},{"line_number":5058,"context_line":"                db.volume_destroy(self.context, test_vol_id)"},{"line_number":5059,"context_line":"                return"},{"line_number":5060,"context_line":"        self.assertTrue(False)"},{"line_number":5061,"context_line":"        # Cleanup resource"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_65bfabaa","line":5058,"in_reply_to":"3afb71cf_8a9d1668","updated":"2015-06-10 08:10:36.000000000","message":"Done","commit_id":"8afe3559bcf2791d16e5c70baaeee7ea538be5e1"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"59eb86866b7db0673fb11767fd8142862c41e3e5","unresolved":false,"context_lines":[{"line_number":5056,"context_line":"            volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5057,"context_line":"            if volume[\u0027status\u0027] \u003d\u003d \u0027error\u0027:"},{"line_number":5058,"context_line":"                db.volume_destroy(self.context, test_vol_id)"},{"line_number":5059,"context_line":"                return"},{"line_number":5060,"context_line":"        self.assertTrue(False)"},{"line_number":5061,"context_line":"        # Cleanup resource"},{"line_number":5062,"context_line":"        db.volume_destroy(self.context, test_vol_id)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_2a0bc28f","line":5059,"updated":"2015-06-10 07:37:24.000000000","message":"You could use break here and change next line to assert volume status is error.","commit_id":"8afe3559bcf2791d16e5c70baaeee7ea538be5e1"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"1a740a8f7369fa68bfb33418ecf8831fb63ec6af","unresolved":false,"context_lines":[{"line_number":5056,"context_line":"            volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5057,"context_line":"            if volume[\u0027status\u0027] \u003d\u003d \u0027error\u0027:"},{"line_number":5058,"context_line":"                db.volume_destroy(self.context, test_vol_id)"},{"line_number":5059,"context_line":"                return"},{"line_number":5060,"context_line":"        self.assertTrue(False)"},{"line_number":5061,"context_line":"        # Cleanup resource"},{"line_number":5062,"context_line":"        db.volume_destroy(self.context, test_vol_id)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_45df0f0d","line":5059,"in_reply_to":"3afb71cf_2a0bc28f","updated":"2015-06-10 08:10:36.000000000","message":"Done","commit_id":"8afe3559bcf2791d16e5c70baaeee7ea538be5e1"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"7b8fa08fc6da84408f17c4c9be343b3d8dd12c07","unresolved":false,"context_lines":[{"line_number":5057,"context_line":"            if volume[\u0027status\u0027] \u003d\u003d \u0027error\u0027:"},{"line_number":5058,"context_line":"                db.volume_destroy(self.context, test_vol_id)"},{"line_number":5059,"context_line":"                return"},{"line_number":5060,"context_line":"        self.assertTrue(False)"},{"line_number":5061,"context_line":"        # Cleanup resource"},{"line_number":5062,"context_line":"        db.volume_destroy(self.context, test_vol_id)"},{"line_number":5063,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_2a67424e","line":5060,"updated":"2015-06-10 07:39:46.000000000","message":"This will be very confusing, please add a message.","commit_id":"8afe3559bcf2791d16e5c70baaeee7ea538be5e1"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"1a740a8f7369fa68bfb33418ecf8831fb63ec6af","unresolved":false,"context_lines":[{"line_number":5057,"context_line":"            if volume[\u0027status\u0027] \u003d\u003d \u0027error\u0027:"},{"line_number":5058,"context_line":"                db.volume_destroy(self.context, test_vol_id)"},{"line_number":5059,"context_line":"                return"},{"line_number":5060,"context_line":"        self.assertTrue(False)"},{"line_number":5061,"context_line":"        # Cleanup resource"},{"line_number":5062,"context_line":"        db.volume_destroy(self.context, test_vol_id)"},{"line_number":5063,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_25e413df","line":5060,"in_reply_to":"3afb71cf_2a67424e","updated":"2015-06-10 08:10:36.000000000","message":"Done","commit_id":"8afe3559bcf2791d16e5c70baaeee7ea538be5e1"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"59eb86866b7db0673fb11767fd8142862c41e3e5","unresolved":false,"context_lines":[{"line_number":5059,"context_line":"                return"},{"line_number":5060,"context_line":"        self.assertTrue(False)"},{"line_number":5061,"context_line":"        # Cleanup resource"},{"line_number":5062,"context_line":"        db.volume_destroy(self.context, test_vol_id)"},{"line_number":5063,"context_line":""},{"line_number":5064,"context_line":"    @mock.patch(\u0027cinder.volume.flows.manager.create_volume.\u0027"},{"line_number":5065,"context_line":"                \u0027CreateVolumeFromSpecTask.execute\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_eaadaa9b","line":5062,"updated":"2015-06-10 07:37:24.000000000","message":"There\u0027s no need for the cleanup.","commit_id":"8afe3559bcf2791d16e5c70baaeee7ea538be5e1"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"1a740a8f7369fa68bfb33418ecf8831fb63ec6af","unresolved":false,"context_lines":[{"line_number":5059,"context_line":"                return"},{"line_number":5060,"context_line":"        self.assertTrue(False)"},{"line_number":5061,"context_line":"        # Cleanup resource"},{"line_number":5062,"context_line":"        db.volume_destroy(self.context, test_vol_id)"},{"line_number":5063,"context_line":""},{"line_number":5064,"context_line":"    @mock.patch(\u0027cinder.volume.flows.manager.create_volume.\u0027"},{"line_number":5065,"context_line":"                \u0027CreateVolumeFromSpecTask.execute\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_e5e95bb8","line":5062,"in_reply_to":"3afb71cf_eaadaa9b","updated":"2015-06-10 08:10:36.000000000","message":"Done","commit_id":"8afe3559bcf2791d16e5c70baaeee7ea538be5e1"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"59eb86866b7db0673fb11767fd8142862c41e3e5","unresolved":false,"context_lines":[{"line_number":5079,"context_line":"        volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5080,"context_line":"        self.assertEqual(\u0027error\u0027, volume[\u0027status\u0027])"},{"line_number":5081,"context_line":"        # cleanup resource"},{"line_number":5082,"context_line":"        db.volume_destroy(self.context, test_vol_id)"},{"line_number":5083,"context_line":""},{"line_number":5084,"context_line":""},{"line_number":5085,"context_line":"class CopyVolumeToImageTestCase(BaseVolumeTestCase):"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_aac9f25a","line":5082,"updated":"2015-06-10 07:37:24.000000000","message":"There\u0027s no need for the cleanup.","commit_id":"8afe3559bcf2791d16e5c70baaeee7ea538be5e1"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"1a740a8f7369fa68bfb33418ecf8831fb63ec6af","unresolved":false,"context_lines":[{"line_number":5079,"context_line":"        volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5080,"context_line":"        self.assertEqual(\u0027error\u0027, volume[\u0027status\u0027])"},{"line_number":5081,"context_line":"        # cleanup resource"},{"line_number":5082,"context_line":"        db.volume_destroy(self.context, test_vol_id)"},{"line_number":5083,"context_line":""},{"line_number":5084,"context_line":""},{"line_number":5085,"context_line":"class CopyVolumeToImageTestCase(BaseVolumeTestCase):"}],"source_content_type":"text/x-python","patch_set":9,"id":"3afb71cf_a5d003f7","line":5082,"in_reply_to":"3afb71cf_aac9f25a","updated":"2015-06-10 08:10:36.000000000","message":"Done","commit_id":"8afe3559bcf2791d16e5c70baaeee7ea538be5e1"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"8a26906ba73403ec9e559728a0a9fe2a0c96726f","unresolved":false,"context_lines":[{"line_number":5009,"context_line":"                \u0027OnFailureRescheduleTask._reschedule\u0027)"},{"line_number":5010,"context_line":"    @mock.patch(\u0027cinder.volume.flows.manager.create_volume.\u0027"},{"line_number":5011,"context_line":"                \u0027CreateVolumeFromSpecTask.execute\u0027)"},{"line_number":5012,"context_line":"    def test_create_volume_raise_rescheduled_exception(self, mock_execute,"},{"line_number":5013,"context_line":"                                                       mock_reschedule):"},{"line_number":5014,"context_line":"        # Create source volume"},{"line_number":5015,"context_line":"        mock_execute.side_effect \u003d exception.DriverNotInitialized()"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afb71cf_ada30500","line":5012,"updated":"2015-06-11 15:09:55.000000000","message":"This test is redundant, there is already a test checking if DriverNotInitialized sets volume status to error (test_create_driver_not_initialized).","commit_id":"3bb15a598135b753668b20baca88a3467d576deb"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"7fdf3809b6f69ef35bdabc31b8d2eacc9bc3944c","unresolved":false,"context_lines":[{"line_number":5009,"context_line":"                \u0027OnFailureRescheduleTask._reschedule\u0027)"},{"line_number":5010,"context_line":"    @mock.patch(\u0027cinder.volume.flows.manager.create_volume.\u0027"},{"line_number":5011,"context_line":"                \u0027CreateVolumeFromSpecTask.execute\u0027)"},{"line_number":5012,"context_line":"    def test_create_volume_raise_rescheduled_exception(self, mock_execute,"},{"line_number":5013,"context_line":"                                                       mock_reschedule):"},{"line_number":5014,"context_line":"        # Create source volume"},{"line_number":5015,"context_line":"        mock_execute.side_effect \u003d exception.DriverNotInitialized()"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afb71cf_9ea86ddd","line":5012,"in_reply_to":"3afb71cf_73a28a9c","updated":"2015-06-11 15:42:30.000000000","message":"Ah, it checks if status is creating. That\u0027s right, it is fine. :)","commit_id":"3bb15a598135b753668b20baca88a3467d576deb"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"984e80492cc335d6d905bac2105e1877c22b7173","unresolved":false,"context_lines":[{"line_number":5009,"context_line":"                \u0027OnFailureRescheduleTask._reschedule\u0027)"},{"line_number":5010,"context_line":"    @mock.patch(\u0027cinder.volume.flows.manager.create_volume.\u0027"},{"line_number":5011,"context_line":"                \u0027CreateVolumeFromSpecTask.execute\u0027)"},{"line_number":5012,"context_line":"    def test_create_volume_raise_rescheduled_exception(self, mock_execute,"},{"line_number":5013,"context_line":"                                                       mock_reschedule):"},{"line_number":5014,"context_line":"        # Create source volume"},{"line_number":5015,"context_line":"        mock_execute.side_effect \u003d exception.DriverNotInitialized()"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afb71cf_73a28a9c","line":5012,"in_reply_to":"3afb71cf_ada30500","updated":"2015-06-11 15:28:13.000000000","message":"I think this is valid, since it\u0027s testing that final state after retries is correct, which is not tested anywhere else (I think).","commit_id":"3bb15a598135b753668b20baca88a3467d576deb"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"8a26906ba73403ec9e559728a0a9fe2a0c96726f","unresolved":false,"context_lines":[{"line_number":5036,"context_line":"                \u0027create_volume\u0027, mock_scheduler)"},{"line_number":5037,"context_line":"    @mock.patch(\u0027cinder.volume.flows.manager.create_volume.\u0027"},{"line_number":5038,"context_line":"                \u0027CreateVolumeFromSpecTask.execute\u0027)"},{"line_number":5039,"context_line":"    def test_create_volume_get_error_after_retry(self, mock_execute):"},{"line_number":5040,"context_line":"        # Create source volume"},{"line_number":5041,"context_line":"        mock_execute.side_effect \u003d exception.DriverNotInitialized()"},{"line_number":5042,"context_line":"        test_vol \u003d tests_utils.create_volume(self.context,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afb71cf_8d44c91b","line":5039,"updated":"2015-06-11 15:09:55.000000000","message":"This test actually doesn\u0027t prove your code is right (although it is ;)). It doesn\u0027t check if a volume was set to error *between* failure and rescheduling. Instead of that I would prefer to have another test which mocks scheduler\u0027s create_volume to do nothing and check if in such case volume stays in creating state.\n\nAnother solution would be to check for volume state *inside* mock_scheduler...","commit_id":"3bb15a598135b753668b20baca88a3467d576deb"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"7658b10d4aaf3f5bf8a0c765d7a46bfef5c2689e","unresolved":false,"context_lines":[{"line_number":5036,"context_line":"                \u0027create_volume\u0027, mock_scheduler)"},{"line_number":5037,"context_line":"    @mock.patch(\u0027cinder.volume.flows.manager.create_volume.\u0027"},{"line_number":5038,"context_line":"                \u0027CreateVolumeFromSpecTask.execute\u0027)"},{"line_number":5039,"context_line":"    def test_create_volume_get_error_after_retry(self, mock_execute):"},{"line_number":5040,"context_line":"        # Create source volume"},{"line_number":5041,"context_line":"        mock_execute.side_effect \u003d exception.DriverNotInitialized()"},{"line_number":5042,"context_line":"        test_vol \u003d tests_utils.create_volume(self.context,"}],"source_content_type":"text/x-python","patch_set":10,"id":"1af86dd1_58f9df4c","line":5039,"in_reply_to":"1af86dd1_bbb452d1","updated":"2015-06-12 09:12:31.000000000","message":"Okay, I can agree to that.","commit_id":"3bb15a598135b753668b20baca88a3467d576deb"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"7fdf3809b6f69ef35bdabc31b8d2eacc9bc3944c","unresolved":false,"context_lines":[{"line_number":5036,"context_line":"                \u0027create_volume\u0027, mock_scheduler)"},{"line_number":5037,"context_line":"    @mock.patch(\u0027cinder.volume.flows.manager.create_volume.\u0027"},{"line_number":5038,"context_line":"                \u0027CreateVolumeFromSpecTask.execute\u0027)"},{"line_number":5039,"context_line":"    def test_create_volume_get_error_after_retry(self, mock_execute):"},{"line_number":5040,"context_line":"        # Create source volume"},{"line_number":5041,"context_line":"        mock_execute.side_effect \u003d exception.DriverNotInitialized()"},{"line_number":5042,"context_line":"        test_vol \u003d tests_utils.create_volume(self.context,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afb71cf_3eacd98b","line":5039,"in_reply_to":"3afb71cf_33f1b296","updated":"2015-06-11 15:42:30.000000000","message":"This can be done either by checking in mock_scheduler or like Gorka proposed. Both ways work for me. :)","commit_id":"3bb15a598135b753668b20baca88a3467d576deb"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"df4bded22ef530527319ef5cad63cbb6cc95ffa5","unresolved":false,"context_lines":[{"line_number":5036,"context_line":"                \u0027create_volume\u0027, mock_scheduler)"},{"line_number":5037,"context_line":"    @mock.patch(\u0027cinder.volume.flows.manager.create_volume.\u0027"},{"line_number":5038,"context_line":"                \u0027CreateVolumeFromSpecTask.execute\u0027)"},{"line_number":5039,"context_line":"    def test_create_volume_get_error_after_retry(self, mock_execute):"},{"line_number":5040,"context_line":"        # Create source volume"},{"line_number":5041,"context_line":"        mock_execute.side_effect \u003d exception.DriverNotInitialized()"},{"line_number":5042,"context_line":"        test_vol \u003d tests_utils.create_volume(self.context,"}],"source_content_type":"text/x-python","patch_set":10,"id":"1af86dd1_bbb452d1","line":5039,"in_reply_to":"3afb71cf_3eacd98b","updated":"2015-06-12 06:33:10.000000000","message":"Guys, I feel we have tested this situation as you said. In test_create_volume_raise_rescheduled_exception, we mock the mock_reschedule.return_value \u003d None, it just let the scheduler\u0027s create_volume to do nothing, and check the volume\u0027s status still be error. Do we really need another test to prove it again?","commit_id":"3bb15a598135b753668b20baca88a3467d576deb"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"984e80492cc335d6d905bac2105e1877c22b7173","unresolved":false,"context_lines":[{"line_number":5036,"context_line":"                \u0027create_volume\u0027, mock_scheduler)"},{"line_number":5037,"context_line":"    @mock.patch(\u0027cinder.volume.flows.manager.create_volume.\u0027"},{"line_number":5038,"context_line":"                \u0027CreateVolumeFromSpecTask.execute\u0027)"},{"line_number":5039,"context_line":"    def test_create_volume_get_error_after_retry(self, mock_execute):"},{"line_number":5040,"context_line":"        # Create source volume"},{"line_number":5041,"context_line":"        mock_execute.side_effect \u003d exception.DriverNotInitialized()"},{"line_number":5042,"context_line":"        test_vol \u003d tests_utils.create_volume(self.context,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afb71cf_33f1b296","line":5039,"in_reply_to":"3afb71cf_8d44c91b","updated":"2015-06-11 15:28:13.000000000","message":"You are correct, we should check that the status is not changed in between. And I would do that with a mock (setting wraps to True) of error_out_volume and at the end check what changes have been made.","commit_id":"3bb15a598135b753668b20baca88a3467d576deb"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"8a26906ba73403ec9e559728a0a9fe2a0c96726f","unresolved":false,"context_lines":[{"line_number":5050,"context_line":"                          {\u0027retry\u0027: {\u0027num_attempts\u0027: 3, \u0027host\u0027: []}})"},{"line_number":5051,"context_line":"        # Need to ensure the volume status has been changed to error"},{"line_number":5052,"context_line":"        # So query it for 3 times."},{"line_number":5053,"context_line":"        for i in range(1, 4):"},{"line_number":5054,"context_line":"            volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5055,"context_line":"            if volume[\u0027status\u0027] \u003d\u003d \u0027error\u0027:"},{"line_number":5056,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afb71cf_4d47f11d","line":5053,"updated":"2015-06-11 15:09:55.000000000","message":"range(3) would be more readable.","commit_id":"3bb15a598135b753668b20baca88a3467d576deb"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"984e80492cc335d6d905bac2105e1877c22b7173","unresolved":false,"context_lines":[{"line_number":5050,"context_line":"                          {\u0027retry\u0027: {\u0027num_attempts\u0027: 3, \u0027host\u0027: []}})"},{"line_number":5051,"context_line":"        # Need to ensure the volume status has been changed to error"},{"line_number":5052,"context_line":"        # So query it for 3 times."},{"line_number":5053,"context_line":"        for i in range(1, 4):"},{"line_number":5054,"context_line":"            volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5055,"context_line":"            if volume[\u0027status\u0027] \u003d\u003d \u0027error\u0027:"},{"line_number":5056,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afb71cf_d3d6de26","line":5053,"in_reply_to":"3afb71cf_4d47f11d","updated":"2015-06-11 15:28:13.000000000","message":"I agree, but it\u0027s not that big on a difference since it\u0027s a test, that\u0027s why I didn\u0027t raise it.","commit_id":"3bb15a598135b753668b20baca88a3467d576deb"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"df4bded22ef530527319ef5cad63cbb6cc95ffa5","unresolved":false,"context_lines":[{"line_number":5050,"context_line":"                          {\u0027retry\u0027: {\u0027num_attempts\u0027: 3, \u0027host\u0027: []}})"},{"line_number":5051,"context_line":"        # Need to ensure the volume status has been changed to error"},{"line_number":5052,"context_line":"        # So query it for 3 times."},{"line_number":5053,"context_line":"        for i in range(1, 4):"},{"line_number":5054,"context_line":"            volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5055,"context_line":"            if volume[\u0027status\u0027] \u003d\u003d \u0027error\u0027:"},{"line_number":5056,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":10,"id":"1af86dd1_02953fe3","line":5053,"in_reply_to":"3afb71cf_d3d6de26","updated":"2015-06-12 06:33:10.000000000","message":"Sure, it will be updated.","commit_id":"3bb15a598135b753668b20baca88a3467d576deb"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"8a26906ba73403ec9e559728a0a9fe2a0c96726f","unresolved":false,"context_lines":[{"line_number":5059,"context_line":""},{"line_number":5060,"context_line":"    @mock.patch(\u0027cinder.volume.flows.manager.create_volume.\u0027"},{"line_number":5061,"context_line":"                \u0027CreateVolumeFromSpecTask.execute\u0027)"},{"line_number":5062,"context_line":"    def test_create_volume_raise_unrescheduled_exception(self, mock_execute):"},{"line_number":5063,"context_line":"        # create source volume"},{"line_number":5064,"context_line":"        mock_execute.side_effect \u003d exception.MetadataCopyFailure("},{"line_number":5065,"context_line":"            reason\u003d\"Because of DB service down.\")"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afb71cf_aa6bebf0","line":5062,"updated":"2015-06-11 15:09:55.000000000","message":"I think this test is redundant. There are already tests checking if raising MetadataCopyFailure changes volume state to error.","commit_id":"3bb15a598135b753668b20baca88a3467d576deb"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"df4bded22ef530527319ef5cad63cbb6cc95ffa5","unresolved":false,"context_lines":[{"line_number":5059,"context_line":""},{"line_number":5060,"context_line":"    @mock.patch(\u0027cinder.volume.flows.manager.create_volume.\u0027"},{"line_number":5061,"context_line":"                \u0027CreateVolumeFromSpecTask.execute\u0027)"},{"line_number":5062,"context_line":"    def test_create_volume_raise_unrescheduled_exception(self, mock_execute):"},{"line_number":5063,"context_line":"        # create source volume"},{"line_number":5064,"context_line":"        mock_execute.side_effect \u003d exception.MetadataCopyFailure("},{"line_number":5065,"context_line":"            reason\u003d\"Because of DB service down.\")"}],"source_content_type":"text/x-python","patch_set":10,"id":"1af86dd1_62ff5335","line":5062,"in_reply_to":"3afb71cf_7ee6d117","updated":"2015-06-12 06:33:10.000000000","message":"@Michal, This test is that we want to focus on checking the volume status keeping error under unrescheduled exception. I think it is better to change the MedataCopyFailure to VolumeNotFound, right?","commit_id":"3bb15a598135b753668b20baca88a3467d576deb"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"984e80492cc335d6d905bac2105e1877c22b7173","unresolved":false,"context_lines":[{"line_number":5059,"context_line":""},{"line_number":5060,"context_line":"    @mock.patch(\u0027cinder.volume.flows.manager.create_volume.\u0027"},{"line_number":5061,"context_line":"                \u0027CreateVolumeFromSpecTask.execute\u0027)"},{"line_number":5062,"context_line":"    def test_create_volume_raise_unrescheduled_exception(self, mock_execute):"},{"line_number":5063,"context_line":"        # create source volume"},{"line_number":5064,"context_line":"        mock_execute.side_effect \u003d exception.MetadataCopyFailure("},{"line_number":5065,"context_line":"            reason\u003d\"Because of DB service down.\")"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afb71cf_d32ffe3c","line":5062,"in_reply_to":"3afb71cf_aa6bebf0","updated":"2015-06-11 15:28:13.000000000","message":"I think this is valid, since it\u0027s a little different, all those were testing something else and had no retries, this one tests bare volume creation, without copy from snapshot or anything else, just to check the retry on its own.","commit_id":"3bb15a598135b753668b20baca88a3467d576deb"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"7fdf3809b6f69ef35bdabc31b8d2eacc9bc3944c","unresolved":false,"context_lines":[{"line_number":5059,"context_line":""},{"line_number":5060,"context_line":"    @mock.patch(\u0027cinder.volume.flows.manager.create_volume.\u0027"},{"line_number":5061,"context_line":"                \u0027CreateVolumeFromSpecTask.execute\u0027)"},{"line_number":5062,"context_line":"    def test_create_volume_raise_unrescheduled_exception(self, mock_execute):"},{"line_number":5063,"context_line":"        # create source volume"},{"line_number":5064,"context_line":"        mock_execute.side_effect \u003d exception.MetadataCopyFailure("},{"line_number":5065,"context_line":"            reason\u003d\"Because of DB service down.\")"}],"source_content_type":"text/x-python","patch_set":10,"id":"3afb71cf_7ee6d117","line":5062,"in_reply_to":"3afb71cf_d32ffe3c","updated":"2015-06-11 15:42:30.000000000","message":"So in case of this one we don\u0027t have any retries - MetadataCopyFailure doesn\u0027t trigger rescheduling. Also - it is impossible to get such exception when creating raw volume. So in my opinion tests that are using _raise_metadata_copy_failure are very similar and are doing same thing.","commit_id":"3bb15a598135b753668b20baca88a3467d576deb"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"5e3be5935af690276ec71471ecbefb12f2c990a1","unresolved":false,"context_lines":[{"line_number":5051,"context_line":"        # Need to ensure the volume status has been changed to error"},{"line_number":5052,"context_line":"        # So query it for 3 times."},{"line_number":5053,"context_line":"        for i in range(3):"},{"line_number":5054,"context_line":"            volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5055,"context_line":"            if volume[\u0027status\u0027] \u003d\u003d \u0027error\u0027:"},{"line_number":5056,"context_line":"                break"},{"line_number":5057,"context_line":"        # Ensure the volume status has changed to \u0027error\u0027."}],"source_content_type":"text/x-python","patch_set":12,"id":"1af86dd1_bbafade2","line":5054,"updated":"2015-06-12 09:12:15.000000000","message":"This check is dependent on how fast our eventlet greenthread spawned in #5033 will process the request. This creates a potential race condition. It may happen that greenthreads will be scheduled in a way that even after querying 3 times we\u0027ll get an old value. Such situation is unacceptable in unit tests.\n\nWhat we should do is just to mock cinder.scheduler.rpcapi.SchedulerAPI.create_volume and make sure it was called. Another test for scheduler manager should make sure that scheduler manager errors out volume (and such test is probably already there).","commit_id":"7d85adeee498ee8e69dde14da41996a46387c501"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"ab4b13c55932517999b7485d0a8bb0f048b27dbd","unresolved":false,"context_lines":[{"line_number":5051,"context_line":"        # Need to ensure the volume status has been changed to error"},{"line_number":5052,"context_line":"        # So query it for 3 times."},{"line_number":5053,"context_line":"        for i in range(3):"},{"line_number":5054,"context_line":"            volume \u003d db.volume_get(context.get_admin_context(), test_vol_id)"},{"line_number":5055,"context_line":"            if volume[\u0027status\u0027] \u003d\u003d \u0027error\u0027:"},{"line_number":5056,"context_line":"                break"},{"line_number":5057,"context_line":"        # Ensure the volume status has changed to \u0027error\u0027."}],"source_content_type":"text/x-python","patch_set":12,"id":"fa32b979_6554c720","line":5054,"in_reply_to":"1af86dd1_bbafade2","updated":"2015-06-16 08:50:09.000000000","message":"Sure, will update it. And I found such test has been there -test_create_volume_exception_puts_volume_in_error_state.","commit_id":"7d85adeee498ee8e69dde14da41996a46387c501"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"5e3be5935af690276ec71471ecbefb12f2c990a1","unresolved":false,"context_lines":[{"line_number":5064,"context_line":"        test_vol \u003d tests_utils.create_volume(self.context,"},{"line_number":5065,"context_line":"                                             **self.volume_params)"},{"line_number":5066,"context_line":"        test_vol_id \u003d test_vol[\u0027id\u0027]"},{"line_number":5067,"context_line":"        mock_execute.side_effect \u003d exception.VolumeNotFound("},{"line_number":5068,"context_line":"            volume_id\u003dtest_vol_id)"},{"line_number":5069,"context_line":"        self.assertRaises(exception.VolumeNotFound,"},{"line_number":5070,"context_line":"                          self.volume.create_volume,"}],"source_content_type":"text/x-python","patch_set":12,"id":"1af86dd1_db24a9b0","line":5067,"updated":"2015-06-12 09:12:15.000000000","message":"We aren\u0027t testing VolumeNotFound situation anywhere, so I\u0027m fairly okay with adding this test...","commit_id":"7d85adeee498ee8e69dde14da41996a46387c501"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"7f4c85975ceeac463d51f46685f3c40f4c35f01b","unresolved":false,"context_lines":[{"line_number":52,"context_line":"from cinder.openstack.common import fileutils"},{"line_number":53,"context_line":"import cinder.policy"},{"line_number":54,"context_line":"from cinder import quota"},{"line_number":55,"context_line":"from cinder.scheduler import manager as sch_manager"},{"line_number":56,"context_line":"from cinder import test"},{"line_number":57,"context_line":"from cinder.tests.unit.api import fakes"},{"line_number":58,"context_line":"from cinder.tests.unit.brick import fake_lvm"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa32b979_6a51eb61","line":55,"updated":"2015-06-16 11:45:34.000000000","message":"This is not needed now.","commit_id":"f7e11ef509c93f0a7add173f15eb0ee4bb310978"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"5879f1ea848747f11b8838abe7dd0bde4474f402","unresolved":false,"context_lines":[{"line_number":52,"context_line":"from cinder.openstack.common import fileutils"},{"line_number":53,"context_line":"import cinder.policy"},{"line_number":54,"context_line":"from cinder import quota"},{"line_number":55,"context_line":"from cinder.scheduler import manager as sch_manager"},{"line_number":56,"context_line":"from cinder import test"},{"line_number":57,"context_line":"from cinder.tests.unit.api import fakes"},{"line_number":58,"context_line":"from cinder.tests.unit.brick import fake_lvm"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa32b979_0549c9cf","line":55,"in_reply_to":"fa32b979_6301d405","updated":"2015-06-18 09:36:59.000000000","message":"Done","commit_id":"f7e11ef509c93f0a7add173f15eb0ee4bb310978"},{"author":{"_account_id":8074,"name":"Jin Nan Zhang","username":"zhangjinnan","inactive":true},"change_message_id":"5353eea7687b51760a3c012e6e7e310dbf9779d6","unresolved":false,"context_lines":[{"line_number":52,"context_line":"from cinder.openstack.common import fileutils"},{"line_number":53,"context_line":"import cinder.policy"},{"line_number":54,"context_line":"from cinder import quota"},{"line_number":55,"context_line":"from cinder.scheduler import manager as sch_manager"},{"line_number":56,"context_line":"from cinder import test"},{"line_number":57,"context_line":"from cinder.tests.unit.api import fakes"},{"line_number":58,"context_line":"from cinder.tests.unit.brick import fake_lvm"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa32b979_6301d405","line":55,"in_reply_to":"fa32b979_6a51eb61","updated":"2015-06-17 15:14:11.000000000","message":"Done","commit_id":"f7e11ef509c93f0a7add173f15eb0ee4bb310978"}],"cinder/volume/flows/manager/create_volume.py":[{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"c8d761b74d9502363c337cc3b601a70b6495f907","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        # If volume in re-scheduling process, we should not change the volume\u0027s"},{"line_number":199,"context_line":"        # status to error."},{"line_number":200,"context_line":"        cause \u003d list(kwargs[\u0027flow_failures\u0027].values())[0]"},{"line_number":201,"context_line":"        if cause.exception.rescheduled:"},{"line_number":202,"context_line":"            return"},{"line_number":203,"context_line":"        common.error_out_volume(context, self.db, volume_id)"},{"line_number":204,"context_line":"        LOG.error(_LE(\"Volume %s: create failed\"), volume_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_a9dc0375","line":201,"updated":"2015-05-26 11:44:36.000000000","message":"Is this condition actually working? In TaskFlow flow is reverted in a reversed way, so revert for ExtractVolumeRefTask will happen *before* revert for OnFailureRescheduleTask where rescheduled attribute is set on the exception.\n\nYou may want to move error_out_volume to OnFailureRescheduleTask (maybe renamed somehow?) and make it run on every flow (see line 765). Or any other solution?","commit_id":"cfabdaef272e4c92fda07de7bd412de77a990c61"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"25a9331bca7286a7da9b7b8c3243de418f4e1d64","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        # If volume in re-scheduling process, we should not change the volume\u0027s"},{"line_number":199,"context_line":"        # status to error."},{"line_number":200,"context_line":"        cause \u003d list(kwargs[\u0027flow_failures\u0027].values())[0]"},{"line_number":201,"context_line":"        if cause.exception.rescheduled:"},{"line_number":202,"context_line":"            return"},{"line_number":203,"context_line":"        common.error_out_volume(context, self.db, volume_id)"},{"line_number":204,"context_line":"        LOG.error(_LE(\"Volume %s: create failed\"), volume_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_c9ebbf77","line":201,"in_reply_to":"7a016987_a9dc0375","updated":"2015-05-26 12:08:12.000000000","message":"It is working if the volume_flow add OnFailureRescheduleTask. I test it in my environment. In this situation, ExtractVolumeRefTask will execute \u0027revert\u0027 after OnFailureRescheduleTask execute \u0027revert\u0027.  But there is issue that if allow_reschedule is false, volume_flow will not add  OnFailureRescheduleTask, and also cause.exception.rescheduled attribute will not be set. I have updated a new patch to fix this problem. Please review it.","commit_id":"cfabdaef272e4c92fda07de7bd412de77a990c61"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"799e8ea8b722fcfe8f8911509c4eb158061ac056","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        # If volume in re-scheduling process, we should not change the volume\u0027s"},{"line_number":199,"context_line":"        # status to error."},{"line_number":200,"context_line":"        cause \u003d list(kwargs[\u0027flow_failures\u0027].values())[0]"},{"line_number":201,"context_line":"        if cause.exception.rescheduled:"},{"line_number":202,"context_line":"            return"},{"line_number":203,"context_line":"        common.error_out_volume(context, self.db, volume_id)"},{"line_number":204,"context_line":"        LOG.error(_LE(\"Volume %s: create failed\"), volume_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a016987_5b5d56eb","line":201,"in_reply_to":"7a016987_c9ebbf77","updated":"2015-05-26 12:22:28.000000000","message":"Ah, you\u0027re right, OnFailureRescheduleTask is added before ExtractVolumeRefTask. Thanks, my bad.","commit_id":"cfabdaef272e4c92fda07de7bd412de77a990c61"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"2aa11673454049a5c8c5b91386cec7bb6d4ef874","unresolved":false,"context_lines":[{"line_number":197,"context_line":"            return"},{"line_number":198,"context_line":"        # If volume in re-scheduling process, we should not change the volume\u0027s"},{"line_number":199,"context_line":"        # status to error."},{"line_number":200,"context_line":"        cause \u003d list(kwargs[\u0027flow_failures\u0027].values())[0]"},{"line_number":201,"context_line":"        # If not allow reschedule, the exception do not have attribute"},{"line_number":202,"context_line":"        # \u0027rescheduled\u0027. So catch the exception and just pass."},{"line_number":203,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a016987_869c465e","line":200,"updated":"2015-05-28 01:36:21.000000000","message":"Maybe it\u0027s just me and I admit I didn\u0027t read the doc for TaskFlow.  But this is one of example that TaskFlow has made the code difficult to read/understand.  I don\u0027t know if I should -1 this change because I don\u0027t know if \u0027flow_failures\u0027 will always be part of kwargs.","commit_id":"6ce41da61f50dbf6c48253ecd7803dd707b06608"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"a11838f30d88d9f763fa8daf5b5a9f1e7530fc91","unresolved":false,"context_lines":[{"line_number":197,"context_line":"            return"},{"line_number":198,"context_line":"        # If volume in re-scheduling process, we should not change the volume\u0027s"},{"line_number":199,"context_line":"        # status to error."},{"line_number":200,"context_line":"        cause \u003d list(kwargs[\u0027flow_failures\u0027].values())[0]"},{"line_number":201,"context_line":"        # If not allow reschedule, the exception do not have attribute"},{"line_number":202,"context_line":"        # \u0027rescheduled\u0027. So catch the exception and just pass."},{"line_number":203,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5afe65bd_1c9a3b15","line":200,"updated":"2015-06-02 00:56:05.000000000","message":"Michal, Hao: \"**kwargs key \u0027flow_failures\u0027 will contain any failure information\", any failure information is not a clear term defining what is included in \u0027flow_failures\u0027.  I skimed through taskflow\u0027s source code, didn\u0027t find more clear clue. If you can help me with more pointers, that will be helplful.","commit_id":"6ce41da61f50dbf6c48253ecd7803dd707b06608"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"b870be1e86f08f9e94331d8a7b0187ca9f4570d5","unresolved":false,"context_lines":[{"line_number":197,"context_line":"            return"},{"line_number":198,"context_line":"        # If volume in re-scheduling process, we should not change the volume\u0027s"},{"line_number":199,"context_line":"        # status to error."},{"line_number":200,"context_line":"        cause \u003d list(kwargs[\u0027flow_failures\u0027].values())[0]"},{"line_number":201,"context_line":"        # If not allow reschedule, the exception do not have attribute"},{"line_number":202,"context_line":"        # \u0027rescheduled\u0027. So catch the exception and just pass."},{"line_number":203,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5afe65bd_bc37a2e2","line":200,"in_reply_to":"5afe65bd_1c9a3b15","updated":"2015-06-02 04:05:28.000000000","message":"@Zhiteng, I read the taksflow\u0027s source code too and found the \u0027flow_failures\u0027 was set at \u0027https://github.com/openstack/taskflow/blob/master/taskflow/engines/action_engine/executor.py#L80\u0027 when executor call the revert_task method. And the \u0027failures\u0027 came from \n\u0027https://github.com/openstack/taskflow/blob/master/taskflow/engines/action_engine/actions/task.py#L134\u0027 at schedule_reversion method.\n\nLook at the get_failures method, it is just self._failures.copy() at Storage class. The _failures is set at \u0027https://github.com/openstack/taskflow/blob/master/taskflow/storage.py#L450\u0027, that save the task result I think.\n\nI\u0027m not familiar with TaskFlow, but hope those will help you.","commit_id":"6ce41da61f50dbf6c48253ecd7803dd707b06608"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"4fa8ed9fc3ca4368f37792f205a2f28e9405bb78","unresolved":false,"context_lines":[{"line_number":197,"context_line":"            return"},{"line_number":198,"context_line":"        # If volume in re-scheduling process, we should not change the volume\u0027s"},{"line_number":199,"context_line":"        # status to error."},{"line_number":200,"context_line":"        cause \u003d list(kwargs[\u0027flow_failures\u0027].values())[0]"},{"line_number":201,"context_line":"        # If not allow reschedule, the exception do not have attribute"},{"line_number":202,"context_line":"        # \u0027rescheduled\u0027. So catch the exception and just pass."},{"line_number":203,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a016987_49d413bc","line":200,"in_reply_to":"7a016987_239fb951","updated":"2015-05-28 11:10:47.000000000","message":"Reading that comment again - it may seem rude, sorry about that. I just wanted to highlight that TaskFlow\u0027s documentation is well written IMO. :)","commit_id":"6ce41da61f50dbf6c48253ecd7803dd707b06608"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"ef1b58b88b9958745c1e29881f09692780c659f1","unresolved":false,"context_lines":[{"line_number":197,"context_line":"            return"},{"line_number":198,"context_line":"        # If volume in re-scheduling process, we should not change the volume\u0027s"},{"line_number":199,"context_line":"        # status to error."},{"line_number":200,"context_line":"        cause \u003d list(kwargs[\u0027flow_failures\u0027].values())[0]"},{"line_number":201,"context_line":"        # If not allow reschedule, the exception do not have attribute"},{"line_number":202,"context_line":"        # \u0027rescheduled\u0027. So catch the exception and just pass."},{"line_number":203,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5afe65bd_fcfd1f6f","line":200,"in_reply_to":"7a016987_49d413bc","updated":"2015-06-02 00:47:00.000000000","message":"@Zhiteng and Michal, Honestly, it\u0027s harder to understand the code than before with TaskFlow IMO. But I think we shouldn\u0027t decide whether to remove it yet, right? And according the TaskFlow\u0027s doc, the \u0027flow_failures\u0027 will be part of kwargs. So this issue still need to be fixed ASAP.","commit_id":"6ce41da61f50dbf6c48253ecd7803dd707b06608"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"9649ae0c4fa0e85b810d2de5c5712098b175730f","unresolved":false,"context_lines":[{"line_number":197,"context_line":"            return"},{"line_number":198,"context_line":"        # If volume in re-scheduling process, we should not change the volume\u0027s"},{"line_number":199,"context_line":"        # status to error."},{"line_number":200,"context_line":"        cause \u003d list(kwargs[\u0027flow_failures\u0027].values())[0]"},{"line_number":201,"context_line":"        # If not allow reschedule, the exception do not have attribute"},{"line_number":202,"context_line":"        # \u0027rescheduled\u0027. So catch the exception and just pass."},{"line_number":203,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a016987_239fb951","line":200,"in_reply_to":"7a016987_869c465e","updated":"2015-05-28 08:46:01.000000000","message":"Here you go: http://docs.openstack.org/developer/taskflow/atoms.html#taskflow.task.BaseTask.revert\n\nI\u0027ve found that using \"flow_failures TaskFlow\" Google query.","commit_id":"6ce41da61f50dbf6c48253ecd7803dd707b06608"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"fe8da2c63ef76dadbe57473d24358eb4da6a5301","unresolved":false,"context_lines":[{"line_number":201,"context_line":"        # If not allow reschedule, the exception do not have attribute"},{"line_number":202,"context_line":"        # \u0027rescheduled\u0027. So catch the exception and just pass."},{"line_number":203,"context_line":"        try:"},{"line_number":204,"context_line":"            if cause.exception.rescheduled:"},{"line_number":205,"context_line":"                return"},{"line_number":206,"context_line":"        except Exception:"},{"line_number":207,"context_line":"            pass"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a016987_06234457","line":204,"updated":"2015-06-01 10:55:20.000000000","message":"Can we use hasattr (instead of try..except) which is more cleaner?","commit_id":"6ce41da61f50dbf6c48253ecd7803dd707b06608"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"ef1b58b88b9958745c1e29881f09692780c659f1","unresolved":false,"context_lines":[{"line_number":201,"context_line":"        # If not allow reschedule, the exception do not have attribute"},{"line_number":202,"context_line":"        # \u0027rescheduled\u0027. So catch the exception and just pass."},{"line_number":203,"context_line":"        try:"},{"line_number":204,"context_line":"            if cause.exception.rescheduled:"},{"line_number":205,"context_line":"                return"},{"line_number":206,"context_line":"        except Exception:"},{"line_number":207,"context_line":"            pass"}],"source_content_type":"text/x-python","patch_set":2,"id":"5afe65bd_dc5b832d","line":204,"in_reply_to":"7a016987_06234457","updated":"2015-06-02 00:47:00.000000000","message":"Done","commit_id":"6ce41da61f50dbf6c48253ecd7803dd707b06608"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"3fdd34ce991efae5a66336435e32c9824d320a66","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        # \u0027rescheduled\u0027. So just pass."},{"line_number":203,"context_line":"        if (hasattr(cause.exception, \u0027rescheduled\u0027)"},{"line_number":204,"context_line":"                and cause.exception.rescheduled):"},{"line_number":205,"context_line":"            return"},{"line_number":206,"context_line":"        common.error_out_volume(context, self.db, volume_id)"},{"line_number":207,"context_line":"        LOG.error(_LE(\"Volume %s: create failed\"), volume_id)"},{"line_number":208,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5afe65bd_b82c41ac","line":205,"updated":"2015-06-02 12:50:48.000000000","message":"This mix of lower level taskflow detail in the business logic of the revert method cries out for an encapsulation method, perhaps _volume_is_rescheduling or some such.  One ought to be able to read the revert method without having to think about the kinds of questions Winston was asking, and then if needed, look into the _volume_is_rescheduling (or whatever), see that it involves an implementation with taskflow, and then ask those questions :-)","commit_id":"9b3b5914543e78565ba1ea81dd61ac4bc0296c40"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"06303e7a0cf9f25ffe24824ef2d851c078dec427","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        # \u0027rescheduled\u0027. So just pass."},{"line_number":203,"context_line":"        if (hasattr(cause.exception, \u0027rescheduled\u0027)"},{"line_number":204,"context_line":"                and cause.exception.rescheduled):"},{"line_number":205,"context_line":"            return"},{"line_number":206,"context_line":"        common.error_out_volume(context, self.db, volume_id)"},{"line_number":207,"context_line":"        LOG.error(_LE(\"Volume %s: create failed\"), volume_id)"},{"line_number":208,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5afe65bd_7602390d","line":205,"in_reply_to":"5afe65bd_b82c41ac","updated":"2015-06-03 04:05:27.000000000","message":"Emm, I\u0027m not sure get your all point, Tom. But I will have a try to extract those code as a private method. If there is any mistake I understand, please make a comment in next patch.:)","commit_id":"9b3b5914543e78565ba1ea81dd61ac4bc0296c40"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"bcc2465499dd4ab6fdcde72137dc8b98d9cf8993","unresolved":false,"context_lines":[{"line_number":199,"context_line":"        if (hasattr(cause.exception, \u0027rescheduled\u0027)"},{"line_number":200,"context_line":"                and cause.exception.rescheduled):"},{"line_number":201,"context_line":"            return True"},{"line_number":202,"context_line":"        return False"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"    def revert(self, context, volume_id, result, **kwargs):"},{"line_number":205,"context_line":"        if isinstance(result, ft.Failure):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5afe65bd_ea318d0c","line":202,"updated":"2015-06-03 10:35:38.000000000","message":"Could be simplified to:\nreturn getattr(cause.exception, \u0027rescheduled\u0027, False)","commit_id":"2bd9f087906800be74185679b507ea0e4dd6e36c"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"967e40c8337cad5c520a19fbcce8e7ccba979b2c","unresolved":false,"context_lines":[{"line_number":199,"context_line":"        if (hasattr(cause.exception, \u0027rescheduled\u0027)"},{"line_number":200,"context_line":"                and cause.exception.rescheduled):"},{"line_number":201,"context_line":"            return True"},{"line_number":202,"context_line":"        return False"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"    def revert(self, context, volume_id, result, **kwargs):"},{"line_number":205,"context_line":"        if isinstance(result, ft.Failure):"}],"source_content_type":"text/x-python","patch_set":4,"id":"5afe65bd_5e64c247","line":202,"in_reply_to":"5afe65bd_ea318d0c","updated":"2015-06-04 02:35:46.000000000","message":"Sure.","commit_id":"2bd9f087906800be74185679b507ea0e4dd6e36c"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"ede21ba51453e5a125480c67597e4710ededb626","unresolved":false,"context_lines":[{"line_number":206,"context_line":"            return"},{"line_number":207,"context_line":"        # If volume in re-scheduling process, we should not change the volume\u0027s"},{"line_number":208,"context_line":"        # status to error."},{"line_number":209,"context_line":"        if self._volume_is_rescheduling(**kwargs):"},{"line_number":210,"context_line":"            return"},{"line_number":211,"context_line":"        common.error_out_volume(context, self.db, volume_id)"},{"line_number":212,"context_line":"        LOG.error(_LE(\"Volume %s: create failed\"), volume_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5afe65bd_739a2765","line":209,"updated":"2015-06-03 17:58:17.000000000","message":"I agree with Vipin, kwargs[\u0027flow_failures\u0027].values() is taskflow details. otherwise why not just pass kwargs[\u0027flow_failures\u0027].values()[0].exception ?","commit_id":"2bd9f087906800be74185679b507ea0e4dd6e36c"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"bcc2465499dd4ab6fdcde72137dc8b98d9cf8993","unresolved":false,"context_lines":[{"line_number":206,"context_line":"            return"},{"line_number":207,"context_line":"        # If volume in re-scheduling process, we should not change the volume\u0027s"},{"line_number":208,"context_line":"        # status to error."},{"line_number":209,"context_line":"        if self._volume_is_rescheduling(**kwargs):"},{"line_number":210,"context_line":"            return"},{"line_number":211,"context_line":"        common.error_out_volume(context, self.db, volume_id)"},{"line_number":212,"context_line":"        LOG.error(_LE(\"Volume %s: create failed\"), volume_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5afe65bd_eabced6d","line":209,"updated":"2015-06-03 10:35:38.000000000","message":"We should pass only kwargs[\u0027flow_failures\u0027].values() to _volume_is_rescheduling method","commit_id":"2bd9f087906800be74185679b507ea0e4dd6e36c"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"bdb6563a2f9bb6518d41452312add74fa7f7be23","unresolved":false,"context_lines":[{"line_number":206,"context_line":"            return"},{"line_number":207,"context_line":"        # If volume in re-scheduling process, we should not change the volume\u0027s"},{"line_number":208,"context_line":"        # status to error."},{"line_number":209,"context_line":"        if self._volume_is_rescheduling(**kwargs):"},{"line_number":210,"context_line":"            return"},{"line_number":211,"context_line":"        common.error_out_volume(context, self.db, volume_id)"},{"line_number":212,"context_line":"        LOG.error(_LE(\"Volume %s: create failed\"), volume_id)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5afe65bd_c42365a6","line":209,"in_reply_to":"5afe65bd_eabced6d","updated":"2015-06-03 11:38:00.000000000","message":"_volume_is_rescheduling is meant to hide the taskflow related details. The logic was extracted to a new method (_volume_is_rescheduling) as part of addressing a review comment. Therefore, IMO, passing kwargs is fine.","commit_id":"2bd9f087906800be74185679b507ea0e4dd6e36c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"97a967026c1651949422682f933ce75ae61c0677","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        return volume_ref"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def _volume_is_rescheduling(self, **kwargs):"},{"line_number":196,"context_line":"        cause \u003d list(kwargs[\u0027flow_failures\u0027].values())[0]"},{"line_number":197,"context_line":"        # If not allow reschedule, the exception do not have attribute"},{"line_number":198,"context_line":"        # \u0027rescheduled\u0027. So just return False."},{"line_number":199,"context_line":"        return getattr(cause.exception, \u0027rescheduled\u0027, False)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3afb71cf_4e26ab69","line":196,"updated":"2015-06-05 14:57:04.000000000","message":"If we are going to access directly flow_failures (not with dictionary\u0027s get method)  we should add it as an argument to this method instead of accessing it in the dictionary.","commit_id":"34a5b3db80693ddd0d3d849a6cc8ad04fd9f84fb"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"2839861bf757458844f15683e369ca759d64213f","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        return volume_ref"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def _volume_is_rescheduling(self, **kwargs):"},{"line_number":196,"context_line":"        cause \u003d list(kwargs[\u0027flow_failures\u0027].values())[0]"},{"line_number":197,"context_line":"        # If not allow reschedule, the exception do not have attribute"},{"line_number":198,"context_line":"        # \u0027rescheduled\u0027. So just return False."},{"line_number":199,"context_line":"        return getattr(cause.exception, \u0027rescheduled\u0027, False)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3afb71cf_61bba373","line":196,"in_reply_to":"3afb71cf_4e26ab69","updated":"2015-06-06 09:27:53.000000000","message":"There has a discussion about this argument in patch 4. That we think _volume_is_rescheduling is meant to hide the taskflow related details. So should not pass the flow_failures as argument.","commit_id":"34a5b3db80693ddd0d3d849a6cc8ad04fd9f84fb"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"54a4607f7c92538b28a4686608ad955cd0e80a79","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        return volume_ref"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def _volume_is_rescheduling(self, **kwargs):"},{"line_number":196,"context_line":"        cause \u003d list(kwargs[\u0027flow_failures\u0027].values())[0]"},{"line_number":197,"context_line":"        # If not allow reschedule, the exception do not have attribute"},{"line_number":198,"context_line":"        # \u0027rescheduled\u0027. So just return False."},{"line_number":199,"context_line":"        return getattr(cause.exception, \u0027rescheduled\u0027, False)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3afb71cf_02ba9b6e","line":196,"in_reply_to":"3afb71cf_61bba373","updated":"2015-06-08 07:57:16.000000000","message":"That discussion was different. On that one they were talking about passing kwargs[\u0027flow_failures\u0027].values() and it was said that the idea of this method was to hide taskflow details, but since this method know about such details it can make use of that information to be neater:\n\n def _volume_is_rescheduling(self, flow_failures, **kwargs):\n     cause \u003d flow_failures.values()[0]\n     return getattr(cause.exception, \u0027rescheduled\u0027, False)","commit_id":"34a5b3db80693ddd0d3d849a6cc8ad04fd9f84fb"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"97a967026c1651949422682f933ce75ae61c0677","unresolved":false,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def _volume_is_rescheduling(self, **kwargs):"},{"line_number":196,"context_line":"        cause \u003d list(kwargs[\u0027flow_failures\u0027].values())[0]"},{"line_number":197,"context_line":"        # If not allow reschedule, the exception do not have attribute"},{"line_number":198,"context_line":"        # \u0027rescheduled\u0027. So just return False."},{"line_number":199,"context_line":"        return getattr(cause.exception, \u0027rescheduled\u0027, False)"},{"line_number":200,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3afb71cf_2ee24f04","line":197,"updated":"2015-06-05 14:57:04.000000000","message":"s/allow/allowing\n\ns/do not/does not","commit_id":"34a5b3db80693ddd0d3d849a6cc8ad04fd9f84fb"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"2839861bf757458844f15683e369ca759d64213f","unresolved":false,"context_lines":[{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def _volume_is_rescheduling(self, **kwargs):"},{"line_number":196,"context_line":"        cause \u003d list(kwargs[\u0027flow_failures\u0027].values())[0]"},{"line_number":197,"context_line":"        # If not allow reschedule, the exception do not have attribute"},{"line_number":198,"context_line":"        # \u0027rescheduled\u0027. So just return False."},{"line_number":199,"context_line":"        return getattr(cause.exception, \u0027rescheduled\u0027, False)"},{"line_number":200,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3afb71cf_c1637719","line":197,"in_reply_to":"3afb71cf_2ee24f04","updated":"2015-06-06 09:27:53.000000000","message":"Done","commit_id":"34a5b3db80693ddd0d3d849a6cc8ad04fd9f84fb"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"76599323f3cb4b4f3bc3c70a15a715e4b7f25582","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        # Use a different context when rescheduling."},{"line_number":164,"context_line":"        if self.reschedule_context:"},{"line_number":165,"context_line":"            context \u003d self.reschedule_context"},{"line_number":166,"context_line":"            try:"},{"line_number":167,"context_line":"                self._pre_reschedule(context, volume_id)"},{"line_number":168,"context_line":"                self._reschedule(context, cause, **kwargs)"},{"line_number":169,"context_line":"                self._post_reschedule(context, volume_id)"}],"source_content_type":"text/x-python","patch_set":12,"id":"1af86dd1_36ece2c6","line":166,"updated":"2015-06-12 17:38:47.000000000","message":"Shouldn\u0027t this whole _pre_reschedule be removed? We don\u0027t need to change to \u0027creating\u0027 state anymore since we haven\u0027t changed it to error.","commit_id":"7d85adeee498ee8e69dde14da41996a46387c501"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"b3ab7dfb4cc3e3d76c72c0806d17d59f14dc3b89","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        # Use a different context when rescheduling."},{"line_number":164,"context_line":"        if self.reschedule_context:"},{"line_number":165,"context_line":"            context \u003d self.reschedule_context"},{"line_number":166,"context_line":"            try:"},{"line_number":167,"context_line":"                self._pre_reschedule(context, volume_id)"},{"line_number":168,"context_line":"                self._reschedule(context, cause, **kwargs)"},{"line_number":169,"context_line":"                self._post_reschedule(context, volume_id)"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa32b979_d276f03d","line":166,"in_reply_to":"1af86dd1_36ece2c6","updated":"2015-06-15 08:12:12.000000000","message":"I agree, but resetting the host and setting scheduled_at need to be done somewhere. Scheduler maybe?","commit_id":"7d85adeee498ee8e69dde14da41996a46387c501"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"ab4b13c55932517999b7485d0a8bb0f048b27dbd","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        # Use a different context when rescheduling."},{"line_number":164,"context_line":"        if self.reschedule_context:"},{"line_number":165,"context_line":"            context \u003d self.reschedule_context"},{"line_number":166,"context_line":"            try:"},{"line_number":167,"context_line":"                self._pre_reschedule(context, volume_id)"},{"line_number":168,"context_line":"                self._reschedule(context, cause, **kwargs)"},{"line_number":169,"context_line":"                self._post_reschedule(context, volume_id)"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa32b979_12c8a321","line":166,"in_reply_to":"fa32b979_d276f03d","updated":"2015-06-16 08:50:09.000000000","message":"It\u0027s not much differences and benefits to scheduler, just remove updating status is fine IMHO.","commit_id":"7d85adeee498ee8e69dde14da41996a46387c501"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"76599323f3cb4b4f3bc3c70a15a715e4b7f25582","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        cause \u003d list(kwargs[\u0027flow_failures\u0027].values())[0]"},{"line_number":197,"context_line":"        # If not allowing reschedule, the exception does not have attribute"},{"line_number":198,"context_line":"        # \u0027rescheduled\u0027. So just return False."},{"line_number":199,"context_line":"        return getattr(cause.exception, \u0027rescheduled\u0027, False)"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    def revert(self, context, volume_id, result, **kwargs):"},{"line_number":202,"context_line":"        if isinstance(result, ft.Failure):"}],"source_content_type":"text/x-python","patch_set":12,"id":"1af86dd1_96ba76e0","line":199,"updated":"2015-06-12 17:38:47.000000000","message":"I just realized that there are other case were it may not be rescheduled, like when any of these exceptions are thrown:\n          exception.ImageCopyFailure,                                         \n          exception.MetadataCopyFailure,                                      \n          exception.MetadataCreateFailure,                                    \n          exception.MetadataUpdateFailure,                                    \n          exception.VolumeNotFound,                                           \n          exception.SnapshotNotFound,                                         \n          exception.VolumeTypeNotFound,                                       \n          exception.ImageUnacceptable,\n\nAnd in any of those cases we would end up with a final result of \u0027creating\u0027, instead of error.","commit_id":"7d85adeee498ee8e69dde14da41996a46387c501"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"b3ab7dfb4cc3e3d76c72c0806d17d59f14dc3b89","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        cause \u003d list(kwargs[\u0027flow_failures\u0027].values())[0]"},{"line_number":197,"context_line":"        # If not allowing reschedule, the exception does not have attribute"},{"line_number":198,"context_line":"        # \u0027rescheduled\u0027. So just return False."},{"line_number":199,"context_line":"        return getattr(cause.exception, \u0027rescheduled\u0027, False)"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    def revert(self, context, volume_id, result, **kwargs):"},{"line_number":202,"context_line":"        if isinstance(result, ft.Failure):"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa32b979_f2c1b42f","line":199,"in_reply_to":"1af86dd1_96ba76e0","updated":"2015-06-15 08:12:12.000000000","message":"This isn\u0027t trivial, but actually checking the exception for rescheduled attribute that was injected covers these cases.","commit_id":"7d85adeee498ee8e69dde14da41996a46387c501"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"aa9c6a2c8f9d1f9baa33d51e2deb76e330e53ada","unresolved":false,"context_lines":[{"line_number":196,"context_line":"        cause \u003d list(kwargs[\u0027flow_failures\u0027].values())[0]"},{"line_number":197,"context_line":"        # If not allowing reschedule, the exception does not have attribute"},{"line_number":198,"context_line":"        # \u0027rescheduled\u0027. So just return False."},{"line_number":199,"context_line":"        return getattr(cause.exception, \u0027rescheduled\u0027, False)"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    def revert(self, context, volume_id, result, **kwargs):"},{"line_number":202,"context_line":"        if isinstance(result, ft.Failure):"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa32b979_ed00f5ac","line":199,"in_reply_to":"fa32b979_f2c1b42f","updated":"2015-06-15 08:36:52.000000000","message":"I mentioned what I think could be a nice solution in a patch comment:\n\n\u003e To resolve issue commented by Michal and the one with volumes left as creating that I say in my comment, I think it would be better to always include OnFailureRescheduleTask in the creation flow and pass it a do_reschedule argument specifying if it should really reschedule or not (this creation argument whold be `allow_reschedule and request_spec and retry`) and remove revert from ExtractVolumeRefTask and do the error state change in OnFailureRescheduleTask.\n\n\u003e That way we can remove the line Michal mentioned as well as fix the issue of leaving the volume in \u0027creating\u0027 and changes in states between retries.","commit_id":"7d85adeee498ee8e69dde14da41996a46387c501"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"08fd7e825660b7b0505f3be62e94b47c86e7bc39","unresolved":false,"context_lines":[{"line_number":633,"context_line":"            LOG.exception(_LE(\"Unable to create volume. \""},{"line_number":634,"context_line":"                              \"Volume driver %s not initialized\"), driver_name)"},{"line_number":635,"context_line":"            # NOTE(flaper87): Set the error status before"},{"line_number":636,"context_line":"            # raising any exception."},{"line_number":637,"context_line":"            self.db.volume_update(context, volume_id, dict(status\u003d\u0027error\u0027))"},{"line_number":638,"context_line":"            raise exception.DriverNotInitialized()"},{"line_number":639,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"1af86dd1_b2f6a7d5","line":636,"updated":"2015-06-12 13:51:30.000000000","message":"In case of failing this check volume still get errored out between reschedules. I wonder why we have this update there in the first place...?","commit_id":"7d85adeee498ee8e69dde14da41996a46387c501"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"706316af84eebd10f28746b4ed6e8c2227b20ea5","unresolved":false,"context_lines":[{"line_number":633,"context_line":"            LOG.exception(_LE(\"Unable to create volume. \""},{"line_number":634,"context_line":"                              \"Volume driver %s not initialized\"), driver_name)"},{"line_number":635,"context_line":"            # NOTE(flaper87): Set the error status before"},{"line_number":636,"context_line":"            # raising any exception."},{"line_number":637,"context_line":"            self.db.volume_update(context, volume_id, dict(status\u003d\u0027error\u0027))"},{"line_number":638,"context_line":"            raise exception.DriverNotInitialized()"},{"line_number":639,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"1af86dd1_73eee78c","line":636,"updated":"2015-06-12 18:57:25.000000000","message":"This exception should be allow to reschedule because there is no way for volume manager to figure out whether there are any other candidates can serve the same request.  But to be realisitic, if volume driver isn\u0027t properly initialized, the volume manager won\u0027t send status update to scheduler, so scheduler won\u0027t pick up this backend to serve, except the cached status for the same backend was still valid for a short period of time.  Anyway, thus exception should still allow reschedule.","commit_id":"7d85adeee498ee8e69dde14da41996a46387c501"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"25d5efba97b7dfe5d8229c0b7b9182908e512d5e","unresolved":false,"context_lines":[{"line_number":633,"context_line":"            LOG.exception(_LE(\"Unable to create volume. \""},{"line_number":634,"context_line":"                              \"Volume driver %s not initialized\"), driver_name)"},{"line_number":635,"context_line":"            # NOTE(flaper87): Set the error status before"},{"line_number":636,"context_line":"            # raising any exception."},{"line_number":637,"context_line":"            self.db.volume_update(context, volume_id, dict(status\u003d\u0027error\u0027))"},{"line_number":638,"context_line":"            raise exception.DriverNotInitialized()"},{"line_number":639,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"fa32b979_dfa934e1","line":636,"in_reply_to":"1af86dd1_73eee78c","updated":"2015-06-13 08:21:27.000000000","message":"You are right, it should not be added to no_reschedule_types, but the DB update can still be removed.","commit_id":"7d85adeee498ee8e69dde14da41996a46387c501"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"df0ccd988d93843adcd38fd47bf58631241d90fd","unresolved":false,"context_lines":[{"line_number":633,"context_line":"            LOG.exception(_LE(\"Unable to create volume. \""},{"line_number":634,"context_line":"                              \"Volume driver %s not initialized\"), driver_name)"},{"line_number":635,"context_line":"            # NOTE(flaper87): Set the error status before"},{"line_number":636,"context_line":"            # raising any exception."},{"line_number":637,"context_line":"            self.db.volume_update(context, volume_id, dict(status\u003d\u0027error\u0027))"},{"line_number":638,"context_line":"            raise exception.DriverNotInitialized()"},{"line_number":639,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"1af86dd1_d25ec344","line":636,"in_reply_to":"1af86dd1_b2f6a7d5","updated":"2015-06-12 14:05:34.000000000","message":"https://review.openstack.org/#/c/61088/ - this commit introduced it. I think the status change line can be safely removed.","commit_id":"7d85adeee498ee8e69dde14da41996a46387c501"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"76599323f3cb4b4f3bc3c70a15a715e4b7f25582","unresolved":false,"context_lines":[{"line_number":633,"context_line":"            LOG.exception(_LE(\"Unable to create volume. \""},{"line_number":634,"context_line":"                              \"Volume driver %s not initialized\"), driver_name)"},{"line_number":635,"context_line":"            # NOTE(flaper87): Set the error status before"},{"line_number":636,"context_line":"            # raising any exception."},{"line_number":637,"context_line":"            self.db.volume_update(context, volume_id, dict(status\u003d\u0027error\u0027))"},{"line_number":638,"context_line":"            raise exception.DriverNotInitialized()"},{"line_number":639,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"1af86dd1_5b354744","line":636,"in_reply_to":"1af86dd1_d25ec344","updated":"2015-06-12 17:38:47.000000000","message":"Nice catch!\n\nI agree, this looks like it can be removed and maybe this exception should be added to OnFailureRescheduleTask.no_reschedule_types","commit_id":"7d85adeee498ee8e69dde14da41996a46387c501"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"96ab80da138a39317ee41ce650b35ae03f15e1e5","unresolved":false,"context_lines":[{"line_number":633,"context_line":"            LOG.exception(_LE(\"Unable to create volume. \""},{"line_number":634,"context_line":"                              \"Volume driver %s not initialized\"), driver_name)"},{"line_number":635,"context_line":"            # NOTE(flaper87): Set the error status before"},{"line_number":636,"context_line":"            # raising any exception."},{"line_number":637,"context_line":"            self.db.volume_update(context, volume_id, dict(status\u003d\u0027error\u0027))"},{"line_number":638,"context_line":"            raise exception.DriverNotInitialized()"},{"line_number":639,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"1af86dd1_812b5282","line":636,"in_reply_to":"1af86dd1_d25ec344","updated":"2015-06-12 17:34:42.000000000","message":"That\u0027s a really a good catch.  I think back to the time when the author did this change, he and the reviewer forgot about rescheduling.  That\u0027s why they did this status update.  I agree we should remove it.","commit_id":"7d85adeee498ee8e69dde14da41996a46387c501"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"7f4c85975ceeac463d51f46685f3c40f4c35f01b","unresolved":false,"context_lines":[{"line_number":46,"context_line":")"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"class OnFailureRescheduleTask(flow_utils.CinderTask):"},{"line_number":50,"context_line":"    \"\"\"Triggers a rescheduling request to be sent when reverting occurs."},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    Reversion strategy: Triggers the rescheduling mechanism whereby a cast gets"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa32b979_34634fde","line":49,"updated":"2015-06-16 11:45:34.000000000","message":"I wonder if we shouldn\u0027t rename this now? Anyone have opinion on that?","commit_id":"f7e11ef509c93f0a7add173f15eb0ee4bb310978"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"c7158e7397de856bf24b96b2fd0e3ac87bcd6ce0","unresolved":false,"context_lines":[{"line_number":46,"context_line":")"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"class OnFailureRescheduleTask(flow_utils.CinderTask):"},{"line_number":50,"context_line":"    \"\"\"Triggers a rescheduling request to be sent when reverting occurs."},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    Reversion strategy: Triggers the rescheduling mechanism whereby a cast gets"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa32b979_9e380af1","line":49,"in_reply_to":"fa32b979_05e93014","updated":"2015-06-17 07:02:03.000000000","message":"Okay, right, the only thing that changed is now here we\u0027re erroring our the volume.","commit_id":"f7e11ef509c93f0a7add173f15eb0ee4bb310978"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"fbb1a40b5f1f40028634349fa95dcda1e19a2a04","unresolved":false,"context_lines":[{"line_number":46,"context_line":")"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"class OnFailureRescheduleTask(flow_utils.CinderTask):"},{"line_number":50,"context_line":"    \"\"\"Triggers a rescheduling request to be sent when reverting occurs."},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    Reversion strategy: Triggers the rescheduling mechanism whereby a cast gets"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa32b979_05e93014","line":49,"in_reply_to":"fa32b979_34634fde","updated":"2015-06-16 11:54:30.000000000","message":"The name is still valid, even if it doesn\u0027t always reschedule it\u0027s still the on failure rescheduler task. Some times it\u0027ll reschedule 3 times, other 1 and other none. It\u0027s not like the name is OnFailureAlwasyRescheduleTask. ;-)\n\nAlthough I wouldn\u0027t be against a new name, like ReschedulerTask or something else.","commit_id":"f7e11ef509c93f0a7add173f15eb0ee4bb310978"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a77aaae19752a01a4973fc57c335d7c61188ac4f","unresolved":false,"context_lines":[{"line_number":134,"context_line":"            # sent to the scheduler? (since what happens if it never gets"},{"line_number":135,"context_line":"            # there??). It\u0027s almost like we need a status of \u0027on-the-way-to"},{"line_number":136,"context_line":"            # scheduler\u0027 in the future."},{"line_number":137,"context_line":"            # We don\u0027t need to update the volume\u0027s status to error, since"},{"line_number":138,"context_line":"            # we haven\u0027t changed it to error."},{"line_number":139,"context_line":"            update \u003d {"},{"line_number":140,"context_line":"                \u0027scheduled_at\u0027: timeutils.utcnow(),"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa32b979_5cf639d8","line":137,"updated":"2015-06-16 10:59:09.000000000","message":"s/error/creating","commit_id":"f7e11ef509c93f0a7add173f15eb0ee4bb310978"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"5879f1ea848747f11b8838abe7dd0bde4474f402","unresolved":false,"context_lines":[{"line_number":134,"context_line":"            # sent to the scheduler? (since what happens if it never gets"},{"line_number":135,"context_line":"            # there??). It\u0027s almost like we need a status of \u0027on-the-way-to"},{"line_number":136,"context_line":"            # scheduler\u0027 in the future."},{"line_number":137,"context_line":"            # We don\u0027t need to update the volume\u0027s status to error, since"},{"line_number":138,"context_line":"            # we haven\u0027t changed it to error."},{"line_number":139,"context_line":"            update \u003d {"},{"line_number":140,"context_line":"                \u0027scheduled_at\u0027: timeutils.utcnow(),"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa32b979_25a5ed45","line":137,"in_reply_to":"fa32b979_5cf639d8","updated":"2015-06-18 09:36:59.000000000","message":"Done","commit_id":"f7e11ef509c93f0a7add173f15eb0ee4bb310978"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a77aaae19752a01a4973fc57c335d7c61188ac4f","unresolved":false,"context_lines":[{"line_number":155,"context_line":"        # TaskFlow to support better way of returning data from reverted flow."},{"line_number":156,"context_line":"        volume_id \u003d kwargs[\u0027volume_id\u0027]"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"        if isinstance(result, ft.Failure):"},{"line_number":159,"context_line":"            return"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        # If do not want to be rescheduled, just set the volume\u0027s status to"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa32b979_7cff7d67","line":158,"updated":"2015-06-16 10:59:09.000000000","message":"This should not be here, otherwise you wouldn\u0027t change status to error.","commit_id":"f7e11ef509c93f0a7add173f15eb0ee4bb310978"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"474480bfa2c9886bd5bc3bff87395bd762b7ea28","unresolved":false,"context_lines":[{"line_number":155,"context_line":"        # TaskFlow to support better way of returning data from reverted flow."},{"line_number":156,"context_line":"        volume_id \u003d kwargs[\u0027volume_id\u0027]"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"        if isinstance(result, ft.Failure):"},{"line_number":159,"context_line":"            return"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        # If do not want to be rescheduled, just set the volume\u0027s status to"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa32b979_6681f8c2","line":158,"updated":"2015-06-17 08:32:10.000000000","message":"Would this be true at all?  The execute() should always succeed because it has nothing.","commit_id":"f7e11ef509c93f0a7add173f15eb0ee4bb310978"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"5879f1ea848747f11b8838abe7dd0bde4474f402","unresolved":false,"context_lines":[{"line_number":155,"context_line":"        # TaskFlow to support better way of returning data from reverted flow."},{"line_number":156,"context_line":"        volume_id \u003d kwargs[\u0027volume_id\u0027]"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"        if isinstance(result, ft.Failure):"},{"line_number":159,"context_line":"            return"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        # If do not want to be rescheduled, just set the volume\u0027s status to"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa32b979_a0148b4c","line":158,"in_reply_to":"fa32b979_6681f8c2","updated":"2015-06-18 09:36:59.000000000","message":"Emm, Zhiteng is right I think. Maybe we don\u0027t need this?","commit_id":"f7e11ef509c93f0a7add173f15eb0ee4bb310978"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a77aaae19752a01a4973fc57c335d7c61188ac4f","unresolved":false,"context_lines":[{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        # If do not want to be rescheduled, just set the volume\u0027s status to"},{"line_number":162,"context_line":"        # error and return."},{"line_number":163,"context_line":"        if not self.do_reschedule:"},{"line_number":164,"context_line":"            common.error_out_volume(context, self.db, volume_id)"},{"line_number":165,"context_line":"            LOG.error(_LE(\"Volume %s: create failed\"), volume_id)"},{"line_number":166,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa32b979_3c2815e2","line":163,"updated":"2015-06-16 10:59:09.000000000","message":"if not self.do_reschedule or isinstance(result, ft.Failure):","commit_id":"f7e11ef509c93f0a7add173f15eb0ee4bb310978"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"5879f1ea848747f11b8838abe7dd0bde4474f402","unresolved":false,"context_lines":[{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        # If do not want to be rescheduled, just set the volume\u0027s status to"},{"line_number":162,"context_line":"        # error and return."},{"line_number":163,"context_line":"        if not self.do_reschedule:"},{"line_number":164,"context_line":"            common.error_out_volume(context, self.db, volume_id)"},{"line_number":165,"context_line":"            LOG.error(_LE(\"Volume %s: create failed\"), volume_id)"},{"line_number":166,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa32b979_e0505334","line":163,"in_reply_to":"fa32b979_3c2815e2","updated":"2015-06-18 09:36:59.000000000","message":"Done","commit_id":"f7e11ef509c93f0a7add173f15eb0ee4bb310978"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a77aaae19752a01a4973fc57c335d7c61188ac4f","unresolved":false,"context_lines":[{"line_number":766,"context_line":"    retry \u003d filter_properties.get(\u0027retry\u0027, None)"},{"line_number":767,"context_line":""},{"line_number":768,"context_line":"    # Always add OnFailureRescheduleTask and we handle the change of volume\u0027s"},{"line_number":769,"context_line":"    # status when revert task flow. Meanwhile, no need revert process of"},{"line_number":770,"context_line":"    # ExtractVolumeRefTask."},{"line_number":771,"context_line":"    do_reschedule \u003d False"},{"line_number":772,"context_line":"    if allow_reschedule and request_spec and retry:"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa32b979_3c56d55c","line":769,"updated":"2015-06-16 10:59:09.000000000","message":"s/no need revert/no need to revert","commit_id":"f7e11ef509c93f0a7add173f15eb0ee4bb310978"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"5879f1ea848747f11b8838abe7dd0bde4474f402","unresolved":false,"context_lines":[{"line_number":766,"context_line":"    retry \u003d filter_properties.get(\u0027retry\u0027, None)"},{"line_number":767,"context_line":""},{"line_number":768,"context_line":"    # Always add OnFailureRescheduleTask and we handle the change of volume\u0027s"},{"line_number":769,"context_line":"    # status when revert task flow. Meanwhile, no need revert process of"},{"line_number":770,"context_line":"    # ExtractVolumeRefTask."},{"line_number":771,"context_line":"    do_reschedule \u003d False"},{"line_number":772,"context_line":"    if allow_reschedule and request_spec and retry:"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa32b979_0090d793","line":769,"in_reply_to":"fa32b979_3c56d55c","updated":"2015-06-18 09:36:59.000000000","message":"Done","commit_id":"f7e11ef509c93f0a7add173f15eb0ee4bb310978"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a77aaae19752a01a4973fc57c335d7c61188ac4f","unresolved":false,"context_lines":[{"line_number":770,"context_line":"    # ExtractVolumeRefTask."},{"line_number":771,"context_line":"    do_reschedule \u003d False"},{"line_number":772,"context_line":"    if allow_reschedule and request_spec and retry:"},{"line_number":773,"context_line":"        do_reschedule \u003d True"},{"line_number":774,"context_line":"    volume_flow.add(OnFailureRescheduleTask(reschedule_context, db,"},{"line_number":775,"context_line":"                                            scheduler_rpcapi,"},{"line_number":776,"context_line":"                                            do_reschedule))"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa32b979_dccb291d","line":773,"updated":"2015-06-16 10:59:09.000000000","message":"We can change lines 770 to 773 with:\n\n do_reschedule \u003d allow_reschedule and request_spec and retry","commit_id":"f7e11ef509c93f0a7add173f15eb0ee4bb310978"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"5879f1ea848747f11b8838abe7dd0bde4474f402","unresolved":false,"context_lines":[{"line_number":770,"context_line":"    # ExtractVolumeRefTask."},{"line_number":771,"context_line":"    do_reschedule \u003d False"},{"line_number":772,"context_line":"    if allow_reschedule and request_spec and retry:"},{"line_number":773,"context_line":"        do_reschedule \u003d True"},{"line_number":774,"context_line":"    volume_flow.add(OnFailureRescheduleTask(reschedule_context, db,"},{"line_number":775,"context_line":"                                            scheduler_rpcapi,"},{"line_number":776,"context_line":"                                            do_reschedule))"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa32b979_40e1ff2b","line":773,"in_reply_to":"fa32b979_dccb291d","updated":"2015-06-18 09:36:59.000000000","message":"Done","commit_id":"f7e11ef509c93f0a7add173f15eb0ee4bb310978"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"6ac7e247a25a92af656b892d3482ef0f4bf87a86","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        volume_ref \u003d self.db.volume_get(context, volume_id)"},{"line_number":193,"context_line":"        return volume_ref"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def revert(self, context, volume_id, result, **kwargs):"},{"line_number":196,"context_line":"        if isinstance(result, ft.Failure):"},{"line_number":197,"context_line":"            return"},{"line_number":198,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_782cfb98","side":"PARENT","line":195,"updated":"2015-06-18 10:18:51.000000000","message":"Awww, I hate to write that, but I noticed something just a few minutes ago.\n\nTake a look at cinder.volume.flows.manager.manage_existing. This file reuses ExtractVolumeRefTask. So if we\u0027ll change our task here then manage_existing flow will also change and won\u0027t error out volume on failure.\n\nI\u0027ve checked that and in tests there\u0027s no assert on volume state - that\u0027s why Jenkins is passing.\n\nI really hate how a simple fix is made difficult by complexity of this create code.","commit_id":"703ce63c1acd7c710dba867992ee8465022e1490"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"1ad56f1c3d42682a317da1720af810f9fbca3039","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        volume_ref \u003d self.db.volume_get(context, volume_id)"},{"line_number":193,"context_line":"        return volume_ref"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def revert(self, context, volume_id, result, **kwargs):"},{"line_number":196,"context_line":"        if isinstance(result, ft.Failure):"},{"line_number":197,"context_line":"            return"},{"line_number":198,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_fec23384","side":"PARENT","line":195,"in_reply_to":"fa32b979_782cfb98","updated":"2015-06-18 11:05:29.000000000","message":"Good catch!\n\nOk, then my suggestion, which is not that great is:\n\n - Add to this class\u0027 execute method an optional argument set_error\u003dTrue that is stored in the instance.\n - Then when we hit revert we only set DB error if set_error is True.\n - And we provide set_error \u003d False on OnFailureRescheduleTask\u0027s execute method.","commit_id":"703ce63c1acd7c710dba867992ee8465022e1490"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"f1ee66253a18383d1d1b9dc153169eb4cb555a46","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        volume_ref \u003d self.db.volume_get(context, volume_id)"},{"line_number":193,"context_line":"        return volume_ref"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def revert(self, context, volume_id, result, **kwargs):"},{"line_number":196,"context_line":"        if isinstance(result, ft.Failure):"},{"line_number":197,"context_line":"            return"},{"line_number":198,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_818e262c","side":"PARENT","line":195,"in_reply_to":"fa32b979_fec23384","updated":"2015-06-18 12:38:33.000000000","message":"Have a little confuse, can we change the self.set_error\u0027s value in here On OnFailureRescheduleTask\u0027s execute method? Let me have a try..","commit_id":"703ce63c1acd7c710dba867992ee8465022e1490"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"6ac7e247a25a92af656b892d3482ef0f4bf87a86","unresolved":false,"context_lines":[{"line_number":149,"context_line":"                          volume_id)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    def revert(self, context, result, flow_failures, **kwargs):"},{"line_number":152,"context_line":"        # NOTE(dulek): Revert is occurring and manager need to know if"},{"line_number":153,"context_line":"        # rescheduling happened. We\u0027re injecting this information into"},{"line_number":154,"context_line":"        # exception that will be caught there. This is ugly and we need"},{"line_number":155,"context_line":"        # TaskFlow to support better way of returning data from reverted flow."}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_16710144","line":152,"updated":"2015-06-18 10:18:51.000000000","message":"This comment need to be placed just before line 165. It\u0027s describing lines 165-166...","commit_id":"a2fa23972c3989066e5818f30913c35eac0979d1"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"f1ee66253a18383d1d1b9dc153169eb4cb555a46","unresolved":false,"context_lines":[{"line_number":149,"context_line":"                          volume_id)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    def revert(self, context, result, flow_failures, **kwargs):"},{"line_number":152,"context_line":"        # NOTE(dulek): Revert is occurring and manager need to know if"},{"line_number":153,"context_line":"        # rescheduling happened. We\u0027re injecting this information into"},{"line_number":154,"context_line":"        # exception that will be caught there. This is ugly and we need"},{"line_number":155,"context_line":"        # TaskFlow to support better way of returning data from reverted flow."}],"source_content_type":"text/x-python","patch_set":15,"id":"fa32b979_e116124b","line":152,"in_reply_to":"fa32b979_16710144","updated":"2015-06-18 12:38:33.000000000","message":"Sure.","commit_id":"a2fa23972c3989066e5818f30913c35eac0979d1"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"3b90a29ab79072f262b7f4b276a4b33605799c0e","unresolved":false,"context_lines":[{"line_number":87,"context_line":"            exception.ImageUnacceptable,"},{"line_number":88,"context_line":"        ]"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    def execute(self, set_error, **kwargs):"},{"line_number":91,"context_line":"        set_error \u003d False"},{"line_number":92,"context_line":"        return set_error"},{"line_number":93,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"fa32b979_d242a567","line":90,"updated":"2015-06-18 20:45:32.000000000","message":"I believe this should be:\n\n def execute(self, **kwargs):\n     return {\u0027set_error\u0027: False}","commit_id":"515283b41820be30ae5c400aaadd26d259bbee39"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"080a891adb0e5d1b68b93b176d3e3bc20fb1b23e","unresolved":false,"context_lines":[{"line_number":87,"context_line":"            exception.ImageUnacceptable,"},{"line_number":88,"context_line":"        ]"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    def execute(self, set_error, **kwargs):"},{"line_number":91,"context_line":"        set_error \u003d False"},{"line_number":92,"context_line":"        return set_error"},{"line_number":93,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"fa32b979_60cf3156","line":90,"in_reply_to":"fa32b979_d242a567","updated":"2015-06-19 07:32:12.000000000","message":"Done","commit_id":"515283b41820be30ae5c400aaadd26d259bbee39"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"3b90a29ab79072f262b7f4b276a4b33605799c0e","unresolved":false,"context_lines":[{"line_number":208,"context_line":"        volume_ref \u003d self.db.volume_get(context, volume_id)"},{"line_number":209,"context_line":"        return volume_ref"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    def revert(self, context, volume_id, set_error, result, **kwargs):"},{"line_number":212,"context_line":"        if isinstance(result, ft.Failure):"},{"line_number":213,"context_line":"            return"},{"line_number":214,"context_line":"        if set_error:"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa32b979_542e828c","line":211,"updated":"2015-06-18 20:45:32.000000000","message":"I believe set_error should be an optional arg here as well.","commit_id":"515283b41820be30ae5c400aaadd26d259bbee39"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"080a891adb0e5d1b68b93b176d3e3bc20fb1b23e","unresolved":false,"context_lines":[{"line_number":208,"context_line":"        volume_ref \u003d self.db.volume_get(context, volume_id)"},{"line_number":209,"context_line":"        return volume_ref"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    def revert(self, context, volume_id, set_error, result, **kwargs):"},{"line_number":212,"context_line":"        if isinstance(result, ft.Failure):"},{"line_number":213,"context_line":"            return"},{"line_number":214,"context_line":"        if set_error:"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa32b979_a036994e","line":211,"in_reply_to":"fa32b979_542e828c","updated":"2015-06-19 07:32:12.000000000","message":"Done","commit_id":"515283b41820be30ae5c400aaadd26d259bbee39"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"3b90a29ab79072f262b7f4b276a4b33605799c0e","unresolved":false,"context_lines":[{"line_number":209,"context_line":"        return volume_ref"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    def revert(self, context, volume_id, set_error, result, **kwargs):"},{"line_number":212,"context_line":"        if isinstance(result, ft.Failure):"},{"line_number":213,"context_line":"            return"},{"line_number":214,"context_line":"        if set_error:"},{"line_number":215,"context_line":"            common.error_out_volume(context, self.db, volume_id)"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa32b979_f2e9e97f","line":212,"updated":"2015-06-18 20:45:32.000000000","message":"Why aren\u0027t we setting the volume to error in this case?","commit_id":"515283b41820be30ae5c400aaadd26d259bbee39"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"080a891adb0e5d1b68b93b176d3e3bc20fb1b23e","unresolved":false,"context_lines":[{"line_number":209,"context_line":"        return volume_ref"},{"line_number":210,"context_line":""},{"line_number":211,"context_line":"    def revert(self, context, volume_id, set_error, result, **kwargs):"},{"line_number":212,"context_line":"        if isinstance(result, ft.Failure):"},{"line_number":213,"context_line":"            return"},{"line_number":214,"context_line":"        if set_error:"},{"line_number":215,"context_line":"            common.error_out_volume(context, self.db, volume_id)"}],"source_content_type":"text/x-python","patch_set":16,"id":"fa32b979_4070ed0a","line":212,"in_reply_to":"fa32b979_f2e9e97f","updated":"2015-06-19 07:32:12.000000000","message":"As the NOTE said, if the volume has deleted before got here. The volume should not be set to error.","commit_id":"515283b41820be30ae5c400aaadd26d259bbee39"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"451d6b514fbcf56d8ad5dfb662c34286401c06a1","unresolved":false,"context_lines":[{"line_number":54,"context_line":"    this volume elsewhere."},{"line_number":55,"context_line":"    \"\"\""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    default_provides \u003d \u0027set_error\u0027"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def __init__(self, reschedule_context, db, scheduler_rpcapi,"},{"line_number":60,"context_line":"                 do_reschedule):"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa32b979_10af8695","line":57,"updated":"2015-06-19 14:15:17.000000000","message":"Remove this","commit_id":"96877dc395f805cd3777d3d0dadbe9eba711e7df"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"13fdd3744e8622af2fb3eb06711ecb8e76405523","unresolved":false,"context_lines":[{"line_number":54,"context_line":"    this volume elsewhere."},{"line_number":55,"context_line":"    \"\"\""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    default_provides \u003d \u0027set_error\u0027"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    def __init__(self, reschedule_context, db, scheduler_rpcapi,"},{"line_number":60,"context_line":"                 do_reschedule):"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa32b979_5644cd72","line":57,"in_reply_to":"fa32b979_10af8695","updated":"2015-06-23 03:53:42.000000000","message":"Done","commit_id":"96877dc395f805cd3777d3d0dadbe9eba711e7df"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"451d6b514fbcf56d8ad5dfb662c34286401c06a1","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        ]"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    def execute(self, **kwargs):"},{"line_number":91,"context_line":"        return {\u0027set_error\u0027: False}"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    def _reschedule(self, context, cause, request_spec, filter_properties,"},{"line_number":94,"context_line":"                    snapshot_id, image_id, volume_id, **kwargs):"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa32b979_709e1265","line":91,"updated":"2015-06-19 14:15:17.000000000","message":"Revert to pass","commit_id":"96877dc395f805cd3777d3d0dadbe9eba711e7df"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"13fdd3744e8622af2fb3eb06711ecb8e76405523","unresolved":false,"context_lines":[{"line_number":88,"context_line":"        ]"},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    def execute(self, **kwargs):"},{"line_number":91,"context_line":"        return {\u0027set_error\u0027: False}"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    def _reschedule(self, context, cause, request_spec, filter_properties,"},{"line_number":94,"context_line":"                    snapshot_id, image_id, volume_id, **kwargs):"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa32b979_d658dd58","line":91,"in_reply_to":"fa32b979_709e1265","updated":"2015-06-23 03:53:42.000000000","message":"Done","commit_id":"96877dc395f805cd3777d3d0dadbe9eba711e7df"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"451d6b514fbcf56d8ad5dfb662c34286401c06a1","unresolved":false,"context_lines":[{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    default_provides \u003d \u0027volume_ref\u0027"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    def __init__(self, db, host):"},{"line_number":197,"context_line":"        super(ExtractVolumeRefTask, self).__init__(addons\u003d[ACTION])"},{"line_number":198,"context_line":"        self.db \u003d db"},{"line_number":199,"context_line":"        self.host \u003d host"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa32b979_f0978238","line":196,"updated":"2015-06-19 14:15:17.000000000","message":"Add optional argument set_error\u003dTrue","commit_id":"96877dc395f805cd3777d3d0dadbe9eba711e7df"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"13fdd3744e8622af2fb3eb06711ecb8e76405523","unresolved":false,"context_lines":[{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    default_provides \u003d \u0027volume_ref\u0027"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    def __init__(self, db, host):"},{"line_number":197,"context_line":"        super(ExtractVolumeRefTask, self).__init__(addons\u003d[ACTION])"},{"line_number":198,"context_line":"        self.db \u003d db"},{"line_number":199,"context_line":"        self.host \u003d host"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa32b979_9631f50d","line":196,"in_reply_to":"fa32b979_f0978238","updated":"2015-06-23 03:53:42.000000000","message":"Done","commit_id":"96877dc395f805cd3777d3d0dadbe9eba711e7df"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"451d6b514fbcf56d8ad5dfb662c34286401c06a1","unresolved":false,"context_lines":[{"line_number":196,"context_line":"    def __init__(self, db, host):"},{"line_number":197,"context_line":"        super(ExtractVolumeRefTask, self).__init__(addons\u003d[ACTION])"},{"line_number":198,"context_line":"        self.db \u003d db"},{"line_number":199,"context_line":"        self.host \u003d host"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    def execute(self, context, volume_id, set_error\u003dTrue):"},{"line_number":202,"context_line":"        # NOTE(harlowja): this will fetch the volume from the database, if"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa32b979_d09fde4e","line":199,"updated":"2015-06-19 14:15:17.000000000","message":"self.set_error \u003d set_error","commit_id":"96877dc395f805cd3777d3d0dadbe9eba711e7df"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"13fdd3744e8622af2fb3eb06711ecb8e76405523","unresolved":false,"context_lines":[{"line_number":196,"context_line":"    def __init__(self, db, host):"},{"line_number":197,"context_line":"        super(ExtractVolumeRefTask, self).__init__(addons\u003d[ACTION])"},{"line_number":198,"context_line":"        self.db \u003d db"},{"line_number":199,"context_line":"        self.host \u003d host"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    def execute(self, context, volume_id, set_error\u003dTrue):"},{"line_number":202,"context_line":"        # NOTE(harlowja): this will fetch the volume from the database, if"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa32b979_563b6deb","line":199,"in_reply_to":"fa32b979_d09fde4e","updated":"2015-06-23 03:53:42.000000000","message":"Done","commit_id":"96877dc395f805cd3777d3d0dadbe9eba711e7df"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"13fdd3744e8622af2fb3eb06711ecb8e76405523","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        self.db \u003d db"},{"line_number":199,"context_line":"        self.host \u003d host"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    def execute(self, context, volume_id, set_error\u003dTrue):"},{"line_number":202,"context_line":"        # NOTE(harlowja): this will fetch the volume from the database, if"},{"line_number":203,"context_line":"        # the volume has been deleted before we got here then this should fail."},{"line_number":204,"context_line":"        #"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa32b979_91233fa7","line":201,"updated":"2015-06-23 03:53:42.000000000","message":"I think there is no need set_error\u003dTrue too, right?","commit_id":"96877dc395f805cd3777d3d0dadbe9eba711e7df"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6281765a4ca40cba6bcbd36c6e986e7024243d5f","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        self.db \u003d db"},{"line_number":199,"context_line":"        self.host \u003d host"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"    def execute(self, context, volume_id, set_error\u003dTrue):"},{"line_number":202,"context_line":"        # NOTE(harlowja): this will fetch the volume from the database, if"},{"line_number":203,"context_line":"        # the volume has been deleted before we got here then this should fail."},{"line_number":204,"context_line":"        #"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa32b979_b3c8c075","line":201,"in_reply_to":"fa32b979_91233fa7","updated":"2015-06-23 09:05:30.000000000","message":"Yes, I forgot to add that one to the comments. Good eye! :-)","commit_id":"96877dc395f805cd3777d3d0dadbe9eba711e7df"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"451d6b514fbcf56d8ad5dfb662c34286401c06a1","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        volume_ref \u003d self.db.volume_get(context, volume_id)"},{"line_number":208,"context_line":"        return volume_ref"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"    def revert(self, context, volume_id, set_error\u003dTrue, result, **kwargs):"},{"line_number":211,"context_line":"        if isinstance(result, ft.Failure):"},{"line_number":212,"context_line":"            return"},{"line_number":213,"context_line":"        if set_error:"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa32b979_491fb2b7","line":210,"updated":"2015-06-19 14:15:17.000000000","message":"Remove set_error optional arg","commit_id":"96877dc395f805cd3777d3d0dadbe9eba711e7df"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"13fdd3744e8622af2fb3eb06711ecb8e76405523","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        volume_ref \u003d self.db.volume_get(context, volume_id)"},{"line_number":208,"context_line":"        return volume_ref"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"    def revert(self, context, volume_id, set_error\u003dTrue, result, **kwargs):"},{"line_number":211,"context_line":"        if isinstance(result, ft.Failure):"},{"line_number":212,"context_line":"            return"},{"line_number":213,"context_line":"        if set_error:"}],"source_content_type":"text/x-python","patch_set":17,"id":"fa32b979_763eb1db","line":210,"in_reply_to":"fa32b979_491fb2b7","updated":"2015-06-23 03:53:42.000000000","message":"Done","commit_id":"96877dc395f805cd3777d3d0dadbe9eba711e7df"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"451d6b514fbcf56d8ad5dfb662c34286401c06a1","unresolved":false,"context_lines":[{"line_number":210,"context_line":"    def revert(self, context, volume_id, set_error\u003dTrue, result, **kwargs):"},{"line_number":211,"context_line":"        if isinstance(result, ft.Failure):"},{"line_number":212,"context_line":"            return"},{"line_number":213,"context_line":"        if set_error:"},{"line_number":214,"context_line":"            common.error_out_volume(context, self.db, volume_id)"},{"line_number":215,"context_line":"            LOG.error(_LE(\"Volume %s: create failed\"), volume_id)"},{"line_number":216,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"fa32b979_1039c641","line":213,"updated":"2015-06-19 14:15:17.000000000","message":"if self.set_error","commit_id":"96877dc395f805cd3777d3d0dadbe9eba711e7df"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"451d6b514fbcf56d8ad5dfb662c34286401c06a1","unresolved":false,"context_lines":[{"line_number":767,"context_line":"        \u0027cgsnapshot_id\u0027: cgsnapshot_id,"},{"line_number":768,"context_line":"    }"},{"line_number":769,"context_line":""},{"line_number":770,"context_line":"    volume_flow.add(ExtractVolumeRefTask(db, host))"},{"line_number":771,"context_line":""},{"line_number":772,"context_line":"    retry \u003d filter_properties.get(\u0027retry\u0027, None)"},{"line_number":773,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"fa32b979_f00a2279","line":770,"updated":"2015-06-19 14:15:17.000000000","message":"volume_flow.add(ExtractVolumeRefTask(db, host, set_error\u003dFalse))","commit_id":"96877dc395f805cd3777d3d0dadbe9eba711e7df"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"13fdd3744e8622af2fb3eb06711ecb8e76405523","unresolved":false,"context_lines":[{"line_number":767,"context_line":"        \u0027cgsnapshot_id\u0027: cgsnapshot_id,"},{"line_number":768,"context_line":"    }"},{"line_number":769,"context_line":""},{"line_number":770,"context_line":"    volume_flow.add(ExtractVolumeRefTask(db, host))"},{"line_number":771,"context_line":""},{"line_number":772,"context_line":"    retry \u003d filter_properties.get(\u0027retry\u0027, None)"},{"line_number":773,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"fa32b979_d6069d1f","line":770,"in_reply_to":"fa32b979_f00a2279","updated":"2015-06-23 03:53:42.000000000","message":"Done","commit_id":"96877dc395f805cd3777d3d0dadbe9eba711e7df"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"385f90713e8ee12cfe8fe83db942d514dede39d4","unresolved":false,"context_lines":[{"line_number":127,"context_line":"        \"\"\"Actions that happen before the rescheduling attempt occur here.\"\"\""},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        try:"},{"line_number":130,"context_line":"            # Reset the volume state."},{"line_number":131,"context_line":"            #"},{"line_number":132,"context_line":"            # NOTE(harlowja): this is awkward to be done here, shouldn\u0027t"},{"line_number":133,"context_line":"            # this happen at the scheduler itself and not before it gets"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa32b979_f8c4e792","line":130,"updated":"2015-06-23 08:48:17.000000000","message":"Remove above line or change it to \u0027update volume\u0027s timestamp\u0027","commit_id":"a2f90d858d4f988d09eaeb5424e3ec61f0c3bdbb"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"1656d21d1df378eeefc4fdea02509867ad9e8fa2","unresolved":false,"context_lines":[{"line_number":127,"context_line":"        \"\"\"Actions that happen before the rescheduling attempt occur here.\"\"\""},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        try:"},{"line_number":130,"context_line":"            # Reset the volume state."},{"line_number":131,"context_line":"            #"},{"line_number":132,"context_line":"            # NOTE(harlowja): this is awkward to be done here, shouldn\u0027t"},{"line_number":133,"context_line":"            # this happen at the scheduler itself and not before it gets"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa32b979_7e799ffb","line":130,"in_reply_to":"fa32b979_f8c4e792","updated":"2015-06-23 09:55:07.000000000","message":"Sure.","commit_id":"a2f90d858d4f988d09eaeb5424e3ec61f0c3bdbb"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e51ff2a8aad18265c5f207646c6c8857fa6f9995","unresolved":false,"context_lines":[{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        # If do not want to be rescheduled, just set the volume\u0027s status to"},{"line_number":155,"context_line":"        # error and return."},{"line_number":156,"context_line":"        if not self.do_reschedule or isinstance(result, ft.Failure):"},{"line_number":157,"context_line":"            common.error_out_volume(context, self.db, volume_id)"},{"line_number":158,"context_line":"            LOG.error(_LE(\"Volume %s: create failed\"), volume_id)"},{"line_number":159,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa32b979_d301c407","line":156,"updated":"2015-06-23 09:15:52.000000000","message":"@wanghao: Maybe we should revert this to how you had it in patch 14 (as it is in ExtractVolumeRefTask\u0027s revert method) so it doesn\u0027t set it to error on ft.Failure, for the same reason that it isn\u0027t set to error there.","commit_id":"a2f90d858d4f988d09eaeb5424e3ec61f0c3bdbb"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"385f90713e8ee12cfe8fe83db942d514dede39d4","unresolved":false,"context_lines":[{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        # If do not want to be rescheduled, just set the volume\u0027s status to"},{"line_number":155,"context_line":"        # error and return."},{"line_number":156,"context_line":"        if not self.do_reschedule or isinstance(result, ft.Failure):"},{"line_number":157,"context_line":"            common.error_out_volume(context, self.db, volume_id)"},{"line_number":158,"context_line":"            LOG.error(_LE(\"Volume %s: create failed\"), volume_id)"},{"line_number":159,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa32b979_18c0ab7b","line":156,"updated":"2015-06-23 08:48:17.000000000","message":"Will the 2nd condition ever be true?","commit_id":"a2f90d858d4f988d09eaeb5424e3ec61f0c3bdbb"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"1656d21d1df378eeefc4fdea02509867ad9e8fa2","unresolved":false,"context_lines":[{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        # If do not want to be rescheduled, just set the volume\u0027s status to"},{"line_number":155,"context_line":"        # error and return."},{"line_number":156,"context_line":"        if not self.do_reschedule or isinstance(result, ft.Failure):"},{"line_number":157,"context_line":"            common.error_out_volume(context, self.db, volume_id)"},{"line_number":158,"context_line":"            LOG.error(_LE(\"Volume %s: create failed\"), volume_id)"},{"line_number":159,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa32b979_f9f949c9","line":156,"in_reply_to":"fa32b979_18c0ab7b","updated":"2015-06-23 09:55:07.000000000","message":"@zhiteng, No, it will be true when execute has some fail.","commit_id":"a2f90d858d4f988d09eaeb5424e3ec61f0c3bdbb"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"1656d21d1df378eeefc4fdea02509867ad9e8fa2","unresolved":false,"context_lines":[{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        # If do not want to be rescheduled, just set the volume\u0027s status to"},{"line_number":155,"context_line":"        # error and return."},{"line_number":156,"context_line":"        if not self.do_reschedule or isinstance(result, ft.Failure):"},{"line_number":157,"context_line":"            common.error_out_volume(context, self.db, volume_id)"},{"line_number":158,"context_line":"            LOG.error(_LE(\"Volume %s: create failed\"), volume_id)"},{"line_number":159,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa32b979_5eb93b78","line":156,"in_reply_to":"fa32b979_d301c407","updated":"2015-06-23 09:55:07.000000000","message":"As zhiteng said in patch 14, we can no need this because execute has do nothing.","commit_id":"a2f90d858d4f988d09eaeb5424e3ec61f0c3bdbb"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"385f90713e8ee12cfe8fe83db942d514dede39d4","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        return volume_ref"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    def revert(self, context, volume_id, result, **kwargs):"},{"line_number":210,"context_line":"        if isinstance(result, ft.Failure):"},{"line_number":211,"context_line":"            return"},{"line_number":212,"context_line":"        if self.set_error:"},{"line_number":213,"context_line":"            common.error_out_volume(context, self.db, volume_id)"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa32b979_b8be5ffd","line":210,"updated":"2015-06-23 08:48:17.000000000","message":"If this is true, should volume be set to \u0027error\u0027?","commit_id":"a2f90d858d4f988d09eaeb5424e3ec61f0c3bdbb"},{"author":{"_account_id":2759,"name":"Huang Zhiteng","email":"winston.d@gmail.com","username":"zhiteng-huang"},"change_message_id":"5d7cc2dc78cfd338ddccdb2cf1b5fce5b1c79a95","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        return volume_ref"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    def revert(self, context, volume_id, result, **kwargs):"},{"line_number":210,"context_line":"        if isinstance(result, ft.Failure):"},{"line_number":211,"context_line":"            return"},{"line_number":212,"context_line":"        if self.set_error:"},{"line_number":213,"context_line":"            common.error_out_volume(context, self.db, volume_id)"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa32b979_59a875db","line":210,"updated":"2015-06-23 09:29:47.000000000","message":"volume being deleted is one case can cause this flow fail.  How about db level failure (for example: db connection timeout)?","commit_id":"a2f90d858d4f988d09eaeb5424e3ec61f0c3bdbb"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"1656d21d1df378eeefc4fdea02509867ad9e8fa2","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        return volume_ref"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    def revert(self, context, volume_id, result, **kwargs):"},{"line_number":210,"context_line":"        if isinstance(result, ft.Failure):"},{"line_number":211,"context_line":"            return"},{"line_number":212,"context_line":"        if self.set_error:"},{"line_number":213,"context_line":"            common.error_out_volume(context, self.db, volume_id)"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa32b979_b958a157","line":210,"in_reply_to":"fa32b979_59a875db","updated":"2015-06-23 09:55:07.000000000","message":"That we maybe also can\u0027t handle the volume status to error as db connection timeout. I think as this situation we may need operator to fix this issue.","commit_id":"a2f90d858d4f988d09eaeb5424e3ec61f0c3bdbb"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e51ff2a8aad18265c5f207646c6c8857fa6f9995","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        return volume_ref"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    def revert(self, context, volume_id, result, **kwargs):"},{"line_number":210,"context_line":"        if isinstance(result, ft.Failure):"},{"line_number":211,"context_line":"            return"},{"line_number":212,"context_line":"        if self.set_error:"},{"line_number":213,"context_line":"            common.error_out_volume(context, self.db, volume_id)"}],"source_content_type":"text/x-python","patch_set":18,"id":"fa32b979_7379d8fb","line":210,"in_reply_to":"fa32b979_b8be5ffd","updated":"2015-06-23 09:15:52.000000000","message":"This was answered in patch 16.\n\n \u003e As the NOTE said, if the volume has deleted before got here. The volume should not be set to error.","commit_id":"a2f90d858d4f988d09eaeb5424e3ec61f0c3bdbb"}]}