)]}'
{"nova/tests/unit/virt/ironic/test_driver.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"0d8d942e21dd9305d03c62c05377856550850f46","unresolved":false,"context_lines":[{"line_number":450,"context_line":"            instances.append(fake_instance.fake_instance_obj(self.ctx,"},{"line_number":451,"context_line":"                                                             id\u003di,"},{"line_number":452,"context_line":"                                                             uuid\u003duuid))"},{"line_number":453,"context_line":"            nodes.append(ironic_utils.get_test_node(instance_uuid\u003duuid))"},{"line_number":454,"context_line":""},{"line_number":455,"context_line":"        mock_inst_by_uuid.side_effect \u003d instances"},{"line_number":456,"context_line":"        mock_call.side_effect \u003d exception.NovaException"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa32b979_89dc3674","line":453,"updated":"2015-06-17 16:43:47.000000000","message":"nit: nodes variable not being used anywhere\n\nTo be honest, this whole for loop can be removed","commit_id":"f37709ace7f0269fdef785c78dda9e76ec093764"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"0d8d942e21dd9305d03c62c05377856550850f46","unresolved":false,"context_lines":[{"line_number":452,"context_line":"                                                             uuid\u003duuid))"},{"line_number":453,"context_line":"            nodes.append(ironic_utils.get_test_node(instance_uuid\u003duuid))"},{"line_number":454,"context_line":""},{"line_number":455,"context_line":"        mock_inst_by_uuid.side_effect \u003d instances"},{"line_number":456,"context_line":"        mock_call.side_effect \u003d exception.NovaException"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"        response \u003d self.driver.list_instances()"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa32b979_89f8b6c4","line":455,"updated":"2015-06-17 16:43:47.000000000","message":"mock_inst_by_uuid shouldn\u0027t be called (and we should assert that) so we don\u0027t need to set the side_effect here","commit_id":"f37709ace7f0269fdef785c78dda9e76ec093764"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"0d8d942e21dd9305d03c62c05377856550850f46","unresolved":false,"context_lines":[{"line_number":458,"context_line":"        response \u003d self.driver.list_instances()"},{"line_number":459,"context_line":"        mock_call.assert_called_with(\"node.list\", associated\u003dTrue, limit\u003d0)"},{"line_number":460,"context_line":"        expected_calls \u003d []"},{"line_number":461,"context_line":"        mock_inst_by_uuid.assert_has_calls(expected_calls)"},{"line_number":462,"context_line":"        self.assertEqual([], sorted(response))"},{"line_number":463,"context_line":""},{"line_number":464,"context_line":"    @mock.patch.object(cw.IronicClientWrapper, \u0027call\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa32b979_696a12b5","line":461,"updated":"2015-06-17 16:43:47.000000000","message":"nit: no reason to create the expected_calls variable.\n\n self.assertFalse(mock_inst_by_uuid.called)","commit_id":"f37709ace7f0269fdef785c78dda9e76ec093764"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"0d8d942e21dd9305d03c62c05377856550850f46","unresolved":false,"context_lines":[{"line_number":459,"context_line":"        mock_call.assert_called_with(\"node.list\", associated\u003dTrue, limit\u003d0)"},{"line_number":460,"context_line":"        expected_calls \u003d []"},{"line_number":461,"context_line":"        mock_inst_by_uuid.assert_has_calls(expected_calls)"},{"line_number":462,"context_line":"        self.assertEqual([], sorted(response))"},{"line_number":463,"context_line":""},{"line_number":464,"context_line":"    @mock.patch.object(cw.IronicClientWrapper, \u0027call\u0027)"},{"line_number":465,"context_line":"    def test_list_instance_uuids(self, mock_call):"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa32b979_29b72a3b","line":462,"updated":"2015-06-17 16:43:47.000000000","message":"nit: sorted() not needed.\n\n...\n\nFor a better output you could use:\n\n self.assertThat(response, HasLength(0))","commit_id":"f37709ace7f0269fdef785c78dda9e76ec093764"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"0d8d942e21dd9305d03c62c05377856550850f46","unresolved":false,"context_lines":[{"line_number":460,"context_line":"        expected_calls \u003d []"},{"line_number":461,"context_line":"        mock_inst_by_uuid.assert_has_calls(expected_calls)"},{"line_number":462,"context_line":"        self.assertEqual([], sorted(response))"},{"line_number":463,"context_line":""},{"line_number":464,"context_line":"    @mock.patch.object(cw.IronicClientWrapper, \u0027call\u0027)"},{"line_number":465,"context_line":"    def test_list_instance_uuids(self, mock_call):"},{"line_number":466,"context_line":"        num_nodes \u003d 2"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa32b979_69977248","line":463,"updated":"2015-06-17 16:43:47.000000000","message":"The whole test should look like:\n\n    from testtools.matchers import HasLength\n\n    @mock.patch.object(cw.IronicClientWrapper, \u0027call\u0027)\n    @mock.patch.object(objects.Instance, \u0027get_by_uuid\u0027)\n    def test_list_instances_fail(self, mock_inst_by_uuid, mock_call):\n        mock_call.side_effect \u003d exception.NovaException\n        response \u003d self.driver.list_instances()\n        mock_call.assert_called_with(\"node.list\", associated\u003dTrue, limit\u003d0)\n        self.assertFalse(mock_inst_by_uuid.called)\n        self.assertThat(response, HasLength(0))","commit_id":"f37709ace7f0269fdef785c78dda9e76ec093764"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"726098fe08f3515194ad0dc13bdc023d4c8c889b","unresolved":false,"context_lines":[{"line_number":19,"context_line":"import mock"},{"line_number":20,"context_line":"from oslo_config import cfg"},{"line_number":21,"context_line":"from oslo_serialization import jsonutils"},{"line_number":22,"context_line":"from oslo_utils import uuidutils"},{"line_number":23,"context_line":"import six"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"from nova.api.metadata import base as instance_metadata"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa32b979_9325e30b","line":22,"updated":"2015-06-19 13:40:49.000000000","message":"from testtools.matchers import HasLength\n\nShould be grouped here to be PEP8 compat [1]. tl;dr things should be grouped as:\n\n* standard library imports\n* related third party imports\n* local application/library specific imports\n\n[1] https://www.python.org/dev/peps/pep-0008/#imports","commit_id":"762fd4bb0e305206aa404ea400d27d59b7fdf3cc"},{"author":{"_account_id":8125,"name":"Michael Davies","email":"michael@the-davies.net","username":"mrda"},"change_message_id":"7debfee67c718911ae41767a9706c629badde964","unresolved":false,"context_lines":[{"line_number":19,"context_line":"import mock"},{"line_number":20,"context_line":"from oslo_config import cfg"},{"line_number":21,"context_line":"from oslo_serialization import jsonutils"},{"line_number":22,"context_line":"from oslo_utils import uuidutils"},{"line_number":23,"context_line":"import six"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"from nova.api.metadata import base as instance_metadata"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa32b979_f333e5f4","line":22,"in_reply_to":"fa32b979_9325e30b","updated":"2015-06-22 05:14:02.000000000","message":"Agreed.  +1","commit_id":"762fd4bb0e305206aa404ea400d27d59b7fdf3cc"}],"nova/virt/ironic/driver.py":[{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"f4e93110f2e4b5f27f96d449b3cefd1ed1c4d6b7","unresolved":false,"context_lines":[{"line_number":430,"context_line":"        \"\"\""},{"line_number":431,"context_line":"        # NOTE(lucasagomes): limit \u003d\u003d 0 is an indicator to continue"},{"line_number":432,"context_line":"        # pagination until there\u0027re no more values to be returned."},{"line_number":433,"context_line":"        try:"},{"line_number":434,"context_line":"            node_list \u003d self.ironicclient.call(\"node.list\", associated\u003dTrue,"},{"line_number":435,"context_line":"                                               limit\u003d0)"},{"line_number":436,"context_line":"        except exception.NovaException:"}],"source_content_type":"text/x-python","patch_set":1,"id":"5afe65bd_815692d0","line":433,"updated":"2015-06-03 00:17:09.000000000","message":"I think a helper function would be better for this.  To reduce the duplication.\n\nMaybe something like:\n\n def _get_node_list(self, **kwargs):\n   try:\n      node_list \u003d self.ironicclient.call(\u0027node.list\u0027, **kwargs)\n  except:\n    node_list \u003d []\n  return node_list","commit_id":"f639b332fcf935096de51e87c45d3e38e27ecb17"},{"author":{"_account_id":8125,"name":"Michael Davies","email":"michael@the-davies.net","username":"mrda"},"change_message_id":"6b41c96d95e5f81e980aaeba44150840d7b245ef","unresolved":false,"context_lines":[{"line_number":430,"context_line":"        \"\"\""},{"line_number":431,"context_line":"        # NOTE(lucasagomes): limit \u003d\u003d 0 is an indicator to continue"},{"line_number":432,"context_line":"        # pagination until there\u0027re no more values to be returned."},{"line_number":433,"context_line":"        try:"},{"line_number":434,"context_line":"            node_list \u003d self.ironicclient.call(\"node.list\", associated\u003dTrue,"},{"line_number":435,"context_line":"                                               limit\u003d0)"},{"line_number":436,"context_line":"        except exception.NovaException:"}],"source_content_type":"text/x-python","patch_set":1,"id":"5afe65bd_e229c438","line":433,"in_reply_to":"5afe65bd_815692d0","updated":"2015-06-03 02:19:59.000000000","message":"+1","commit_id":"f639b332fcf935096de51e87c45d3e38e27ecb17"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"47ea005842b42d66b83930ead61b182bdb51c154","unresolved":false,"context_lines":[{"line_number":446,"context_line":"        try:"},{"line_number":447,"context_line":"            node_list \u003d self.ironicclient.call(\"node.list\", **kwargs)"},{"line_number":448,"context_line":"        except exception.NovaException:"},{"line_number":449,"context_line":"            node_list \u003d []"},{"line_number":450,"context_line":"        return node_list"},{"line_number":451,"context_line":""},{"line_number":452,"context_line":"    def list_instances(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3afb71cf_29c3de58","line":449,"updated":"2015-06-09 23:32:19.000000000","message":"Now probably need a unit test for this.  That tests an exception patch and success path.","commit_id":"4a1ab974eef5358ff5b35a35f2331e5e24b08b1d"},{"author":{"_account_id":13790,"name":"Zhenzan Zhou","email":"linuxosa@gmail.com","username":"zhenzanzhou"},"change_message_id":"3e85de13d99d8200d558e8079354c9eae40fa425","unresolved":false,"context_lines":[{"line_number":446,"context_line":"        try:"},{"line_number":447,"context_line":"            node_list \u003d self.ironicclient.call(\"node.list\", **kwargs)"},{"line_number":448,"context_line":"        except exception.NovaException:"},{"line_number":449,"context_line":"            node_list \u003d []"},{"line_number":450,"context_line":"        return node_list"},{"line_number":451,"context_line":""},{"line_number":452,"context_line":"    def list_instances(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3afb71cf_4dce232f","line":449,"in_reply_to":"3afb71cf_29c3de58","updated":"2015-06-10 01:24:19.000000000","message":"Thank you, John. I thought it has unit test cases. But I just found there is no exception path for list_instances, only exception path for client_wrapper.call(). I\u0027ll add one.","commit_id":"4a1ab974eef5358ff5b35a35f2331e5e24b08b1d"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"0d8d942e21dd9305d03c62c05377856550850f46","unresolved":false,"context_lines":[{"line_number":445,"context_line":"        \"\"\""},{"line_number":446,"context_line":"        try:"},{"line_number":447,"context_line":"            node_list \u003d self.ironicclient.call(\"node.list\", **kwargs)"},{"line_number":448,"context_line":"        except exception.NovaException:"},{"line_number":449,"context_line":"            node_list \u003d []"},{"line_number":450,"context_line":"        return node_list"},{"line_number":451,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"fa32b979_6e253807","line":448,"updated":"2015-06-17 16:43:47.000000000","message":"Just pointing out to other reviewers:\n\nWe already log a message saying we can\u0027t contact the Ironic API before raising the NovaException[1]\n\n[1] https://github.com/openstack/nova/blob/master/nova/virt/ironic/client_wrapper.py#L140-L142","commit_id":"f37709ace7f0269fdef785c78dda9e76ec093764"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"388c98c1cf58e69b1b26dd229df383aa086d6095","unresolved":false,"context_lines":[{"line_number":446,"context_line":"        try:"},{"line_number":447,"context_line":"            node_list \u003d self.ironicclient.call(\"node.list\", **kwargs)"},{"line_number":448,"context_line":"        except exception.NovaException:"},{"line_number":449,"context_line":"            node_list \u003d []"},{"line_number":450,"context_line":"        return node_list"},{"line_number":451,"context_line":""},{"line_number":452,"context_line":"    def list_instances(self):"}],"source_content_type":"text/x-python","patch_set":5,"id":"9a41bdd9_e6dc7848","line":449,"updated":"2015-07-17 16:36:39.000000000","message":"Should we not think about logging a warning here?","commit_id":"cce06a1e9855d9eed3f7c653200853f23466d791"}]}