)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"209a603965e576167f9cc575efdc12e4f35ff1dd","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Co-Authored-By: wanghao \u003csxmatch1986@gmail.com\u003e"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Closes-Bug: 1618458"},{"line_number":16,"context_line":"Change-Id: I72d0dd15cb0e4d6b09034ddb024eee1988ce078e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"fa7ab95a_b6c3f237","line":15,"updated":"2016-08-31 08:54:13.000000000","message":"I don\u0027t consider test coverage to be a bug that should be tracked in LP.","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"},{"author":{"_account_id":16212,"name":"Chuck Fouts","email":"fchuck@netapp.com","username":"cFouts"},"change_message_id":"7bada91693119903bbbc23727fe75cfd4df117e4","unresolved":false,"context_lines":[{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Co-Authored-By: wanghao \u003csxmatch1986@gmail.com\u003e"},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Closes-Bug: 1618458"},{"line_number":16,"context_line":"Change-Id: I72d0dd15cb0e4d6b09034ddb024eee1988ce078e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"fa7ab95a_68aa5087","line":15,"in_reply_to":"fa7ab95a_b6c3f237","updated":"2016-08-31 20:57:47.000000000","message":"Agreed","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"}],"cinder/tests/unit/test_volume.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"209a603965e576167f9cc575efdc12e4f35ff1dd","unresolved":false,"context_lines":[{"line_number":4662,"context_line":"    @mock.patch(\u0027cinder.volume.utils.notify_about_volume_usage\u0027)"},{"line_number":4663,"context_line":"    def test_manage_volume_with_notify(self, mock_notify, mock_size,"},{"line_number":4664,"context_line":"                                       mock_manage):"},{"line_number":4665,"context_line":"        elevated \u003d context.get_admin_context()"},{"line_number":4666,"context_line":"        db.volume_type_create(elevated, {\u0027name\u0027: \u0027type1\u0027, \u0027extra_specs\u0027: {}})"},{"line_number":4667,"context_line":"        vol_type \u003d db.volume_type_get_by_name(elevated, \u0027type1\u0027)"},{"line_number":4668,"context_line":"        # create source volume"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_368282c9","line":4665,"updated":"2016-08-31 08:54:13.000000000","message":"-1: This is self.context","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"116e65ecd5ada85373da33d736f200764a24eaef","unresolved":false,"context_lines":[{"line_number":4662,"context_line":"    @mock.patch(\u0027cinder.volume.utils.notify_about_volume_usage\u0027)"},{"line_number":4663,"context_line":"    def test_manage_volume_with_notify(self, mock_notify, mock_size,"},{"line_number":4664,"context_line":"                                       mock_manage):"},{"line_number":4665,"context_line":"        elevated \u003d context.get_admin_context()"},{"line_number":4666,"context_line":"        db.volume_type_create(elevated, {\u0027name\u0027: \u0027type1\u0027, \u0027extra_specs\u0027: {}})"},{"line_number":4667,"context_line":"        vol_type \u003d db.volume_type_get_by_name(elevated, \u0027type1\u0027)"},{"line_number":4668,"context_line":"        # create source volume"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_79f2f29b","line":4665,"in_reply_to":"fa7ab95a_08c1c42e","updated":"2016-09-01 08:33:02.000000000","message":"It should have worked, unless it\u0027s a problem with the project id and/or user id.  But I don\u0027t think it\u0027s all that important to figure out why it doesn\u0027t work, so it\u0027s ok.","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"},{"author":{"_account_id":16212,"name":"Chuck Fouts","email":"fchuck@netapp.com","username":"cFouts"},"change_message_id":"7bada91693119903bbbc23727fe75cfd4df117e4","unresolved":false,"context_lines":[{"line_number":4662,"context_line":"    @mock.patch(\u0027cinder.volume.utils.notify_about_volume_usage\u0027)"},{"line_number":4663,"context_line":"    def test_manage_volume_with_notify(self, mock_notify, mock_size,"},{"line_number":4664,"context_line":"                                       mock_manage):"},{"line_number":4665,"context_line":"        elevated \u003d context.get_admin_context()"},{"line_number":4666,"context_line":"        db.volume_type_create(elevated, {\u0027name\u0027: \u0027type1\u0027, \u0027extra_specs\u0027: {}})"},{"line_number":4667,"context_line":"        vol_type \u003d db.volume_type_get_by_name(elevated, \u0027type1\u0027)"},{"line_number":4668,"context_line":"        # create source volume"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_08c1c42e","line":4665,"in_reply_to":"fa7ab95a_368282c9","updated":"2016-08-31 20:57:47.000000000","message":"get_admin_context() is actually a function in cinder.context.py that is outside the RequestContext class. It is a wrapper to create an admin RequestContext.\n\nI tried using self.context and the test failed. I also used self.context.elevated and the test failed. I reverted to using the original \u0027elevated\u0027 context.","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"209a603965e576167f9cc575efdc12e4f35ff1dd","unresolved":false,"context_lines":[{"line_number":4663,"context_line":"    def test_manage_volume_with_notify(self, mock_notify, mock_size,"},{"line_number":4664,"context_line":"                                       mock_manage):"},{"line_number":4665,"context_line":"        elevated \u003d context.get_admin_context()"},{"line_number":4666,"context_line":"        db.volume_type_create(elevated, {\u0027name\u0027: \u0027type1\u0027, \u0027extra_specs\u0027: {}})"},{"line_number":4667,"context_line":"        vol_type \u003d db.volume_type_get_by_name(elevated, \u0027type1\u0027)"},{"line_number":4668,"context_line":"        # create source volume"},{"line_number":4669,"context_line":"        volume_params \u003d {}"},{"line_number":4670,"context_line":"        volume_params[\u0027volume_type_id\u0027] \u003d vol_type[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_161c9e76","line":4667,"range":{"start_line":4666,"start_character":0,"end_line":4667,"end_character":64},"updated":"2016-08-31 08:54:13.000000000","message":"-1: You can just do:\n\n vol_type \u003d db.volume_type_create(self.context,\n                                  {\u0027name\u0027: \u0027type1\u0027, \u0027extra_specs\u0027: {}})\n\nOr\n\n vol_type \u003d test_utils.create_volume_type(\n     self.context, self, {\u0027name\u0027: \u0027type1\u0027, \u0027extra_specs\u0027: {}})\n\nNo need to reload the data from the DB","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"},{"author":{"_account_id":16212,"name":"Chuck Fouts","email":"fchuck@netapp.com","username":"cFouts"},"change_message_id":"7bada91693119903bbbc23727fe75cfd4df117e4","unresolved":false,"context_lines":[{"line_number":4663,"context_line":"    def test_manage_volume_with_notify(self, mock_notify, mock_size,"},{"line_number":4664,"context_line":"                                       mock_manage):"},{"line_number":4665,"context_line":"        elevated \u003d context.get_admin_context()"},{"line_number":4666,"context_line":"        db.volume_type_create(elevated, {\u0027name\u0027: \u0027type1\u0027, \u0027extra_specs\u0027: {}})"},{"line_number":4667,"context_line":"        vol_type \u003d db.volume_type_get_by_name(elevated, \u0027type1\u0027)"},{"line_number":4668,"context_line":"        # create source volume"},{"line_number":4669,"context_line":"        volume_params \u003d {}"},{"line_number":4670,"context_line":"        volume_params[\u0027volume_type_id\u0027] \u003d vol_type[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_ea9e5062","line":4667,"range":{"start_line":4666,"start_character":0,"end_line":4667,"end_character":64},"in_reply_to":"fa7ab95a_161c9e76","updated":"2016-08-31 20:57:47.000000000","message":"Done","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"209a603965e576167f9cc575efdc12e4f35ff1dd","unresolved":false,"context_lines":[{"line_number":4667,"context_line":"        vol_type \u003d db.volume_type_get_by_name(elevated, \u0027type1\u0027)"},{"line_number":4668,"context_line":"        # create source volume"},{"line_number":4669,"context_line":"        volume_params \u003d {}"},{"line_number":4670,"context_line":"        volume_params[\u0027volume_type_id\u0027] \u003d vol_type[\u0027id\u0027]"},{"line_number":4671,"context_line":"        volume_params[\u0027status\u0027] \u003d \u0027managing\u0027"},{"line_number":4672,"context_line":"        test_vol \u003d tests_utils.create_volume(self.context, **volume_params)"},{"line_number":4673,"context_line":"        mock_size.return_value \u003d 1"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_760b2a2c","line":4670,"range":{"start_line":4670,"start_character":42,"end_line":4670,"end_character":56},"updated":"2016-08-31 08:54:13.000000000","message":"-1: vol_type.id","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"},{"author":{"_account_id":16212,"name":"Chuck Fouts","email":"fchuck@netapp.com","username":"cFouts"},"change_message_id":"7bada91693119903bbbc23727fe75cfd4df117e4","unresolved":false,"context_lines":[{"line_number":4667,"context_line":"        vol_type \u003d db.volume_type_get_by_name(elevated, \u0027type1\u0027)"},{"line_number":4668,"context_line":"        # create source volume"},{"line_number":4669,"context_line":"        volume_params \u003d {}"},{"line_number":4670,"context_line":"        volume_params[\u0027volume_type_id\u0027] \u003d vol_type[\u0027id\u0027]"},{"line_number":4671,"context_line":"        volume_params[\u0027status\u0027] \u003d \u0027managing\u0027"},{"line_number":4672,"context_line":"        test_vol \u003d tests_utils.create_volume(self.context, **volume_params)"},{"line_number":4673,"context_line":"        mock_size.return_value \u003d 1"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_bb6e6494","line":4670,"range":{"start_line":4670,"start_character":42,"end_line":4670,"end_character":56},"in_reply_to":"fa7ab95a_760b2a2c","updated":"2016-08-31 20:57:47.000000000","message":"Done","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"}],"cinder/tests/unit/volume/flows/test_manage_volume_flow.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"209a603965e576167f9cc575efdc12e4f35ff1dd","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        mock_error_out.assert_called_once_with(volume_ref, reason\u003dmock.ANY)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def test_get_flow(self):"},{"line_number":102,"context_line":"        mock_context \u003d mock.MagicMock()"},{"line_number":103,"context_line":"        mock_db \u003d mock.MagicMock()"},{"line_number":104,"context_line":"        mock_driver \u003d mock.MagicMock()"},{"line_number":105,"context_line":"        mock_host \u003d mock.MagicMock()"},{"line_number":106,"context_line":"        mock_volume \u003d mock.MagicMock()"},{"line_number":107,"context_line":"        mock_ref \u003d mock.MagicMock()"},{"line_number":108,"context_line":"        mock_volume_flow \u003d mock.MagicMock()"},{"line_number":109,"context_line":"        mock_flow_builder \u003d self.mock_object("},{"line_number":110,"context_line":"            manager, \u0027volume_flow_builder\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_59ebb9b0","line":107,"range":{"start_line":102,"start_character":0,"end_line":107,"end_character":35},"updated":"2016-08-31 08:54:13.000000000","message":"nit: Why not use sentinels for these?\n\n mock.sentinel.context\n mock.sentinel.db\n ...","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"},{"author":{"_account_id":16212,"name":"Chuck Fouts","email":"fchuck@netapp.com","username":"cFouts"},"change_message_id":"7bada91693119903bbbc23727fe75cfd4df117e4","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        mock_error_out.assert_called_once_with(volume_ref, reason\u003dmock.ANY)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def test_get_flow(self):"},{"line_number":102,"context_line":"        mock_context \u003d mock.MagicMock()"},{"line_number":103,"context_line":"        mock_db \u003d mock.MagicMock()"},{"line_number":104,"context_line":"        mock_driver \u003d mock.MagicMock()"},{"line_number":105,"context_line":"        mock_host \u003d mock.MagicMock()"},{"line_number":106,"context_line":"        mock_volume \u003d mock.MagicMock()"},{"line_number":107,"context_line":"        mock_ref \u003d mock.MagicMock()"},{"line_number":108,"context_line":"        mock_volume_flow \u003d mock.MagicMock()"},{"line_number":109,"context_line":"        mock_flow_builder \u003d self.mock_object("},{"line_number":110,"context_line":"            manager, \u0027volume_flow_builder\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_6cc17d2e","line":107,"range":{"start_line":102,"start_character":0,"end_line":107,"end_character":35},"in_reply_to":"fa7ab95a_59ebb9b0","updated":"2016-08-31 20:57:47.000000000","message":"Thanks for pointing this out. I like being able to reduce the number of lines of code. Especially when it comes to creating mocks.","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"116e65ecd5ada85373da33d736f200764a24eaef","unresolved":false,"context_lines":[{"line_number":99,"context_line":"        mock_error_out.assert_called_once_with(volume_ref, reason\u003dmock.ANY)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def test_get_flow(self):"},{"line_number":102,"context_line":"        mock_context \u003d mock.MagicMock()"},{"line_number":103,"context_line":"        mock_db \u003d mock.MagicMock()"},{"line_number":104,"context_line":"        mock_driver \u003d mock.MagicMock()"},{"line_number":105,"context_line":"        mock_host \u003d mock.MagicMock()"},{"line_number":106,"context_line":"        mock_volume \u003d mock.MagicMock()"},{"line_number":107,"context_line":"        mock_ref \u003d mock.MagicMock()"},{"line_number":108,"context_line":"        mock_volume_flow \u003d mock.MagicMock()"},{"line_number":109,"context_line":"        mock_flow_builder \u003d self.mock_object("},{"line_number":110,"context_line":"            manager, \u0027volume_flow_builder\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_b9e8fa17","line":107,"range":{"start_line":102,"start_character":0,"end_line":107,"end_character":35},"in_reply_to":"fa7ab95a_6cc17d2e","updated":"2016-09-01 08:33:02.000000000","message":"Yeah, me too, because it makes it hard to see the actual test.","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"209a603965e576167f9cc575efdc12e4f35ff1dd","unresolved":false,"context_lines":[{"line_number":105,"context_line":"        mock_host \u003d mock.MagicMock()"},{"line_number":106,"context_line":"        mock_volume \u003d mock.MagicMock()"},{"line_number":107,"context_line":"        mock_ref \u003d mock.MagicMock()"},{"line_number":108,"context_line":"        mock_volume_flow \u003d mock.MagicMock()"},{"line_number":109,"context_line":"        mock_flow_builder \u003d self.mock_object("},{"line_number":110,"context_line":"            manager, \u0027volume_flow_builder\u0027,"},{"line_number":111,"context_line":"            mock.Mock(return_value\u003dmock_volume_flow))"},{"line_number":112,"context_line":"        mock_taskflow_engine \u003d self.mock_object(taskflow.engines, \u0027load\u0027)"},{"line_number":113,"context_line":"        mock_store \u003d {"},{"line_number":114,"context_line":"            \u0027context\u0027: mock_context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_59457915","line":111,"range":{"start_line":108,"start_character":0,"end_line":111,"end_character":53},"updated":"2016-08-31 08:54:13.000000000","message":"-1: This is just:\n\n mock_flow_builder \u003d self.mock_object(manager, \u0027volume_flow_builder\u0027)\n\nAnd then on L126:\n\n mock_flow_builder.return_value\n\nAnd even if you preferred to have an independent mock that is not automatically created, with the new mock_object functionality you don\u0027t need to pass a Mock object:\n\n mock_flow_builder \u003d self.mock_object(\n     manager, \u0027volume_flow_builder\u0027,\n     return_value\u003dmock_volume_flow)","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"},{"author":{"_account_id":16212,"name":"Chuck Fouts","email":"fchuck@netapp.com","username":"cFouts"},"change_message_id":"7bada91693119903bbbc23727fe75cfd4df117e4","unresolved":false,"context_lines":[{"line_number":105,"context_line":"        mock_host \u003d mock.MagicMock()"},{"line_number":106,"context_line":"        mock_volume \u003d mock.MagicMock()"},{"line_number":107,"context_line":"        mock_ref \u003d mock.MagicMock()"},{"line_number":108,"context_line":"        mock_volume_flow \u003d mock.MagicMock()"},{"line_number":109,"context_line":"        mock_flow_builder \u003d self.mock_object("},{"line_number":110,"context_line":"            manager, \u0027volume_flow_builder\u0027,"},{"line_number":111,"context_line":"            mock.Mock(return_value\u003dmock_volume_flow))"},{"line_number":112,"context_line":"        mock_taskflow_engine \u003d self.mock_object(taskflow.engines, \u0027load\u0027)"},{"line_number":113,"context_line":"        mock_store \u003d {"},{"line_number":114,"context_line":"            \u0027context\u0027: mock_context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_6cf97d4d","line":111,"range":{"start_line":108,"start_character":0,"end_line":111,"end_character":53},"in_reply_to":"fa7ab95a_59457915","updated":"2016-08-31 20:57:47.000000000","message":"I actually prefer having the return_value on a separate line as you show here. I\u0027ve seen the other pattern used more often so I thought it was a community style.","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"209a603965e576167f9cc575efdc12e4f35ff1dd","unresolved":false,"context_lines":[{"line_number":110,"context_line":"            manager, \u0027volume_flow_builder\u0027,"},{"line_number":111,"context_line":"            mock.Mock(return_value\u003dmock_volume_flow))"},{"line_number":112,"context_line":"        mock_taskflow_engine \u003d self.mock_object(taskflow.engines, \u0027load\u0027)"},{"line_number":113,"context_line":"        mock_store \u003d {"},{"line_number":114,"context_line":"            \u0027context\u0027: mock_context,"},{"line_number":115,"context_line":"            \u0027volume\u0027: mock_volume,"},{"line_number":116,"context_line":"            \u0027manage_existing_ref\u0027: mock_ref,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_b96be584","line":113,"updated":"2016-08-31 08:54:13.000000000","message":"-1: This name is confusing, as this is not a mock.  A better name would be expected_store or something like that.","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"},{"author":{"_account_id":16212,"name":"Chuck Fouts","email":"fchuck@netapp.com","username":"cFouts"},"change_message_id":"7bada91693119903bbbc23727fe75cfd4df117e4","unresolved":false,"context_lines":[{"line_number":110,"context_line":"            manager, \u0027volume_flow_builder\u0027,"},{"line_number":111,"context_line":"            mock.Mock(return_value\u003dmock_volume_flow))"},{"line_number":112,"context_line":"        mock_taskflow_engine \u003d self.mock_object(taskflow.engines, \u0027load\u0027)"},{"line_number":113,"context_line":"        mock_store \u003d {"},{"line_number":114,"context_line":"            \u0027context\u0027: mock_context,"},{"line_number":115,"context_line":"            \u0027volume\u0027: mock_volume,"},{"line_number":116,"context_line":"            \u0027manage_existing_ref\u0027: mock_ref,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_4caaf91a","line":113,"in_reply_to":"fa7ab95a_b96be584","updated":"2016-08-31 20:57:47.000000000","message":"Done","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"209a603965e576167f9cc575efdc12e4f35ff1dd","unresolved":false,"context_lines":[{"line_number":140,"context_line":"        volume_flow \u003d manager.volume_flow_builder(\"foo_manager\","},{"line_number":141,"context_line":"                                                  mock_db,"},{"line_number":142,"context_line":"                                                  mock_driver)"},{"line_number":143,"context_line":"        provides \u003d set([\u0027self\u0027, ])"},{"line_number":144,"context_line":"        revert_provides \u003d [\u0027self\u0027, \u0027result\u0027, \u0027flow_failures\u0027, ]"},{"line_number":145,"context_line":"        for node in volume_flow.iter_nodes():"},{"line_number":146,"context_line":"            task \u003d node[0]"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_192f71f7","line":143,"range":{"start_line":143,"start_character":30,"end_line":143,"end_character":32},"updated":"2016-08-31 08:54:13.000000000","message":"nit: No comma is needed here","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"},{"author":{"_account_id":16212,"name":"Chuck Fouts","email":"fchuck@netapp.com","username":"cFouts"},"change_message_id":"7bada91693119903bbbc23727fe75cfd4df117e4","unresolved":false,"context_lines":[{"line_number":140,"context_line":"        volume_flow \u003d manager.volume_flow_builder(\"foo_manager\","},{"line_number":141,"context_line":"                                                  mock_db,"},{"line_number":142,"context_line":"                                                  mock_driver)"},{"line_number":143,"context_line":"        provides \u003d set([\u0027self\u0027, ])"},{"line_number":144,"context_line":"        revert_provides \u003d [\u0027self\u0027, \u0027result\u0027, \u0027flow_failures\u0027, ]"},{"line_number":145,"context_line":"        for node in volume_flow.iter_nodes():"},{"line_number":146,"context_line":"            task \u003d node[0]"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_4c067921","line":143,"range":{"start_line":143,"start_character":30,"end_line":143,"end_character":32},"in_reply_to":"fa7ab95a_192f71f7","updated":"2016-08-31 20:57:47.000000000","message":"Done","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"209a603965e576167f9cc575efdc12e4f35ff1dd","unresolved":false,"context_lines":[{"line_number":141,"context_line":"                                                  mock_db,"},{"line_number":142,"context_line":"                                                  mock_driver)"},{"line_number":143,"context_line":"        provides \u003d set([\u0027self\u0027, ])"},{"line_number":144,"context_line":"        revert_provides \u003d [\u0027self\u0027, \u0027result\u0027, \u0027flow_failures\u0027, ]"},{"line_number":145,"context_line":"        for node in volume_flow.iter_nodes():"},{"line_number":146,"context_line":"            task \u003d node[0]"},{"line_number":147,"context_line":"            # Subsequent tasks may use parameters defined in a previous task\u0027s"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_993b8138","line":144,"range":{"start_line":144,"start_character":60,"end_line":144,"end_character":62},"updated":"2016-08-31 08:54:13.000000000","message":"ditto","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"},{"author":{"_account_id":16212,"name":"Chuck Fouts","email":"fchuck@netapp.com","username":"cFouts"},"change_message_id":"7bada91693119903bbbc23727fe75cfd4df117e4","unresolved":false,"context_lines":[{"line_number":141,"context_line":"                                                  mock_db,"},{"line_number":142,"context_line":"                                                  mock_driver)"},{"line_number":143,"context_line":"        provides \u003d set([\u0027self\u0027, ])"},{"line_number":144,"context_line":"        revert_provides \u003d [\u0027self\u0027, \u0027result\u0027, \u0027flow_failures\u0027, ]"},{"line_number":145,"context_line":"        for node in volume_flow.iter_nodes():"},{"line_number":146,"context_line":"            task \u003d node[0]"},{"line_number":147,"context_line":"            # Subsequent tasks may use parameters defined in a previous task\u0027s"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_6c033d10","line":144,"range":{"start_line":144,"start_character":60,"end_line":144,"end_character":62},"in_reply_to":"fa7ab95a_993b8138","updated":"2016-08-31 20:57:47.000000000","message":"Done","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"}],"cinder/volume/flows/manager/manage_existing.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"209a603965e576167f9cc575efdc12e4f35ff1dd","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        \u0027manage_existing_ref\u0027: ref,"},{"line_number":109,"context_line":"        \u0027optional_args\u0027: {\u0027is_quota_committed\u0027: False},"},{"line_number":110,"context_line":"    }"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    volume_flow \u003d volume_flow_builder(flow_name, db, driver)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    # Now load (but do not run) the flow using the provided initial data."},{"line_number":115,"context_line":"    return taskflow.engines.load(volume_flow, store\u003dcreate_what)"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"def volume_flow_builder(flow_name, db, driver):"},{"line_number":119,"context_line":"    volume_flow \u003d linear_flow.Flow(flow_name)"},{"line_number":120,"context_line":"    volume_flow.add(create_mgr.NotifyVolumeActionTask(db,"},{"line_number":121,"context_line":"                                                      \"manage_existing.start\"),"},{"line_number":122,"context_line":"                    PrepareForQuotaReservationTask(db, driver),"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_b91cc56b","line":119,"range":{"start_line":111,"start_character":0,"end_line":119,"end_character":45},"updated":"2016-08-31 08:54:13.000000000","message":"-1: Why are we splitting this?  I think it makes it harder to read the code.  If this is for the tests, we can still test that all expected arguments exist with the old code.","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"52ba01627ec8f39215da5bc99dd9d5dc7edca166","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        \u0027manage_existing_ref\u0027: ref,"},{"line_number":109,"context_line":"        \u0027optional_args\u0027: {\u0027is_quota_committed\u0027: False},"},{"line_number":110,"context_line":"    }"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    volume_flow \u003d volume_flow_builder(flow_name, db, driver)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    # Now load (but do not run) the flow using the provided initial data."},{"line_number":115,"context_line":"    return taskflow.engines.load(volume_flow, store\u003dcreate_what)"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"def volume_flow_builder(flow_name, db, driver):"},{"line_number":119,"context_line":"    volume_flow \u003d linear_flow.Flow(flow_name)"},{"line_number":120,"context_line":"    volume_flow.add(create_mgr.NotifyVolumeActionTask(db,"},{"line_number":121,"context_line":"                                                      \"manage_existing.start\"),"},{"line_number":122,"context_line":"                    PrepareForQuotaReservationTask(db, driver),"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_c6ac398f","line":119,"range":{"start_line":111,"start_character":0,"end_line":119,"end_character":45},"in_reply_to":"fa7ab95a_b91cc56b","updated":"2016-08-31 10:53:50.000000000","message":"I thought it\u0027s refactoring to the code, not for the tests, so I suggest to move the test code to patch 361454. What you guys think?","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"},{"author":{"_account_id":16212,"name":"Chuck Fouts","email":"fchuck@netapp.com","username":"cFouts"},"change_message_id":"7bada91693119903bbbc23727fe75cfd4df117e4","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        \u0027manage_existing_ref\u0027: ref,"},{"line_number":109,"context_line":"        \u0027optional_args\u0027: {\u0027is_quota_committed\u0027: False},"},{"line_number":110,"context_line":"    }"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    volume_flow \u003d volume_flow_builder(flow_name, db, driver)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    # Now load (but do not run) the flow using the provided initial data."},{"line_number":115,"context_line":"    return taskflow.engines.load(volume_flow, store\u003dcreate_what)"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"def volume_flow_builder(flow_name, db, driver):"},{"line_number":119,"context_line":"    volume_flow \u003d linear_flow.Flow(flow_name)"},{"line_number":120,"context_line":"    volume_flow.add(create_mgr.NotifyVolumeActionTask(db,"},{"line_number":121,"context_line":"                                                      \"manage_existing.start\"),"},{"line_number":122,"context_line":"                    PrepareForQuotaReservationTask(db, driver),"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_fb39c1d9","line":119,"range":{"start_line":111,"start_character":0,"end_line":119,"end_character":45},"in_reply_to":"fa7ab95a_b91cc56b","updated":"2016-08-31 20:57:47.000000000","message":"You\u0027re right that this refactor doesn\u0027t improve readability. However, I don\u0027t see a way to get the flow object from the object returned by taskflow.engines.load() in get_flow(). https://github.com/openstack/taskflow/blob/master/taskflow/engines/helpers.py\n\nI wanted to test the parameters required by the tasks in the order that they are actually added to volume_flow. I\u0027m not sure how to otherwise test this. Would appreciate additional suggestions.\n\nwanghao\u0027s test will catch the same bug but is more of an integration test than a unit test. I see value in having a more granular unit test but I am willing to drop this test in order to get this patch merged.","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"},{"author":{"_account_id":16212,"name":"Chuck Fouts","email":"fchuck@netapp.com","username":"cFouts"},"change_message_id":"7bada91693119903bbbc23727fe75cfd4df117e4","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        \u0027manage_existing_ref\u0027: ref,"},{"line_number":109,"context_line":"        \u0027optional_args\u0027: {\u0027is_quota_committed\u0027: False},"},{"line_number":110,"context_line":"    }"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    volume_flow \u003d volume_flow_builder(flow_name, db, driver)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    # Now load (but do not run) the flow using the provided initial data."},{"line_number":115,"context_line":"    return taskflow.engines.load(volume_flow, store\u003dcreate_what)"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"def volume_flow_builder(flow_name, db, driver):"},{"line_number":119,"context_line":"    volume_flow \u003d linear_flow.Flow(flow_name)"},{"line_number":120,"context_line":"    volume_flow.add(create_mgr.NotifyVolumeActionTask(db,"},{"line_number":121,"context_line":"                                                      \"manage_existing.start\"),"},{"line_number":122,"context_line":"                    PrepareForQuotaReservationTask(db, driver),"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_f411c0b0","line":119,"range":{"start_line":111,"start_character":0,"end_line":119,"end_character":45},"in_reply_to":"fa7ab95a_c6ac398f","updated":"2016-08-31 20:57:47.000000000","message":"I agree with this. I\u0027ll do this after sending a patch to respond to the comments for patch 2.","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"116e65ecd5ada85373da33d736f200764a24eaef","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        \u0027manage_existing_ref\u0027: ref,"},{"line_number":109,"context_line":"        \u0027optional_args\u0027: {\u0027is_quota_committed\u0027: False},"},{"line_number":110,"context_line":"    }"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    volume_flow \u003d volume_flow_builder(flow_name, db, driver)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    # Now load (but do not run) the flow using the provided initial data."},{"line_number":115,"context_line":"    return taskflow.engines.load(volume_flow, store\u003dcreate_what)"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"def volume_flow_builder(flow_name, db, driver):"},{"line_number":119,"context_line":"    volume_flow \u003d linear_flow.Flow(flow_name)"},{"line_number":120,"context_line":"    volume_flow.add(create_mgr.NotifyVolumeActionTask(db,"},{"line_number":121,"context_line":"                                                      \"manage_existing.start\"),"},{"line_number":122,"context_line":"                    PrepareForQuotaReservationTask(db, driver),"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa7ab95a_59c9ae78","line":119,"range":{"start_line":111,"start_character":0,"end_line":119,"end_character":45},"in_reply_to":"fa7ab95a_fb39c1d9","updated":"2016-09-01 08:33:02.000000000","message":"One way to do it is, using the old code, mocking taskflow.engines.load and replace it with a test method that will do the asserts you currently do with returned value of new volume_flow builder, because you will be receiving volume_flow and store parameters in the test method.","commit_id":"e6ff74be604cad233f9e68d9db9068be77482090"}]}