)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b2cf8fa2cb8c873930caea972111c6a39267d465","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix a wrong argument of delete_keys method"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add \u0027QoSSpecs\u0027 to an argument of get_notifier()"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Closes-Bug: #1443340"},{"line_number":12,"context_line":"Change-Id: Ie53fdd41486fdb7d0694a4a1b7015ba7bcd3c161"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"fa32b979_ef20e24b","line":9,"updated":"2015-06-15 12:23:16.000000000","message":"I think this commit message is a little confusing, because first you talk about a wrong argument and then adding an argument.\n\nMaybe you could rephrase it and be more specific. Like:\n\n Method delete_keys in class QoSSpecsController is missing argument \u0027QoSSpecs\u0027 when calling get_notifier. \n This patch adds missing argument.","commit_id":"b6e87caa2a1dafb6d225052229bc405d19e843ff"},{"author":{"_account_id":14162,"name":"Daisuke Fujita","email":"fuzita.daisuke@jp.fujitsu.com","username":"daisukefujita"},"change_message_id":"e5d4d05a711da1eebb64a8a5943d0d79a9ec6b89","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix a wrong argument of delete_keys method"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add \u0027QoSSpecs\u0027 to an argument of get_notifier()"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Closes-Bug: #1443340"},{"line_number":12,"context_line":"Change-Id: Ie53fdd41486fdb7d0694a4a1b7015ba7bcd3c161"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"fa32b979_5c21674c","line":9,"in_reply_to":"fa32b979_ef20e24b","updated":"2015-06-16 05:45:12.000000000","message":"Thank you for your review.\n\nThe message which you suggested is better than this commit message.\nI\u0027ll change this.","commit_id":"b6e87caa2a1dafb6d225052229bc405d19e843ff"}],"cinder/api/contrib/qos_specs_manage.py":[{"author":{"_account_id":16708,"name":"Kendall Nelson","display_name":"Kendall (diablo_rojo)","email":"kennelson11@gmail.com","username":"kjnelson"},"change_message_id":"36521b05128a902a2b451b4121b9e24109d96dec","unresolved":false,"context_lines":[{"line_number":264,"context_line":"        try:"},{"line_number":265,"context_line":"            qos_specs.delete_keys(context, id, keys)"},{"line_number":266,"context_line":"            notifier_info \u003d dict(id\u003did)"},{"line_number":267,"context_line":"            rpc.get_notifier(\u0027QoSSpecs\u0027).info(context, \u0027qos_specs.delete_keys\u0027,"},{"line_number":268,"context_line":"                                              notifier_info)"},{"line_number":269,"context_line":"        except exception.QoSSpecsNotFound as err:"},{"line_number":270,"context_line":"            notifier_err \u003d dict(id\u003did, error_message\u003derr)"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_c83815f2","line":267,"updated":"2015-07-01 16:03:18.000000000","message":"I\u0027m confused about which argument QoSSpecs is since get_notifier takes three arguments. I think just passing one in defaults to being the first argument which would be service? Setting it to a variable \"service\" and then passing that in would be helpful.","commit_id":"46cdbba12c1caae07f21b3cf74b559a3bd72caee"},{"author":{"_account_id":14162,"name":"Daisuke Fujita","email":"fuzita.daisuke@jp.fujitsu.com","username":"daisukefujita"},"change_message_id":"a92ecb7524789e822e56713560c7937bf2ba1ef3","unresolved":false,"context_lines":[{"line_number":264,"context_line":"        try:"},{"line_number":265,"context_line":"            qos_specs.delete_keys(context, id, keys)"},{"line_number":266,"context_line":"            notifier_info \u003d dict(id\u003did)"},{"line_number":267,"context_line":"            rpc.get_notifier(\u0027QoSSpecs\u0027).info(context, \u0027qos_specs.delete_keys\u0027,"},{"line_number":268,"context_line":"                                              notifier_info)"},{"line_number":269,"context_line":"        except exception.QoSSpecsNotFound as err:"},{"line_number":270,"context_line":"            notifier_err \u003d dict(id\u003did, error_message\u003derr)"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_92b9b617","line":267,"updated":"2015-07-02 08:08:39.000000000","message":"This patch just unified method form like other methods in this class.","commit_id":"46cdbba12c1caae07f21b3cf74b559a3bd72caee"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b9fe5133365ee06d47ef194b530c60b128bb4035","unresolved":false,"context_lines":[{"line_number":264,"context_line":"        try:"},{"line_number":265,"context_line":"            qos_specs.delete_keys(context, id, keys)"},{"line_number":266,"context_line":"            notifier_info \u003d dict(id\u003did)"},{"line_number":267,"context_line":"            rpc.get_notifier(\u0027QoSSpecs\u0027).info(context, \u0027qos_specs.delete_keys\u0027,"},{"line_number":268,"context_line":"                                              notifier_info)"},{"line_number":269,"context_line":"        except exception.QoSSpecsNotFound as err:"},{"line_number":270,"context_line":"            notifier_err \u003d dict(id\u003did, error_message\u003derr)"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_1f306ae7","line":267,"in_reply_to":"ba3cc151_c83815f2","updated":"2015-07-01 18:15:09.000000000","message":"It is being passed as a position argument, so yes, it would be the first argument.\n\nAnyway, on line 109 you can see it used in a similar way.","commit_id":"46cdbba12c1caae07f21b3cf74b559a3bd72caee"}],"cinder/tests/unit/api/contrib/test_qos_specs_manage.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b2cf8fa2cb8c873930caea972111c6a39267d465","unresolved":false,"context_lines":[{"line_number":274,"context_line":""},{"line_number":275,"context_line":"    @mock.patch(\u0027cinder.volume.qos_specs.delete_keys\u0027,"},{"line_number":276,"context_line":"                side_effect\u003dreturn_qos_specs_delete_keys)"},{"line_number":277,"context_line":"    def test_qos_specs_delete_keys_get_notifier(self, mock_qos_delete_keys):"},{"line_number":278,"context_line":"        body \u003d {\"keys\": [\u0027bar\u0027, \u0027zoo\u0027]}"},{"line_number":279,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027/v2/fake/qos-specs/666/delete_keys\u0027)"},{"line_number":280,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_2f832ace","line":277,"updated":"2015-06-15 12:23:16.000000000","message":"A docstring explaining what the test is going to test would be nice.","commit_id":"b6e87caa2a1dafb6d225052229bc405d19e843ff"},{"author":{"_account_id":14162,"name":"Daisuke Fujita","email":"fuzita.daisuke@jp.fujitsu.com","username":"daisukefujita"},"change_message_id":"e5d4d05a711da1eebb64a8a5943d0d79a9ec6b89","unresolved":false,"context_lines":[{"line_number":274,"context_line":""},{"line_number":275,"context_line":"    @mock.patch(\u0027cinder.volume.qos_specs.delete_keys\u0027,"},{"line_number":276,"context_line":"                side_effect\u003dreturn_qos_specs_delete_keys)"},{"line_number":277,"context_line":"    def test_qos_specs_delete_keys_get_notifier(self, mock_qos_delete_keys):"},{"line_number":278,"context_line":"        body \u003d {\"keys\": [\u0027bar\u0027, \u0027zoo\u0027]}"},{"line_number":279,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027/v2/fake/qos-specs/666/delete_keys\u0027)"},{"line_number":280,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_5c2647cf","line":277,"in_reply_to":"fa32b979_2f832ace","updated":"2015-06-16 05:45:12.000000000","message":"Thank you for your comment.","commit_id":"b6e87caa2a1dafb6d225052229bc405d19e843ff"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"2bd459ec54445755c438a37dccaf2478ae099f7f","unresolved":false,"context_lines":[{"line_number":279,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027/v2/fake/qos-specs/666/delete_keys\u0027)"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"        notifier \u003d fake_notifier.get_fake_notifier(\u0027QoSSpecs\u0027)"},{"line_number":282,"context_line":"        with mock.patch(\u0027cinder.rpc.get_notifier\u0027, return_value\u003dnotifier):"},{"line_number":283,"context_line":"            self.controller.delete_keys(req, \u0027666\u0027, body)"},{"line_number":284,"context_line":"            self.assertEqual(1, notifier.get_notification_count())"},{"line_number":285,"context_line":"            self.assertEqual("}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_bc72287b","line":282,"updated":"2015-06-15 04:18:48.000000000","message":"I am doubting the accuracy of this test cases.\nIf you replace Line 282 with\nwith mock.patch(\u0027cinder.rpc.get_notifier\u0027, return_value\u003dnotifier) as mock_notifier:\n\nand add one line above Line 283:\nmock_notifier.assert_called_with(\u0027QoSSpecs\u0027)\n\nthen the test will fail reporting mock_notifier not called.\nHowever, we are expecting it is called.\n\nIt is possible all the test cases related to test_qos_specs_delete_keys_* have this issue, which is rpc.get_notifier not called.","commit_id":"b6e87caa2a1dafb6d225052229bc405d19e843ff"},{"author":{"_account_id":14162,"name":"Daisuke Fujita","email":"fuzita.daisuke@jp.fujitsu.com","username":"daisukefujita"},"change_message_id":"e5d4d05a711da1eebb64a8a5943d0d79a9ec6b89","unresolved":false,"context_lines":[{"line_number":279,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027/v2/fake/qos-specs/666/delete_keys\u0027)"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"        notifier \u003d fake_notifier.get_fake_notifier(\u0027QoSSpecs\u0027)"},{"line_number":282,"context_line":"        with mock.patch(\u0027cinder.rpc.get_notifier\u0027, return_value\u003dnotifier):"},{"line_number":283,"context_line":"            self.controller.delete_keys(req, \u0027666\u0027, body)"},{"line_number":284,"context_line":"            self.assertEqual(1, notifier.get_notification_count())"},{"line_number":285,"context_line":"            self.assertEqual("}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_1c82ff94","line":282,"updated":"2015-06-16 05:45:12.000000000","message":"It is very kind of you.\n\nI\u0027ll have a think about it.","commit_id":"b6e87caa2a1dafb6d225052229bc405d19e843ff"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b2cf8fa2cb8c873930caea972111c6a39267d465","unresolved":false,"context_lines":[{"line_number":279,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027/v2/fake/qos-specs/666/delete_keys\u0027)"},{"line_number":280,"context_line":""},{"line_number":281,"context_line":"        notifier \u003d fake_notifier.get_fake_notifier(\u0027QoSSpecs\u0027)"},{"line_number":282,"context_line":"        with mock.patch(\u0027cinder.rpc.get_notifier\u0027, return_value\u003dnotifier):"},{"line_number":283,"context_line":"            self.controller.delete_keys(req, \u0027666\u0027, body)"},{"line_number":284,"context_line":"            self.assertEqual(1, notifier.get_notification_count())"},{"line_number":285,"context_line":"            self.assertEqual("}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_aff2da98","line":282,"updated":"2015-06-15 12:23:16.000000000","message":"You should add autospec\u003dTrue to the mock so it checks arguments checked and raise TypeError on error.\n\nYou should as well assign the mock to a variable \n\n with mock.patch(\u0027cinder.rpc.get_notifier\u0027, return_value\u003dnotifier, autospec\u003dTrue) as mock_get_notifier:\n\nAnd then you need to check that it was properly called:\n\n mock_get_notifier.assert_called_once_with(\u0027QoSSpecs\u0027)","commit_id":"b6e87caa2a1dafb6d225052229bc405d19e843ff"}]}