)]}'
{"nova/tests/unit/virt/ironic/test_driver.py":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"5423bd904e1824704df08a2db790ca27f5edd065","unresolved":false,"context_lines":[{"line_number":884,"context_line":"                                    flavor\u003dflavor)"},{"line_number":885,"context_line":"        # assert node.update is not called"},{"line_number":886,"context_line":"        self.assertFalse(mock_update.called)"},{"line_number":887,"context_line":""},{"line_number":888,"context_line":"    @mock.patch.object(FAKE_CLIENT.node, \u0027update\u0027)"},{"line_number":889,"context_line":"    def test__cleanup_deploy_without_flavor(self, mock_update):"},{"line_number":890,"context_line":"        node \u003d ironic_utils.get_test_node(driver\u003d\u0027fake\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_82e37aa3","line":887,"updated":"2015-07-29 14:24:52.000000000","message":"maybe assert that mock_validate was called?","commit_id":"47ac53a7343a692343b8158ae53839099128edc6"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"48c98f8d1b6cf76226a27c2e2ec11384500de1d6","unresolved":false,"context_lines":[{"line_number":884,"context_line":"                                    flavor\u003dflavor)"},{"line_number":885,"context_line":"        # assert node.update is not called"},{"line_number":886,"context_line":"        self.assertFalse(mock_update.called)"},{"line_number":887,"context_line":""},{"line_number":888,"context_line":"    @mock.patch.object(FAKE_CLIENT.node, \u0027update\u0027)"},{"line_number":889,"context_line":"    def test__cleanup_deploy_without_flavor(self, mock_update):"},{"line_number":890,"context_line":"        node \u003d ironic_utils.get_test_node(driver\u003d\u0027fake\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_3d8ac71b","line":887,"in_reply_to":"3a50d1a3_82e37aa3","updated":"2015-07-29 14:36:12.000000000","message":"Done","commit_id":"47ac53a7343a692343b8158ae53839099128edc6"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"bf81c66c2f953d01c7bf5ac955cbe74acbd6e20c","unresolved":false,"context_lines":[{"line_number":1171,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.ctx, node\u003dnode_uuid)"},{"line_number":1172,"context_line":""},{"line_number":1173,"context_line":"        mock_node.get_by_instance_uuid.side_effect \u003d (node,"},{"line_number":1174,"context_line":"            ironic_exception.NotFound())"},{"line_number":1175,"context_line":"        self.driver.destroy(self.ctx, instance, None, None)"},{"line_number":1176,"context_line":"        mock_node.set_provision_state.assert_called_once_with(node_uuid,"},{"line_number":1177,"context_line":"                                                              \u0027deleted\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_bddd77e1","line":1174,"updated":"2015-07-29 14:34:37.000000000","message":"You have two new tests for the same behavior, but mocking that behavior different ways. Seems like it should be consistent.\n\nI prefer mocking _validate_instance_and_node here because it could be unclear to the reader why this side effect causes this behavior (or at least they have more code to trace...)","commit_id":"47ac53a7343a692343b8158ae53839099128edc6"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"48c98f8d1b6cf76226a27c2e2ec11384500de1d6","unresolved":false,"context_lines":[{"line_number":1171,"context_line":"        instance \u003d fake_instance.fake_instance_obj(self.ctx, node\u003dnode_uuid)"},{"line_number":1172,"context_line":""},{"line_number":1173,"context_line":"        mock_node.get_by_instance_uuid.side_effect \u003d (node,"},{"line_number":1174,"context_line":"            ironic_exception.NotFound())"},{"line_number":1175,"context_line":"        self.driver.destroy(self.ctx, instance, None, None)"},{"line_number":1176,"context_line":"        mock_node.set_provision_state.assert_called_once_with(node_uuid,"},{"line_number":1177,"context_line":"                                                              \u0027deleted\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_7d7fefaa","line":1174,"in_reply_to":"3a50d1a3_bddd77e1","updated":"2015-07-29 14:36:12.000000000","message":"++\n\nI think I could test _unprovision directly instead of going through destroy()","commit_id":"47ac53a7343a692343b8158ae53839099128edc6"}],"nova/virt/ironic/driver.py":[{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"da291ba5130d79ffd29451fc12aa26d8112f09d2","unresolved":false,"context_lines":[{"line_number":818,"context_line":"            except exception.InstanceNotFound:"},{"line_number":819,"context_line":"                LOG.debug(\"Instance already removed from Ironic\","},{"line_number":820,"context_line":"                          instance\u003dinstance)"},{"line_number":821,"context_line":"                raise loopingcall.LoopingCallDone()"},{"line_number":822,"context_line":"            if node.provision_state in (ironic_states.NOSTATE,"},{"line_number":823,"context_line":"                                        ironic_states.CLEANING,"},{"line_number":824,"context_line":"                                        ironic_states.CLEANFAIL,"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a50d1a3_f5532fe0","line":821,"updated":"2015-07-29 04:56:19.000000000","message":"The corresponding test for this change seems necessary.\n\nor better to avoid to change here like the calls which are not covered with this patch.","commit_id":"be7c979db913561f291c8a023513e96fdd6e4b23"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"96ca01eba2c4599de5d5cb971c7f41044c6c1e39","unresolved":false,"context_lines":[{"line_number":818,"context_line":"            except exception.InstanceNotFound:"},{"line_number":819,"context_line":"                LOG.debug(\"Instance already removed from Ironic\","},{"line_number":820,"context_line":"                          instance\u003dinstance)"},{"line_number":821,"context_line":"                raise loopingcall.LoopingCallDone()"},{"line_number":822,"context_line":"            if node.provision_state in (ironic_states.NOSTATE,"},{"line_number":823,"context_line":"                                        ironic_states.CLEANING,"},{"line_number":824,"context_line":"                                        ironic_states.CLEANFAIL,"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a50d1a3_8aaba574","line":821,"in_reply_to":"3a50d1a3_8a1ba572","updated":"2015-07-29 08:00:05.000000000","message":"Sorry for my unclear comment.\n\nAccording the patch title, I guess the main purpose of this patch is for _cleanup_deploy() method. then I guessed the corresponding test for this part doesn\u0027t exist. So it also is fine for me even if removing this part of change instead of adding the corresponding test.","commit_id":"be7c979db913561f291c8a023513e96fdd6e4b23"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"164db400c2da698d8094ca01744e39001676884b","unresolved":false,"context_lines":[{"line_number":818,"context_line":"            except exception.InstanceNotFound:"},{"line_number":819,"context_line":"                LOG.debug(\"Instance already removed from Ironic\","},{"line_number":820,"context_line":"                          instance\u003dinstance)"},{"line_number":821,"context_line":"                raise loopingcall.LoopingCallDone()"},{"line_number":822,"context_line":"            if node.provision_state in (ironic_states.NOSTATE,"},{"line_number":823,"context_line":"                                        ironic_states.CLEANING,"},{"line_number":824,"context_line":"                                        ironic_states.CLEANFAIL,"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a50d1a3_eb37cbe0","line":821,"in_reply_to":"3a50d1a3_8aaba574","updated":"2015-07-29 09:10:56.000000000","message":"Hi I\u0027m going to add a test to it.\n\nRe second point. The patch is about the destroy() which includes _cleanup_deploy(), _unprovision(), etc... Sorry if the commit message is unclear. What I mentioned there is the clean up that happens in Ironic (the service).","commit_id":"be7c979db913561f291c8a023513e96fdd6e4b23"},{"author":{"_account_id":8125,"name":"Michael Davies","email":"michael@the-davies.net","username":"mrda"},"change_message_id":"dd9c5960edf79033dafc0287c3af55890ca24980","unresolved":false,"context_lines":[{"line_number":818,"context_line":"            except exception.InstanceNotFound:"},{"line_number":819,"context_line":"                LOG.debug(\"Instance already removed from Ironic\","},{"line_number":820,"context_line":"                          instance\u003dinstance)"},{"line_number":821,"context_line":"                raise loopingcall.LoopingCallDone()"},{"line_number":822,"context_line":"            if node.provision_state in (ironic_states.NOSTATE,"},{"line_number":823,"context_line":"                                        ironic_states.CLEANING,"},{"line_number":824,"context_line":"                                        ironic_states.CLEANFAIL,"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a50d1a3_8a1ba572","line":821,"in_reply_to":"3a50d1a3_f5532fe0","updated":"2015-07-29 07:51:58.000000000","message":"I agree, we should have a corresponding test for this too (I\u0027m not sure about Ken\u0027ichi\u0027s second point though)","commit_id":"be7c979db913561f291c8a023513e96fdd6e4b23"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"5423bd904e1824704df08a2db790ca27f5edd065","unresolved":false,"context_lines":[{"line_number":371,"context_line":""},{"line_number":372,"context_line":"        # TODO(lucasagomes): This code is here for backwards compatibility"},{"line_number":373,"context_line":"        # with old versions of Ironic that won\u0027t clean up the instance"},{"line_number":374,"context_line":"        # association as part of the node\u0027s tears down. Should be removed"},{"line_number":375,"context_line":"        # on the next cycle (M)."},{"line_number":376,"context_line":"        patch.append({\u0027op\u0027: \u0027remove\u0027, \u0027path\u0027: \u0027/instance_uuid\u0027})"},{"line_number":377,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_51946235","line":374,"updated":"2015-07-29 14:24:52.000000000","message":"nit: s/tears/tear/\n\n(only if you do another revision)","commit_id":"47ac53a7343a692343b8158ae53839099128edc6"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"48c98f8d1b6cf76226a27c2e2ec11384500de1d6","unresolved":false,"context_lines":[{"line_number":371,"context_line":""},{"line_number":372,"context_line":"        # TODO(lucasagomes): This code is here for backwards compatibility"},{"line_number":373,"context_line":"        # with old versions of Ironic that won\u0027t clean up the instance"},{"line_number":374,"context_line":"        # association as part of the node\u0027s tears down. Should be removed"},{"line_number":375,"context_line":"        # on the next cycle (M)."},{"line_number":376,"context_line":"        patch.append({\u0027op\u0027: \u0027remove\u0027, \u0027path\u0027: \u0027/instance_uuid\u0027})"},{"line_number":377,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a50d1a3_bd7ed716","line":374,"in_reply_to":"3a50d1a3_51946235","updated":"2015-07-29 14:36:12.000000000","message":"Done","commit_id":"47ac53a7343a692343b8158ae53839099128edc6"}]}