)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"1abe983f2fa6963ca98c0f5db5e7467e7f5b4e6b","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Amador Pahim \u003capahim@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-06-25 16:52:21 -0300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Do not accept qos name with only invisible characters"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Closes-bug: #1465967"},{"line_number":10,"context_line":"Signed-off-by: Amador Pahim \u003capahim@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"ba3cc151_a585815a","line":7,"updated":"2015-07-02 15:23:56.000000000","message":"Invisible is not the right word, spaces would be better.","commit_id":"d160ea61dda84ff0bd07305664cb8895fdd0edcd"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"1abe983f2fa6963ca98c0f5db5e7467e7f5b4e6b","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Do not accept qos name with only invisible characters"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Closes-bug: #1465967"},{"line_number":10,"context_line":"Signed-off-by: Amador Pahim \u003capahim@redhat.com\u003e"},{"line_number":11,"context_line":"Change-Id: I986edd2900ce057e6400125e1ccc0fd63a940401"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"ba3cc151_c5d9a558","line":9,"updated":"2015-07-02 15:23:56.000000000","message":"This needs an APIInpact flag since it will return an error when the name is an empty string, which was not doing before.","commit_id":"d160ea61dda84ff0bd07305664cb8895fdd0edcd"},{"author":{"_account_id":11343,"name":"Gal Sagie","email":"gal.sagie@gmail.com","username":"GalSagie"},"change_message_id":"6db72a5ca6e2f00413d5830b6b64fda4662f26cf","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Do not accept qos name with only spaces"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"APIImpact"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Closes-bug: #1465967"},{"line_number":12,"context_line":"Signed-off-by: Amador Pahim \u003capahim@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"ba3cc151_dd9cdc5f","line":9,"updated":"2015-07-04 11:55:07.000000000","message":"Not sure that i would call this \"APIImpact\", this is more a bug solving, i dont think anyone intended for all spaces to be valid name.\n\nAlso you wrote two same Signed-off-by, you can remove one if you resend this patch","commit_id":"7d4e11b50eaa1609865e24cf0818b811f53b69b9"},{"author":{"_account_id":1207,"name":"Duncan Thomas","email":"duncan.thomas@gmail.com","username":"duncan-thomas"},"change_message_id":"ca447a398a722f26499d7b3d0f75a035acf69804","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Do not accept qos name with only spaces"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"APIImpact"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"Closes-bug: #1465967"},{"line_number":12,"context_line":"Signed-off-by: Amador Pahim \u003capahim@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"ba3cc151_cf5ae92a","line":9,"updated":"2015-07-03 20:22:00.000000000","message":"Please add a reason for this change; if nothing else it makes writing release notes easier.\n\nNote that we accept other names with white-space only values in other places.","commit_id":"7d4e11b50eaa1609865e24cf0818b811f53b69b9"}],"cinder/api/contrib/qos_specs_manage.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"44bb40bd800be126d5d2c9440c901c94203b4b56","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        specs \u003d body[\u0027qos_specs\u0027]"},{"line_number":130,"context_line":"        name \u003d specs.get(\u0027name\u0027, None)"},{"line_number":131,"context_line":"        if name is None or name.strip() \u003d\u003d \"\":"},{"line_number":132,"context_line":"            msg \u003d _(\"Please specify a name for QoS specs.\")"},{"line_number":133,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"fa32b979_d1eff984","line":132,"updated":"2015-06-25 16:16:28.000000000","message":"This error message doesn\u0027t make sense for the failure when a whitespace-only name is provided.\n\nIMO we should leave this check as-is and add a second one below it with a different message indicating that the supplied name is invalid.","commit_id":"8a4113b8f372b69fa57fb485fa3a5b82f01b141d"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"1abe983f2fa6963ca98c0f5db5e7467e7f5b4e6b","unresolved":false,"context_lines":[{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        specs \u003d body[\u0027qos_specs\u0027]"},{"line_number":130,"context_line":"        name \u003d specs.get(\u0027name\u0027, None)"},{"line_number":131,"context_line":"        if name is None or name \u003d\u003d \"\":"},{"line_number":132,"context_line":"            msg \u003d _(\"Please specify a name for QoS specs.\")"},{"line_number":133,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":134,"context_line":"        elif name.strip() \u003d\u003d \"\":"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_af98021d","line":131,"updated":"2015-07-02 15:23:56.000000000","message":"You should remove the name \u003d\u003d \"\" portion from here, so it goes into the new elif you have added and you get a your new message that is better for that case.","commit_id":"d160ea61dda84ff0bd07305664cb8895fdd0edcd"},{"author":{"_account_id":11343,"name":"Gal Sagie","email":"gal.sagie@gmail.com","username":"GalSagie"},"change_message_id":"6db72a5ca6e2f00413d5830b6b64fda4662f26cf","unresolved":false,"context_lines":[{"line_number":131,"context_line":"        if name is None:"},{"line_number":132,"context_line":"            msg \u003d _(\"Please specify a name for QoS specs.\")"},{"line_number":133,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":134,"context_line":"        elif name.strip() \u003d\u003d \"\":"},{"line_number":135,"context_line":"            msg \u003d _(\"Please specify a valid name for QoS specs.\")"},{"line_number":136,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":137,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_9d9f3450","line":134,"updated":"2015-07-04 11:55:07.000000000","message":"i would put this in the same \"if\" , just to save some lines of code, the reason for both could be \"Please specify a valid name for QoS specs\" , but either way is ok","commit_id":"7d4e11b50eaa1609865e24cf0818b811f53b69b9"}],"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":"1abe983f2fa6963ca98c0f5db5e7467e7f5b4e6b","unresolved":false,"context_lines":[{"line_number":98,"context_line":"    elif name \u003d\u003d \"444\":"},{"line_number":99,"context_line":"        raise exception.InvalidQoSSpecs(reason\u003dname)"},{"line_number":100,"context_line":"    elif name \u003d\u003d \" \":"},{"line_number":101,"context_line":"        raise exception.InvalidQoSSpecs(reason\u003dname)"},{"line_number":102,"context_line":"    pass"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_6f6c3a08","line":101,"updated":"2015-07-02 15:23:56.000000000","message":"This elif is not needed, as it will not be called.  And if it were, it should be included together with above 444 case.  As in:\n\n elif name in (\"444\", \" \"):\n\nFor this to be called the the qos_specs_manage.py file would have to execute call on line 139:\n\n qos_specs.create(context, name, specs)\n\nBut your new code will give an error before that.","commit_id":"d160ea61dda84ff0bd07305664cb8895fdd0edcd"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"1abe983f2fa6963ca98c0f5db5e7467e7f5b4e6b","unresolved":false,"context_lines":[{"line_number":307,"context_line":"            self.assertEqual(1, notifier.get_notification_count())"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"    @mock.patch(\u0027cinder.volume.qos_specs.create\u0027,"},{"line_number":310,"context_line":"                side_effect\u003dreturn_qos_specs_create)"},{"line_number":311,"context_line":"    def test_create_blank_input(self, mock_qos_get_specs):"},{"line_number":312,"context_line":"        body \u003d {\"qos_specs\": {\"name\": \" \","},{"line_number":313,"context_line":"                              \"consumer\": \"invalid_consumer\"}}"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_32fd35b4","line":310,"updated":"2015-07-02 15:23:56.000000000","message":"You don\u0027t need this mock, since you\u0027ll never get to call that method in this test\u0027s execution","commit_id":"d160ea61dda84ff0bd07305664cb8895fdd0edcd"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"1abe983f2fa6963ca98c0f5db5e7467e7f5b4e6b","unresolved":false,"context_lines":[{"line_number":314,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027/v2/fake/qos-specs\u0027)"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"        notifier \u003d fake_notifier.get_fake_notifier()"},{"line_number":317,"context_line":"        with mock.patch(\u0027cinder.rpc.get_notifier\u0027, return_value\u003dnotifier):"},{"line_number":318,"context_line":"            self.assertRaises(webob.exc.HTTPBadRequest,"},{"line_number":319,"context_line":"                              self.controller.create, req, body)"},{"line_number":320,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_c57a0588","line":317,"updated":"2015-07-02 15:23:56.000000000","message":"You don\u0027t need this mock or the notifier variable for this test.  You would only need it if the code from line 142 in qos_specs_manage.py was executed.","commit_id":"d160ea61dda84ff0bd07305664cb8895fdd0edcd"}]}