)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"7296cd1b472d71049c3fed8fd148728e98ba2ed4","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"cinder list fails with \u0027name\u0027 sort key"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Because attribue `name` is python property, the sqlalchemy doesn\u0027t"},{"line_number":10,"context_line":"know what is `name`. We need to use `hybrid_property` provided by"},{"line_number":11,"context_line":"sqlalchemy.ext.hybrid to let sqlalchemy what the name means."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba3cc151_e6da53e9","line":9,"updated":"2015-07-06 13:44:13.000000000","message":"Need some wording changes: is a python property, sqlalchemy doesn\u0027t","commit_id":"47959a6b5d6533b70b166cb2f48041f69423caff"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"7296cd1b472d71049c3fed8fd148728e98ba2ed4","unresolved":false,"context_lines":[{"line_number":7,"context_line":"cinder list fails with \u0027name\u0027 sort key"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Because attribue `name` is python property, the sqlalchemy doesn\u0027t"},{"line_number":10,"context_line":"know what is `name`. We need to use `hybrid_property` provided by"},{"line_number":11,"context_line":"sqlalchemy.ext.hybrid to let sqlalchemy what the name means."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I9e285c7de2e860b251f881ab82d8d2d93e1191d2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ba3cc151_86990722","line":10,"updated":"2015-07-06 13:44:13.000000000","message":"We need to use the \u0027hybrid_property\u0027","commit_id":"47959a6b5d6533b70b166cb2f48041f69423caff"}],"cinder/db/sqlalchemy/models.py":[{"author":{"_account_id":10485,"name":"Ankit Agrawal","email":"ankitagrawal8188@gmail.com","username":"ankitagrawal"},"change_message_id":"f1bc430511ffd4d1b99979d5a90c02c92ea0b354","unresolved":false,"context_lines":[{"line_number":26,"context_line":"from sqlalchemy.ext.declarative import declarative_base"},{"line_number":27,"context_line":"from sqlalchemy import ForeignKey, DateTime, Boolean"},{"line_number":28,"context_line":"from sqlalchemy.orm import relationship, backref, validates"},{"line_number":29,"context_line":"from sqlalchemy.ext.hybrid import hybrid_property"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba3cc151_9eedd8f1","line":29,"updated":"2015-07-06 09:51:55.000000000","message":"Please arrange these imports in alphabetical order.","commit_id":"47959a6b5d6533b70b166cb2f48041f69423caff"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"5b5487efe4518988e7e34964a75319c50db7e31d","unresolved":false,"context_lines":[{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    @name.expression"},{"line_number":130,"context_line":"    def name(self):"},{"line_number":131,"context_line":"        return self.name_id"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"    ec2_id \u003d Column(Integer)"},{"line_number":134,"context_line":"    user_id \u003d Column(String(255))"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_654927b7","line":131,"updated":"2015-07-07 11:34:23.000000000","message":"I do not think the name of the volume should be name_id.\nIt should be the display name which is meaningful to the users.\nVolume id, and name_id are generated by the cinder service or the back-end, out of the control of the user.\nThe user can can only gives display name, which makes more sense to them.\nWhen one wants to query some volume by name, it does not make sense for them to input some uuids or part of the uuids.","commit_id":"4f00626a64972bd9d5a71327268e76c4519af7fc"},{"author":{"_account_id":2861,"name":"Vincent Hou","email":"shou@us.ibm.com","username":"houshengbo"},"change_message_id":"9f2d5cee5fe1d11d12fe7eb96020574269d6e82a","unresolved":false,"context_lines":[{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    @name.expression"},{"line_number":130,"context_line":"    def name(self):"},{"line_number":131,"context_line":"        return self.name_id"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"    ec2_id \u003d Column(Integer)"},{"line_number":134,"context_line":"    user_id \u003d Column(String(255))"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_85b0eb8c","line":131,"in_reply_to":"ba3cc151_654927b7","updated":"2015-07-07 11:36:48.000000000","message":"Check Gorka\u0027s comments for changes.","commit_id":"4f00626a64972bd9d5a71327268e76c4519af7fc"},{"author":{"_account_id":13747,"name":"Kuo-tung Kao","email":"jelly.k@inwinstack.com","username":"coding1314"},"change_message_id":"a783bcc4b9fec27cd17307768bc07b2fa5092e32","unresolved":false,"context_lines":[{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    @name.expression"},{"line_number":130,"context_line":"    def name(self):"},{"line_number":131,"context_line":"        return self.name_id"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"    ec2_id \u003d Column(Integer)"},{"line_number":134,"context_line":"    user_id \u003d Column(String(255))"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_609a7570","line":131,"in_reply_to":"ba3cc151_85b0eb8c","updated":"2015-07-07 11:50:11.000000000","message":"Understand. Maybe we need to add some comments for the name to avoid misunderstanding. IMO.","commit_id":"4f00626a64972bd9d5a71327268e76c4519af7fc"}],"cinder/tests/unit/api/v2/test_volumes.py":[{"author":{"_account_id":10485,"name":"Ankit Agrawal","email":"ankitagrawal8188@gmail.com","username":"ankitagrawal"},"change_message_id":"340f208880e5f8b0f1ddd1e1b7ad3c3ce0e24414","unresolved":false,"context_lines":[{"line_number":969,"context_line":"        https://bugs.launchpad.net/cinder/+bug/1404020"},{"line_number":970,"context_line":"        If cinder-api doesn\u0027t change from name to display_name in sort_keys,"},{"line_number":971,"context_line":"        the test case will raise error message like the bug report."},{"line_number":972,"context_line":"        \u0027\u0027\u0027"},{"line_number":973,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027/v2/volumes?sort\u003dname\u0027)"},{"line_number":974,"context_line":"        try:"},{"line_number":975,"context_line":"            self.controller.index(req)"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_abefd21c","line":972,"updated":"2015-07-08 07:36:29.000000000","message":"Please format newly added docstrings to meet hacking rules [1] because there is a patch [2] to fix such issues in existing codebase.\n\n[1] http://docs.openstack.org/developer/hacking/#docstrings\n[2] https://review.openstack.org/#/c/184820/","commit_id":"e12385d46fc5590ce39a7958a4120b544704aebf"},{"author":{"_account_id":13747,"name":"Kuo-tung Kao","email":"jelly.k@inwinstack.com","username":"coding1314"},"change_message_id":"832c7c997b32fc8485ec553d81d2101f65209556","unresolved":false,"context_lines":[{"line_number":969,"context_line":"        https://bugs.launchpad.net/cinder/+bug/1404020"},{"line_number":970,"context_line":"        If cinder-api doesn\u0027t change from name to display_name in sort_keys,"},{"line_number":971,"context_line":"        the test case will raise error message like the bug report."},{"line_number":972,"context_line":"        \u0027\u0027\u0027"},{"line_number":973,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027/v2/volumes?sort\u003dname\u0027)"},{"line_number":974,"context_line":"        try:"},{"line_number":975,"context_line":"            self.controller.index(req)"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_466abbee","line":972,"in_reply_to":"ba3cc151_abefd21c","updated":"2015-07-08 08:16:57.000000000","message":"Done","commit_id":"e12385d46fc5590ce39a7958a4120b544704aebf"},{"author":{"_account_id":10485,"name":"Ankit Agrawal","email":"ankitagrawal8188@gmail.com","username":"ankitagrawal"},"change_message_id":"340f208880e5f8b0f1ddd1e1b7ad3c3ce0e24414","unresolved":false,"context_lines":[{"line_number":975,"context_line":"            self.controller.index(req)"},{"line_number":976,"context_line":"        except sqlalchemy.exc.ArgumentError as e:"},{"line_number":977,"context_line":"            msg \u003d \"Database expected display_name, but found name. \" \\"},{"line_number":978,"context_line":"                  \"Exception: %s\" % six.text_type(e)"},{"line_number":979,"context_line":"            self.assertEqual(True, False, msg)"},{"line_number":980,"context_line":""},{"line_number":981,"context_line":"    def test_volume_index_limit_offset(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_eb663aba","line":978,"updated":"2015-07-08 07:36:29.000000000","message":"Please do not use back slash \"\\\" for line continuation, use () instead.","commit_id":"e12385d46fc5590ce39a7958a4120b544704aebf"},{"author":{"_account_id":13747,"name":"Kuo-tung Kao","email":"jelly.k@inwinstack.com","username":"coding1314"},"change_message_id":"832c7c997b32fc8485ec553d81d2101f65209556","unresolved":false,"context_lines":[{"line_number":975,"context_line":"            self.controller.index(req)"},{"line_number":976,"context_line":"        except sqlalchemy.exc.ArgumentError as e:"},{"line_number":977,"context_line":"            msg \u003d \"Database expected display_name, but found name. \" \\"},{"line_number":978,"context_line":"                  \"Exception: %s\" % six.text_type(e)"},{"line_number":979,"context_line":"            self.assertEqual(True, False, msg)"},{"line_number":980,"context_line":""},{"line_number":981,"context_line":"    def test_volume_index_limit_offset(self):"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_067913c8","line":978,"in_reply_to":"ba3cc151_eb663aba","updated":"2015-07-08 08:16:57.000000000","message":"Done","commit_id":"e12385d46fc5590ce39a7958a4120b544704aebf"},{"author":{"_account_id":10485,"name":"Ankit Agrawal","email":"ankitagrawal8188@gmail.com","username":"ankitagrawal"},"change_message_id":"340f208880e5f8b0f1ddd1e1b7ad3c3ce0e24414","unresolved":false,"context_lines":[{"line_number":1637,"context_line":"    @mock.patch(\u0027cinder.volume.api.API.get_all\u0027)"},{"line_number":1638,"context_line":"    def test_get_volumes_sort_by_name(self, get_all):"},{"line_number":1639,"context_line":"        \u0027\u0027\u0027When client said `name`, it means `display_name` in"},{"line_number":1640,"context_line":"        database field."},{"line_number":1641,"context_line":"        \u0027\u0027\u0027"},{"line_number":1642,"context_line":""},{"line_number":1643,"context_line":"        req \u003d mock.MagicMock()"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_0bc6fe99","line":1640,"updated":"2015-07-08 07:36:29.000000000","message":"Same as above.","commit_id":"e12385d46fc5590ce39a7958a4120b544704aebf"},{"author":{"_account_id":13747,"name":"Kuo-tung Kao","email":"jelly.k@inwinstack.com","username":"coding1314"},"change_message_id":"832c7c997b32fc8485ec553d81d2101f65209556","unresolved":false,"context_lines":[{"line_number":1637,"context_line":"    @mock.patch(\u0027cinder.volume.api.API.get_all\u0027)"},{"line_number":1638,"context_line":"    def test_get_volumes_sort_by_name(self, get_all):"},{"line_number":1639,"context_line":"        \u0027\u0027\u0027When client said `name`, it means `display_name` in"},{"line_number":1640,"context_line":"        database field."},{"line_number":1641,"context_line":"        \u0027\u0027\u0027"},{"line_number":1642,"context_line":""},{"line_number":1643,"context_line":"        req \u003d mock.MagicMock()"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_267697d7","line":1640,"in_reply_to":"ba3cc151_0bc6fe99","updated":"2015-07-08 08:16:57.000000000","message":"Done","commit_id":"e12385d46fc5590ce39a7958a4120b544704aebf"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"eacfc678ce2e5936d9d0514c2f0f013516379c66","unresolved":false,"context_lines":[{"line_number":967,"context_line":"    def test_volume_index_sort_by_name(self):"},{"line_number":968,"context_line":"        \"\"\"cinder list with \u0027name\u0027 as sort key."},{"line_number":969,"context_line":""},{"line_number":970,"context_line":"        The test case is used for detecting the bug."},{"line_number":971,"context_line":"        https://bugs.launchpad.net/cinder/+bug/1404020"},{"line_number":972,"context_line":"        If cinder-api doesn\u0027t change from name to display_name in sort_keys,"},{"line_number":973,"context_line":"        the test case will raise error message like the bug report."}],"source_content_type":"text/x-python","patch_set":7,"id":"ba3cc151_f15ce01a","line":970,"updated":"2015-07-08 10:51:22.000000000","message":"You could have just written bug #1404020 instead of the URL and everyone know where to look for that bug ;-)","commit_id":"8d4150d7c6e35eea85e44619282d4f75507320c3"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c759bf4f146f28768a886d86f517d6c9a96338d6","unresolved":false,"context_lines":[{"line_number":967,"context_line":"    def test_volume_index_sort_by_name(self):"},{"line_number":968,"context_line":"        \"\"\"cinder list with \u0027name\u0027 as sort key."},{"line_number":969,"context_line":""},{"line_number":970,"context_line":"        The test case is used for detecting the bug."},{"line_number":971,"context_line":"        https://bugs.launchpad.net/cinder/+bug/1404020"},{"line_number":972,"context_line":"        If cinder-api doesn\u0027t change from name to display_name in sort_keys,"},{"line_number":973,"context_line":"        the test case will raise error message like the bug report."}],"source_content_type":"text/x-python","patch_set":7,"id":"ba3cc151_17c6149a","line":970,"in_reply_to":"ba3cc151_31f7a88c","updated":"2015-07-08 11:43:54.000000000","message":"Good idea!  Better safe than sorry.  ;-)","commit_id":"8d4150d7c6e35eea85e44619282d4f75507320c3"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"332a3c63264fb80fa847ed7c04ad98b379a1d8c8","unresolved":false,"context_lines":[{"line_number":967,"context_line":"    def test_volume_index_sort_by_name(self):"},{"line_number":968,"context_line":"        \"\"\"cinder list with \u0027name\u0027 as sort key."},{"line_number":969,"context_line":""},{"line_number":970,"context_line":"        The test case is used for detecting the bug."},{"line_number":971,"context_line":"        https://bugs.launchpad.net/cinder/+bug/1404020"},{"line_number":972,"context_line":"        If cinder-api doesn\u0027t change from name to display_name in sort_keys,"},{"line_number":973,"context_line":"        the test case will raise error message like the bug report."}],"source_content_type":"text/x-python","patch_set":7,"id":"ba3cc151_f3ee1dec","line":970,"in_reply_to":"ba3cc151_31f7a88c","updated":"2015-07-08 13:36:13.000000000","message":"If I remember right, StoryBoard has been abandoned. I\u0027m sure some replacement will come along eventually, but we\u0027re probably safe for awhile.","commit_id":"8d4150d7c6e35eea85e44619282d4f75507320c3"},{"author":{"_account_id":13747,"name":"Kuo-tung Kao","email":"jelly.k@inwinstack.com","username":"coding1314"},"change_message_id":"814ff8e9c89e667f19120cca043834cb75ee0ee8","unresolved":false,"context_lines":[{"line_number":967,"context_line":"    def test_volume_index_sort_by_name(self):"},{"line_number":968,"context_line":"        \"\"\"cinder list with \u0027name\u0027 as sort key."},{"line_number":969,"context_line":""},{"line_number":970,"context_line":"        The test case is used for detecting the bug."},{"line_number":971,"context_line":"        https://bugs.launchpad.net/cinder/+bug/1404020"},{"line_number":972,"context_line":"        If cinder-api doesn\u0027t change from name to display_name in sort_keys,"},{"line_number":973,"context_line":"        the test case will raise error message like the bug report."}],"source_content_type":"text/x-python","patch_set":7,"id":"ba3cc151_31f7a88c","line":970,"in_reply_to":"ba3cc151_f15ce01a","updated":"2015-07-08 11:05:29.000000000","message":"It seems that openstack will use StoryBoard as bug tracker instead of launchpad.  I am not sure that we can use the bug id to find the bug in StoryBoard. So I just put the link instead of bug id.","commit_id":"8d4150d7c6e35eea85e44619282d4f75507320c3"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"71b37c42b99a8b25b8ca968469d058586561e1bd","unresolved":false,"context_lines":[{"line_number":979,"context_line":"            msg \u003d (\"Database expected display_name, but found name. \""},{"line_number":980,"context_line":"                   \"Exception: %s\") % six.text_type(e)"},{"line_number":981,"context_line":"            self.assertEqual(True, False, msg)"},{"line_number":982,"context_line":""},{"line_number":983,"context_line":"    def test_volume_index_limit_offset(self):"},{"line_number":984,"context_line":"        def stub_volume_get_all_by_project(context, project_id, marker, limit,"},{"line_number":985,"context_line":"                                           sort_keys\u003dNone, sort_dirs\u003dNone,"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba3cc151_368d49d3","line":982,"updated":"2015-07-08 16:30:03.000000000","message":"Please, use assertRaises instead try-except statement.\n\n\nhttps://docs.python.org/2/library/unittest.html#unittest.TestCase.assertRaises","commit_id":"8d4150d7c6e35eea85e44619282d4f75507320c3"},{"author":{"_account_id":13747,"name":"Kuo-tung Kao","email":"jelly.k@inwinstack.com","username":"coding1314"},"change_message_id":"b615fda0f9d604ebb9c70f2fa11576f4b58b87aa","unresolved":false,"context_lines":[{"line_number":979,"context_line":"            msg \u003d (\"Database expected display_name, but found name. \""},{"line_number":980,"context_line":"                   \"Exception: %s\") % six.text_type(e)"},{"line_number":981,"context_line":"            self.assertEqual(True, False, msg)"},{"line_number":982,"context_line":""},{"line_number":983,"context_line":"    def test_volume_index_limit_offset(self):"},{"line_number":984,"context_line":"        def stub_volume_get_all_by_project(context, project_id, marker, limit,"},{"line_number":985,"context_line":"                                           sort_keys\u003dNone, sort_dirs\u003dNone,"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba3cc151_a2f01a68","line":982,"in_reply_to":"ba3cc151_177fc615","updated":"2015-07-09 03:14:49.000000000","message":"@Ivan and @Sean\n\n\nNow I mock `sqlalchemy.orm.query.Query.all`, so the pattern `self.assertEqual(True, False)` had gone.","commit_id":"8d4150d7c6e35eea85e44619282d4f75507320c3"},{"author":{"_account_id":13747,"name":"Kuo-tung Kao","email":"jelly.k@inwinstack.com","username":"coding1314"},"change_message_id":"1e8536bca34dc53e1baf50f7cbe1a3ae37eee0ea","unresolved":false,"context_lines":[{"line_number":979,"context_line":"            msg \u003d (\"Database expected display_name, but found name. \""},{"line_number":980,"context_line":"                   \"Exception: %s\") % six.text_type(e)"},{"line_number":981,"context_line":"            self.assertEqual(True, False, msg)"},{"line_number":982,"context_line":""},{"line_number":983,"context_line":"    def test_volume_index_limit_offset(self):"},{"line_number":984,"context_line":"        def stub_volume_get_all_by_project(context, project_id, marker, limit,"},{"line_number":985,"context_line":"                                           sort_keys\u003dNone, sort_dirs\u003dNone,"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba3cc151_7cb159df","line":982,"in_reply_to":"ba3cc151_1c4e0d3b","updated":"2015-07-09 01:30:06.000000000","message":"Do you have any idea to solve the problem? Is it ok if I just write the code like the below?\n\n```\nreq \u003d fakes.HTTPRequest.blank(\u0027/v2/volumes?sort\u003dname\u0027)\nself.controller.index(req)\n```\n\nIf I use the above code, there are no assert function in the test case. It looks strange.\nBut it seems that no way to solve the problem. Or I just remove the test case? Any idea is welcomed. Thank you.","commit_id":"8d4150d7c6e35eea85e44619282d4f75507320c3"},{"author":{"_account_id":13747,"name":"Kuo-tung Kao","email":"jelly.k@inwinstack.com","username":"coding1314"},"change_message_id":"f769e1d229e42de7fc352d8e129b61baa59bbf9d","unresolved":false,"context_lines":[{"line_number":979,"context_line":"            msg \u003d (\"Database expected display_name, but found name. \""},{"line_number":980,"context_line":"                   \"Exception: %s\") % six.text_type(e)"},{"line_number":981,"context_line":"            self.assertEqual(True, False, msg)"},{"line_number":982,"context_line":""},{"line_number":983,"context_line":"    def test_volume_index_limit_offset(self):"},{"line_number":984,"context_line":"        def stub_volume_get_all_by_project(context, project_id, marker, limit,"},{"line_number":985,"context_line":"                                           sort_keys\u003dNone, sort_dirs\u003dNone,"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba3cc151_bc528111","line":982,"in_reply_to":"ba3cc151_368d49d3","updated":"2015-07-09 01:15:25.000000000","message":"assertRaises means it expects a exception is raised. But in the test case, we don\u0027t expect any exception is raised. The function we need is assertNotRaises instead of assertRaises. But I can\u0027t find any function like assertNotRaises. So I just use try-except.\n\n\nThe test case should not raise sqlalchemy.exc.ArgumentError. If it raise sqlalchemy.exc.ArgumentError, it means somebody breaks the code.","commit_id":"8d4150d7c6e35eea85e44619282d4f75507320c3"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"534ab88beb37449030c6775cc8e8cff70c1152e9","unresolved":false,"context_lines":[{"line_number":979,"context_line":"            msg \u003d (\"Database expected display_name, but found name. \""},{"line_number":980,"context_line":"                   \"Exception: %s\") % six.text_type(e)"},{"line_number":981,"context_line":"            self.assertEqual(True, False, msg)"},{"line_number":982,"context_line":""},{"line_number":983,"context_line":"    def test_volume_index_limit_offset(self):"},{"line_number":984,"context_line":"        def stub_volume_get_all_by_project(context, project_id, marker, limit,"},{"line_number":985,"context_line":"                                           sort_keys\u003dNone, sort_dirs\u003dNone,"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba3cc151_fcffa9ff","line":982,"in_reply_to":"ba3cc151_7cb159df","updated":"2015-07-09 01:44:13.000000000","message":"Since the change was just in _get_volumes I actually think the unit test you added below is sufficient. I would say just remove this one.","commit_id":"8d4150d7c6e35eea85e44619282d4f75507320c3"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"d5fe3772d16d81b42053e0bbb74a03271bdd9523","unresolved":false,"context_lines":[{"line_number":979,"context_line":"            msg \u003d (\"Database expected display_name, but found name. \""},{"line_number":980,"context_line":"                   \"Exception: %s\") % six.text_type(e)"},{"line_number":981,"context_line":"            self.assertEqual(True, False, msg)"},{"line_number":982,"context_line":""},{"line_number":983,"context_line":"    def test_volume_index_limit_offset(self):"},{"line_number":984,"context_line":"        def stub_volume_get_all_by_project(context, project_id, marker, limit,"},{"line_number":985,"context_line":"                                           sort_keys\u003dNone, sort_dirs\u003dNone,"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba3cc151_1c4e0d3b","line":982,"in_reply_to":"ba3cc151_bc528111","updated":"2015-07-09 01:20:23.000000000","message":"Unit tests test expected behavior. If you don\u0027t expect an exception to be raised you shouldn\u0027t have any of this handling. assertEqual(True, False) is not a good pattern to use for unit testing.","commit_id":"8d4150d7c6e35eea85e44619282d4f75507320c3"},{"author":{"_account_id":13747,"name":"Kuo-tung Kao","email":"jelly.k@inwinstack.com","username":"coding1314"},"change_message_id":"17d8b1d8e75b392ba4d796397be13ec544020fd5","unresolved":false,"context_lines":[{"line_number":979,"context_line":"            msg \u003d (\"Database expected display_name, but found name. \""},{"line_number":980,"context_line":"                   \"Exception: %s\") % six.text_type(e)"},{"line_number":981,"context_line":"            self.assertEqual(True, False, msg)"},{"line_number":982,"context_line":""},{"line_number":983,"context_line":"    def test_volume_index_limit_offset(self):"},{"line_number":984,"context_line":"        def stub_volume_get_all_by_project(context, project_id, marker, limit,"},{"line_number":985,"context_line":"                                           sort_keys\u003dNone, sort_dirs\u003dNone,"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba3cc151_177fc615","line":982,"in_reply_to":"ba3cc151_fcffa9ff","updated":"2015-07-09 02:04:06.000000000","message":"Ok. I added the test case in patch set 5 because the test case can reproduce the bug.\n\n\nBy the way, I will follow you comment tomorrow if no more comment comes.","commit_id":"8d4150d7c6e35eea85e44619282d4f75507320c3"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"9f9b4fa80c94f5845438ad375aa3401d43e415a6","unresolved":false,"context_lines":[{"line_number":972,"context_line":"        If cinder-api doesn\u0027t change from name to display_name in sort_keys,"},{"line_number":973,"context_line":"        the test case will raise error message like the bug report."},{"line_number":974,"context_line":"        \"\"\""},{"line_number":975,"context_line":"        expected_volumes \u003d [stubs.stub_volume(1, display_name\u003d\u0027vol1\u0027),"},{"line_number":976,"context_line":"                            stubs.stub_volume(2, display_name\u003d\u0027vol2\u0027)]"},{"line_number":977,"context_line":"        with mock.patch.object(sqlalchemy.orm.query.Query, \u0027all\u0027,"},{"line_number":978,"context_line":"                               return_value\u003dexpected_volumes):"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba3cc151_4f113651","line":975,"updated":"2015-07-09 04:03:44.000000000","message":"This name, \u0027expected_volumes\u0027, is misleading, as it suggests that this variable is being used to test which volumes are returned from invoking the method under test, while really it is being used to determine the return value of a mocked method invoked by the method under test.  I suggest \u0027fake_volume_list\u0027 or some such name for this variable.\n\nOr you could keep the name and actually assert that the returned volumes in the res_dict list are the same as these volumes (maybe convert the lists to sets, maybe there is a pre-made assert for list equality irrespective of order).","commit_id":"2a6a4a136574b63beab7fd6da329f2cbd8448e62"},{"author":{"_account_id":13747,"name":"Kuo-tung Kao","email":"jelly.k@inwinstack.com","username":"coding1314"},"change_message_id":"b87f9b6d4dee211fc129981b9436cc4979e65924","unresolved":false,"context_lines":[{"line_number":972,"context_line":"        If cinder-api doesn\u0027t change from name to display_name in sort_keys,"},{"line_number":973,"context_line":"        the test case will raise error message like the bug report."},{"line_number":974,"context_line":"        \"\"\""},{"line_number":975,"context_line":"        expected_volumes \u003d [stubs.stub_volume(1, display_name\u003d\u0027vol1\u0027),"},{"line_number":976,"context_line":"                            stubs.stub_volume(2, display_name\u003d\u0027vol2\u0027)]"},{"line_number":977,"context_line":"        with mock.patch.object(sqlalchemy.orm.query.Query, \u0027all\u0027,"},{"line_number":978,"context_line":"                               return_value\u003dexpected_volumes):"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba3cc151_eff04a93","line":975,"in_reply_to":"ba3cc151_4f113651","updated":"2015-07-09 04:46:43.000000000","message":"Done","commit_id":"2a6a4a136574b63beab7fd6da329f2cbd8448e62"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"9f9b4fa80c94f5845438ad375aa3401d43e415a6","unresolved":false,"context_lines":[{"line_number":979,"context_line":"            req \u003d fakes.HTTPRequest.blank(\u0027/v2/volumes?sort\u003dname\u0027)"},{"line_number":980,"context_line":"            res_dict \u003d self.controller.index(req)"},{"line_number":981,"context_line":"            volumes \u003d res_dict[\u0027volumes\u0027]"},{"line_number":982,"context_line":"            self.assertEqual(len(volumes), 2)"},{"line_number":983,"context_line":""},{"line_number":984,"context_line":"    def test_volume_index_limit_offset(self):"},{"line_number":985,"context_line":"        def stub_volume_get_all_by_project(context, project_id, marker, limit,"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba3cc151_8f217ea0","line":982,"updated":"2015-07-09 04:03:44.000000000","message":"assert order should be expected, actual.  Looks like other tests in this file may not be doing this but IMO we should do the right thing when we add new tests.","commit_id":"2a6a4a136574b63beab7fd6da329f2cbd8448e62"},{"author":{"_account_id":13747,"name":"Kuo-tung Kao","email":"jelly.k@inwinstack.com","username":"coding1314"},"change_message_id":"b87f9b6d4dee211fc129981b9436cc4979e65924","unresolved":false,"context_lines":[{"line_number":979,"context_line":"            req \u003d fakes.HTTPRequest.blank(\u0027/v2/volumes?sort\u003dname\u0027)"},{"line_number":980,"context_line":"            res_dict \u003d self.controller.index(req)"},{"line_number":981,"context_line":"            volumes \u003d res_dict[\u0027volumes\u0027]"},{"line_number":982,"context_line":"            self.assertEqual(len(volumes), 2)"},{"line_number":983,"context_line":""},{"line_number":984,"context_line":"    def test_volume_index_limit_offset(self):"},{"line_number":985,"context_line":"        def stub_volume_get_all_by_project(context, project_id, marker, limit,"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba3cc151_af1a427b","line":982,"in_reply_to":"ba3cc151_8f217ea0","updated":"2015-07-09 04:46:43.000000000","message":"Finally, I follow your comment. In the file, there are two style in the file. Maybe we need to clean up the file.","commit_id":"2a6a4a136574b63beab7fd6da329f2cbd8448e62"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"b5f9af610e0d8eaa1650916669a5e1ed18519db4","unresolved":false,"context_lines":[{"line_number":979,"context_line":"            req \u003d fakes.HTTPRequest.blank(\u0027/v2/volumes?sort\u003dname\u0027)"},{"line_number":980,"context_line":"            res_dict \u003d self.controller.index(req)"},{"line_number":981,"context_line":"            volumes \u003d res_dict[\u0027volumes\u0027]"},{"line_number":982,"context_line":"            self.assertEqual(len(volumes), 2)"},{"line_number":983,"context_line":""},{"line_number":984,"context_line":"    def test_volume_index_limit_offset(self):"},{"line_number":985,"context_line":"        def stub_volume_get_all_by_project(context, project_id, marker, limit,"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba3cc151_ef1e0aba","line":982,"updated":"2015-07-09 04:38:20.000000000","message":"2 is the expected value, len(volumes) the actual value.","commit_id":"e06c66b47cf9e97a807174283be710ff320eb7e1"},{"author":{"_account_id":10485,"name":"Ankit Agrawal","email":"ankitagrawal8188@gmail.com","username":"ankitagrawal"},"change_message_id":"bac86275995b783c853443b22fdaeb7b855c3d69","unresolved":false,"context_lines":[{"line_number":980,"context_line":"            res_dict \u003d self.controller.index(req)"},{"line_number":981,"context_line":"            volumes \u003d res_dict[\u0027volumes\u0027]"},{"line_number":982,"context_line":"            self.assertEqual(2, len(volumes))"},{"line_number":983,"context_line":""},{"line_number":984,"context_line":"    def test_volume_index_limit_offset(self):"},{"line_number":985,"context_line":"        def stub_volume_get_all_by_project(context, project_id, marker, limit,"},{"line_number":986,"context_line":"                                           sort_keys\u003dNone, sort_dirs\u003dNone,"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba3cc151_d9a0acf6","line":983,"updated":"2015-07-09 07:46:34.000000000","message":"I am agree with Sean\u0027s comment on patch set #7 that there is no need to add this additional unit test case, because the test you have added below itself will fail if cinder-api doesn\u0027t change name to display_name in sort_keys.","commit_id":"30d1a580d24454805b9886d3ff3f173b0d74b914"},{"author":{"_account_id":13747,"name":"Kuo-tung Kao","email":"jelly.k@inwinstack.com","username":"coding1314"},"change_message_id":"ee674fc72a1f170ce71d48cdc28a4f95a84ab0be","unresolved":false,"context_lines":[{"line_number":980,"context_line":"            res_dict \u003d self.controller.index(req)"},{"line_number":981,"context_line":"            volumes \u003d res_dict[\u0027volumes\u0027]"},{"line_number":982,"context_line":"            self.assertEqual(2, len(volumes))"},{"line_number":983,"context_line":""},{"line_number":984,"context_line":"    def test_volume_index_limit_offset(self):"},{"line_number":985,"context_line":"        def stub_volume_get_all_by_project(context, project_id, marker, limit,"},{"line_number":986,"context_line":"                                           sort_keys\u003dNone, sort_dirs\u003dNone,"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba3cc151_39cd1866","line":983,"in_reply_to":"ba3cc151_d9a0acf6","updated":"2015-07-09 07:52:11.000000000","message":"Thank you for your suggestion. I will delete the test case tomorrow if most people think deleting the test case is better.","commit_id":"30d1a580d24454805b9886d3ff3f173b0d74b914"},{"author":{"_account_id":9171,"name":"Vipin Balachandran","email":"vipin.bl@gmail.com","username":"vbala"},"change_message_id":"880b71f19ff4e5abfb0adaab61fcaab21a498a58","unresolved":false,"context_lines":[{"line_number":1621,"context_line":""},{"line_number":1622,"context_line":"    @mock.patch(\u0027cinder.volume.api.API.get_all\u0027)"},{"line_number":1623,"context_line":"    def test_get_volumes_sort_by_name(self, get_all):"},{"line_number":1624,"context_line":"        \"\"\"name` in client means `display_name` in database. \"\"\""},{"line_number":1625,"context_line":""},{"line_number":1626,"context_line":"        req \u003d mock.MagicMock()"},{"line_number":1627,"context_line":"        ctxt \u003d context.RequestContext(\u0027fake\u0027, \u0027fake\u0027, auth_token\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a41bdd9_305388e3","line":1624,"updated":"2015-07-13 07:49:28.000000000","message":"name` -\u003e name","commit_id":"234b103260ae4f79c3055d8fc171f63bdaf7b5f5"}],"cinder/tests/unit/db/sqlalchemy/__init__.py":[{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"a136311f682769d4e5afdba28f78eb307f039c44","unresolved":false,"context_lines":[{"line_number":1,"context_line":"__author__ \u003d \u0027kjelly\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba3cc151_a25df716","line":1,"updated":"2015-07-07 02:10:54.000000000","message":"I think these are supposed to be empty.  I have not seen this dine elsewhere.","commit_id":"67d7aca529ae4050f0fa723816d44d8c6493d869"}]}