)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"dc7abce932c15a393bcc3c8167f01ac68483d67a","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2015-02-19 19:17:10 +0530"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Updated the code to display security group\u0027s uuid instead of name in \u0027nova show\u0027"},{"line_number":8,"context_line":"Closes-Bug: 1394462"},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"Change-Id: I7bc73cbc6801ce44bd6b8a1e0cbff9bac5c62e04"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"da86d52c_e7080393","line":8,"updated":"2015-02-19 16:39:40.000000000","message":"The subject line is fairly good, though it could stand to be shortened (ideally, 50 columns max, but that\u0027s not a hard limit).  However, it needs to be offset from the subsequent text with a blank line.  Further, I\u0027d like to see a more detailed description.\n\nPlease see [1] for more information about how we like commit messages to be formatted.\n\n[1] https://wiki.openstack.org/wiki/GitCommitMessages","commit_id":"e6d5e7c2c420c6eb36f897314b7a29b5b67491ea"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"c5627330d4bfa955f18ffc8ad2a38a3c590b8153","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Display security group id in \u0027nova show\u0027 output"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch modifies the \u0027_print_server\u0027 method in \u0027v2/shell.py\u0027"},{"line_number":10,"context_line":"file, to display security group id along with it\u0027s name in the"},{"line_number":11,"context_line":"output of \u0027nova show\u0027 command."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closes-Bug: 1394462"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9a80dd14_4f31caa0","line":10,"updated":"2015-03-16 15:57:27.000000000","message":"femtonit: \"its\" is the possessive you want here; \"it\u0027s\" is a contraction for \"it is\".","commit_id":"d5d9ab43b031124b185fb4e3174e62d33fda56d7"},{"author":{"_account_id":14173,"name":"Sai Kiran","email":"sv7665@att.com","username":"SaiKiran"},"change_message_id":"ee6f40db5153d9a89336dcdbc3e490a0acc6ba30","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Display security group id in \u0027nova show\u0027 output"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch modifies the \u0027_print_server\u0027 method in \u0027v2/shell.py\u0027"},{"line_number":10,"context_line":"file, to display security group id along with it\u0027s name in the"},{"line_number":11,"context_line":"output of \u0027nova show\u0027 command."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Closes-Bug: 1394462"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9a80dd14_52304a07","line":10,"in_reply_to":"9a80dd14_4f31caa0","updated":"2015-03-17 18:57:54.000000000","message":"Done","commit_id":"d5d9ab43b031124b185fb4e3174e62d33fda56d7"}],"novaclient/tests/unit/v2/test_shell.py":[{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"c5627330d4bfa955f18ffc8ad2a38a3c590b8153","unresolved":false,"context_lines":[{"line_number":666,"context_line":"                                                    mock_list_security_group):"},{"line_number":667,"context_line":"        secgroup_list \u003d [FakeSecgroup(\u0027securitygroup1\u0027, 1),"},{"line_number":668,"context_line":"                         FakeSecgroup(\u0027securitygroup2\u0027, 2)]"},{"line_number":669,"context_line":"        mock_list_security_group.return_value \u003d secgroup_list"},{"line_number":670,"context_line":""},{"line_number":671,"context_line":"        self.assertRaises(exceptions.InstanceInErrorState, self.run_command,"},{"line_number":672,"context_line":"                          \u0027boot --flavor 1 --image 1 some-bad-server --poll\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_0fa35243","line":669,"updated":"2015-03-16 15:57:27.000000000","message":"Overall, this looks good, but allow me to show you a couple of alternatives.  This first one packs all the return_value logic into the decorator:\n\n    @mock.patch(\u0027novaclient.v2.servers.Server.list_security_group\u0027,\n                return_value\u003d[\n                    FakeSecgroup(\u0027securitygroup1\u0027, 1),\n                    FakeSecgroup(\u0027securitygroup2\u0027, 2),\n                ])\n\nThe second one eliminates the FakeSecgroup class by using a mock.Mock().  Note the handling on the \u0027name\u0027 attribute; that\u0027s required because the mock.Mock constructor treats the \u0027name\u0027 keyword argument specially.\n\n    @mock.patch(\u0027novaclient.v2.servers.Server.list_security_group\u0027)\n    def test_boot_with_poll_to_check_VM_state_error(self,\n                                                    mock_list_security_group):\n        secgroup_list \u003d [\n            mock.Mock(xname\u003d\u0027securitygroup1\u0027, id\u003d1),\n            mock.Mock(xname\u003d\u0027securitygroup2\u0027, id\u003d2),\n        ]\n        for secgroup in secgroup_list:\n            secgroup.name \u003d secgroup.xname\n        mock_list_security_group.return_value \u003d secgroup_list\n\nOf course, these two approaches could be combined as follows:\n\n    @mock.patch(\u0027novaclient.v2.servers.Server.list_security_group\u0027,\n                return_value\u003d[\n                    mock.Mock(xname\u003d\u0027securitygroup1\u0027, id\u003d1),\n                    mock.Mock(xname\u003d\u0027securitygroup2\u0027, id\u003d2),\n                ])\n    def test_boot_with_poll_to_check_VM_state_error(self,\n                                                    mock_list_security_group):\n        for secgroup in mock_list_security_group.return_value:\n            secgroup.name \u003d secgroup.xname\n\n(This comment is just for education; you may feel free to leave everything the way it is.)","commit_id":"d5d9ab43b031124b185fb4e3174e62d33fda56d7"},{"author":{"_account_id":14173,"name":"Sai Kiran","email":"sv7665@att.com","username":"SaiKiran"},"change_message_id":"ee6f40db5153d9a89336dcdbc3e490a0acc6ba30","unresolved":false,"context_lines":[{"line_number":666,"context_line":"                                                    mock_list_security_group):"},{"line_number":667,"context_line":"        secgroup_list \u003d [FakeSecgroup(\u0027securitygroup1\u0027, 1),"},{"line_number":668,"context_line":"                         FakeSecgroup(\u0027securitygroup2\u0027, 2)]"},{"line_number":669,"context_line":"        mock_list_security_group.return_value \u003d secgroup_list"},{"line_number":670,"context_line":""},{"line_number":671,"context_line":"        self.assertRaises(exceptions.InstanceInErrorState, self.run_command,"},{"line_number":672,"context_line":"                          \u0027boot --flavor 1 --image 1 some-bad-server --poll\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_c9465f4f","line":669,"in_reply_to":"9a80dd14_0fa35243","updated":"2015-03-17 18:57:54.000000000","message":"I really appreciate your explanation.\n\nBut I tried to use the same, facing issues while accessing \"name\" attribute in that mock. I am getting \".name\" of mock as \"\u003cMock name\u003d\u0027securitygroup1.name\u0027 id\u003d\u0027140696040375568\u0027\u003e\" instead of \"securitygroup1\".\n\nIs there any other way to access name attribute in mock.","commit_id":"d5d9ab43b031124b185fb4e3174e62d33fda56d7"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"f710f540d298ee310f0e4f2cb262cfe1f1128aef","unresolved":false,"context_lines":[{"line_number":666,"context_line":"                                                    mock_list_security_group):"},{"line_number":667,"context_line":"        secgroup_list \u003d [FakeSecgroup(\u0027securitygroup1\u0027, 1),"},{"line_number":668,"context_line":"                         FakeSecgroup(\u0027securitygroup2\u0027, 2)]"},{"line_number":669,"context_line":"        mock_list_security_group.return_value \u003d secgroup_list"},{"line_number":670,"context_line":""},{"line_number":671,"context_line":"        self.assertRaises(exceptions.InstanceInErrorState, self.run_command,"},{"line_number":672,"context_line":"                          \u0027boot --flavor 1 --image 1 some-bad-server --poll\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_381c906a","line":669,"in_reply_to":"9a80dd14_c9465f4f","updated":"2015-03-17 21:05:01.000000000","message":"As I said, the \"name\" keyword argument to the mock.Mock() constructor is treated specially.  This is why my mock-based examples use xname, then use a loop to explicitly set the attribute \"name\" to the value of the \"xname\" attribute.  Perhaps it\u0027ll be clearer with an example:\n\n    \u003e\u003e\u003e a \u003d mock.Mock(name\u003d\u0027foo\u0027, id\u003d2)\n    \u003e\u003e\u003e a.name\n    \u003cMock name\u003d\u0027foo.name\u0027 id\u003d\u0027139805218559056\u0027\u003e\n    \u003e\u003e\u003e a.name \u003d \u0027foo\u0027\n    \u003e\u003e\u003e a.name\n    \u0027foo\u0027","commit_id":"d5d9ab43b031124b185fb4e3174e62d33fda56d7"}],"novaclient/v2/shell.py":[{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"dc7abce932c15a393bcc3c8167f01ac68483d67a","unresolved":false,"context_lines":[{"line_number":1837,"context_line":"        # security groups N times where N \u003d\u003d number of nics. Be nice"},{"line_number":1838,"context_line":"        # and only display it once."},{"line_number":1839,"context_line":"        secgroup_list \u003d server.list_security_group()"},{"line_number":1840,"context_line":"        info[\u0027security_groups\u0027] \u003d \\"},{"line_number":1841,"context_line":"            \u0027, \u0027.join(sorted(set(security_group.__dict__[\u0027id\u0027] for security_group in secgroup_list)))"},{"line_number":1842,"context_line":""},{"line_number":1843,"context_line":"    image \u003d info.get(\u0027image\u0027, {})"}],"source_content_type":"text/x-python","patch_set":1,"id":"da86d52c_e71643a6","line":1840,"updated":"2015-02-19 16:39:40.000000000","message":"Please avoid backslash continuation where possible.  You can use parentheses for this.","commit_id":"e6d5e7c2c420c6eb36f897314b7a29b5b67491ea"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"dc7abce932c15a393bcc3c8167f01ac68483d67a","unresolved":false,"context_lines":[{"line_number":1838,"context_line":"        # and only display it once."},{"line_number":1839,"context_line":"        secgroup_list \u003d server.list_security_group()"},{"line_number":1840,"context_line":"        info[\u0027security_groups\u0027] \u003d \\"},{"line_number":1841,"context_line":"            \u0027, \u0027.join(sorted(set(security_group.__dict__[\u0027id\u0027] for security_group in secgroup_list)))"},{"line_number":1842,"context_line":""},{"line_number":1843,"context_line":"    image \u003d info.get(\u0027image\u0027, {})"},{"line_number":1844,"context_line":"    if image:"}],"source_content_type":"text/x-python","patch_set":1,"id":"da86d52c_47262fcf","line":1841,"updated":"2015-02-19 16:39:40.000000000","message":"This line is clearly too long; code lines are limited at 79 columns.  Further, I see no reason to use __dict__ here; you should use attribute access instead.  Finally, I don\u0027t understand why you\u0027re making another round-trip with the server by calling list_security_group(); that data is clearly already in the \"info\" dictionary, or this code wouldn\u0027t even be reached.","commit_id":"e6d5e7c2c420c6eb36f897314b7a29b5b67491ea"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"c5627330d4bfa955f18ffc8ad2a38a3c590b8153","unresolved":false,"context_lines":[{"line_number":1836,"context_line":"        # when we have multiple nics the info will include the"},{"line_number":1837,"context_line":"        # security groups N times where N \u003d\u003d number of nics. Be nice"},{"line_number":1838,"context_line":"        # and only display it once."},{"line_number":1839,"context_line":"        secgroup_list \u003d server.list_security_group()"},{"line_number":1840,"context_line":"        secgroup_info \u003d []"},{"line_number":1841,"context_line":""},{"line_number":1842,"context_line":"        for secgroup in secgroup_list:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_4a045814","line":1839,"updated":"2015-03-16 15:57:27.000000000","message":"Why is the second round trip with the server necessary?  Doesn\u0027t this information already exist in info[\u0027security_groups\u0027]?","commit_id":"d5d9ab43b031124b185fb4e3174e62d33fda56d7"},{"author":{"_account_id":14173,"name":"Sai Kiran","email":"sv7665@att.com","username":"SaiKiran"},"change_message_id":"ee6f40db5153d9a89336dcdbc3e490a0acc6ba30","unresolved":false,"context_lines":[{"line_number":1836,"context_line":"        # when we have multiple nics the info will include the"},{"line_number":1837,"context_line":"        # security groups N times where N \u003d\u003d number of nics. Be nice"},{"line_number":1838,"context_line":"        # and only display it once."},{"line_number":1839,"context_line":"        secgroup_list \u003d server.list_security_group()"},{"line_number":1840,"context_line":"        secgroup_info \u003d []"},{"line_number":1841,"context_line":""},{"line_number":1842,"context_line":"        for secgroup in secgroup_list:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_9e72dbe1","line":1839,"in_reply_to":"9a80dd14_4a045814","updated":"2015-03-17 18:57:54.000000000","message":"info[\u0027security_groups\u0027] contains only the names of \"security groups\", but not the ids. Below is the example:\n\ninfo[\u0027security_groups\u0027] \u003d [{u\u0027name\u0027: u\u0027default\u0027}]\n\nBut we need ID of the secgroup also, so we need second round trip.","commit_id":"d5d9ab43b031124b185fb4e3174e62d33fda56d7"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"f710f540d298ee310f0e4f2cb262cfe1f1128aef","unresolved":false,"context_lines":[{"line_number":1836,"context_line":"        # when we have multiple nics the info will include the"},{"line_number":1837,"context_line":"        # security groups N times where N \u003d\u003d number of nics. Be nice"},{"line_number":1838,"context_line":"        # and only display it once."},{"line_number":1839,"context_line":"        secgroup_list \u003d server.list_security_group()"},{"line_number":1840,"context_line":"        secgroup_info \u003d []"},{"line_number":1841,"context_line":""},{"line_number":1842,"context_line":"        for secgroup in secgroup_list:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_d82294a2","line":1839,"in_reply_to":"9a80dd14_9e72dbe1","updated":"2015-03-17 21:05:01.000000000","message":"OK.  Could you mention that in the comment, just to make it clear?","commit_id":"d5d9ab43b031124b185fb4e3174e62d33fda56d7"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"c5627330d4bfa955f18ffc8ad2a38a3c590b8153","unresolved":false,"context_lines":[{"line_number":1842,"context_line":"        for secgroup in secgroup_list:"},{"line_number":1843,"context_line":"            secgroup_info.append("},{"line_number":1844,"context_line":"                \u0027%s (%s)\u0027 % (secgroup.__dict__[\u0027name\u0027],"},{"line_number":1845,"context_line":"                             secgroup.__dict__[\u0027id\u0027]))"},{"line_number":1846,"context_line":"        info[\u0027security_groups\u0027] \u003d \u0027, \u0027.join(sorted(set(secgroup_info)))"},{"line_number":1847,"context_line":""},{"line_number":1848,"context_line":"    image \u003d info.get(\u0027image\u0027, {})"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_0f6512ae","line":1845,"updated":"2015-03-16 15:57:27.000000000","message":"I still see no reason to use __dict__ here; you should be using attribute access.","commit_id":"d5d9ab43b031124b185fb4e3174e62d33fda56d7"},{"author":{"_account_id":14173,"name":"Sai Kiran","email":"sv7665@att.com","username":"SaiKiran"},"change_message_id":"ee6f40db5153d9a89336dcdbc3e490a0acc6ba30","unresolved":false,"context_lines":[{"line_number":1842,"context_line":"        for secgroup in secgroup_list:"},{"line_number":1843,"context_line":"            secgroup_info.append("},{"line_number":1844,"context_line":"                \u0027%s (%s)\u0027 % (secgroup.__dict__[\u0027name\u0027],"},{"line_number":1845,"context_line":"                             secgroup.__dict__[\u0027id\u0027]))"},{"line_number":1846,"context_line":"        info[\u0027security_groups\u0027] \u003d \u0027, \u0027.join(sorted(set(secgroup_info)))"},{"line_number":1847,"context_line":""},{"line_number":1848,"context_line":"    image \u003d info.get(\u0027image\u0027, {})"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_dee0d398","line":1845,"in_reply_to":"9a80dd14_0f6512ae","updated":"2015-03-17 18:57:54.000000000","message":"Done","commit_id":"d5d9ab43b031124b185fb4e3174e62d33fda56d7"}]}