)]}'
{"nova/tests/virt/xenapi/test_vm_utils.py":[{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"98b548b77a10d1ea7d9622c230e247bad00cf461","unresolved":false,"context_lines":[{"line_number":1454,"context_line":""},{"line_number":1455,"context_line":"    def test_vdi_get_virtual_size(self):"},{"line_number":1456,"context_line":"        session \u003d mock.Mock()"},{"line_number":1457,"context_line":"        session.call_xenapi.return_value \u003d \"123\""},{"line_number":1458,"context_line":"        self.assertEqual(123, vm_utils._vdi_get_virtual_size(session, \"ref\"))"},{"line_number":1459,"context_line":"        session.call_xenapi.assert_called_once_with(\"VDI.get_virtual_size\","},{"line_number":1460,"context_line":"                                                    \"ref\")"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATn%2F%2Fu9Y%3D","line":1457,"updated":"2013-10-29 18:39:59.000000000","message":"As a point of information, you could use:\n\n    session \u003d mock.Mock(**{\u0027call_xenapi.return_value\u0027: \u0027123\u0027})","commit_id":"b17ec02660aacf44ff3a3db4a46228018fa17e86"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"55cfebbe8ecfd365e28f204248b7505e387c98bb","unresolved":false,"context_lines":[{"line_number":1454,"context_line":""},{"line_number":1455,"context_line":"    def test_vdi_get_virtual_size(self):"},{"line_number":1456,"context_line":"        session \u003d mock.Mock()"},{"line_number":1457,"context_line":"        session.call_xenapi.return_value \u003d \"123\""},{"line_number":1458,"context_line":"        self.assertEqual(123, vm_utils._vdi_get_virtual_size(session, \"ref\"))"},{"line_number":1459,"context_line":"        session.call_xenapi.assert_called_once_with(\"VDI.get_virtual_size\","},{"line_number":1460,"context_line":"                                                    \"ref\")"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATn%2F%2FLbc%3D","line":1457,"in_reply_to":"AAAATn%2F%2Fu9Y%3D","updated":"2013-11-13 09:59:52.000000000","message":"I kinda like the long hand because it helps when refactoring code, and seems to read quite nicely, but its good to know.","commit_id":"b17ec02660aacf44ff3a3db4a46228018fa17e86"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"fefb57557c9d29c4d9059e8bf334baabe177b18e","unresolved":false,"context_lines":[{"line_number":1730,"context_line":"        self._assert_transfer_called(session, \"a\")"},{"line_number":1731,"context_line":""},{"line_number":1732,"context_line":""},{"line_number":1733,"context_line":"class ResizeFunctionTestCase(test.NoDBTestCase):"},{"line_number":1734,"context_line":"    def _call_get_resize_func_name(self, brand, version):"},{"line_number":1735,"context_line":"        session \u003d mock.Mock()"},{"line_number":1736,"context_line":"        session.product_brand \u003d brand"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATn%2F%2FLzo%3D","line":1733,"updated":"2013-11-13 09:01:00.000000000","message":"This class would be much better off as a testscenarios test.","commit_id":"b17ec02660aacf44ff3a3db4a46228018fa17e86"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"55cfebbe8ecfd365e28f204248b7505e387c98bb","unresolved":false,"context_lines":[{"line_number":1730,"context_line":"        self._assert_transfer_called(session, \"a\")"},{"line_number":1731,"context_line":""},{"line_number":1732,"context_line":""},{"line_number":1733,"context_line":"class ResizeFunctionTestCase(test.NoDBTestCase):"},{"line_number":1734,"context_line":"    def _call_get_resize_func_name(self, brand, version):"},{"line_number":1735,"context_line":"        session \u003d mock.Mock()"},{"line_number":1736,"context_line":"        session.product_brand \u003d brand"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATn%2F%2FLbk%3D","line":1733,"in_reply_to":"AAAATn%2F%2FLzo%3D","updated":"2013-11-13 09:59:52.000000000","message":"not seen heavy use of that, so was just keeping it simple.","commit_id":"b17ec02660aacf44ff3a3db4a46228018fa17e86"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"98b548b77a10d1ea7d9622c230e247bad00cf461","unresolved":false,"context_lines":[{"line_number":1734,"context_line":"    def _call_get_resize_func_name(self, brand, version):"},{"line_number":1735,"context_line":"        session \u003d mock.Mock()"},{"line_number":1736,"context_line":"        session.product_brand \u003d brand"},{"line_number":1737,"context_line":"        session.product_version \u003d version"},{"line_number":1738,"context_line":""},{"line_number":1739,"context_line":"        return vm_utils._get_resize_func_name(session)"},{"line_number":1740,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATn%2F%2Fu8U%3D","line":1737,"updated":"2013-10-29 18:39:59.000000000","message":"This one\u0027s even easier:\n\n    session \u003d mock.Mock(product_brand\u003dbrand, product_version\u003dversion)\n\nThe only one I\u0027ve run into trouble with is the \"name\" attribute, because mock.Mock tries to do something special with it.","commit_id":"b17ec02660aacf44ff3a3db4a46228018fa17e86"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"4e27cc1d86649776a02ed3c3ab0b0d5c8924fff1","unresolved":false,"context_lines":[{"line_number":1530,"context_line":"                                                    \"vdi_ref\", {})"},{"line_number":1531,"context_line":""},{"line_number":1532,"context_line":"    def test_vdi_get_virtual_size(self):"},{"line_number":1533,"context_line":"        session \u003d mock.Mock()"},{"line_number":1534,"context_line":"        session.call_xenapi.return_value \u003d \"123\""},{"line_number":1535,"context_line":"        self.assertEqual(123, vm_utils._vdi_get_virtual_size(session, \"ref\"))"},{"line_number":1536,"context_line":"        session.call_xenapi.assert_called_once_with(\"VDI.get_virtual_size\","}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAATn%2F%2BlYY%3D","line":1533,"updated":"2013-11-22 19:55:44.000000000","message":"Tip: You could also do:\n\n    session \u003d mock.Mock(**{\u0027call_xenapi.return_value\u0027: \u0027123\u0027})\n\nThat will eliminate line 1534.","commit_id":"a22f8208bf534f0fbc5dbffb06a92e7f1c445731"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"4e27cc1d86649776a02ed3c3ab0b0d5c8924fff1","unresolved":false,"context_lines":[{"line_number":1536,"context_line":"        session.call_xenapi.assert_called_once_with(\"VDI.get_virtual_size\","},{"line_number":1537,"context_line":"                                                    \"ref\")"},{"line_number":1538,"context_line":""},{"line_number":1539,"context_line":"    @mock.patch.object(vm_utils, \u0027_get_resize_func_name\u0027)"},{"line_number":1540,"context_line":"    def test_vdi_resize(self, mock_get_resize_func_name):"},{"line_number":1541,"context_line":"        session \u003d mock.Mock()"},{"line_number":1542,"context_line":"        mock_get_resize_func_name.return_value \u003d \"VDI.fake\""}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAATn%2F%2BlX8%3D","line":1539,"updated":"2013-11-22 19:55:44.000000000","message":"Tip: You could also do:\n\n    @mock.patch.object(vm_utils, \u0027_get_resize_func_name\u0027,\n                       return_value\u003d\u0027VDI.fake\u0027)","commit_id":"a22f8208bf534f0fbc5dbffb06a92e7f1c445731"},{"author":{"_account_id":679,"name":"Kevin L. Mitchell","email":"klmitch@mit.edu","username":"klmitch"},"change_message_id":"4e27cc1d86649776a02ed3c3ab0b0d5c8924fff1","unresolved":false,"context_lines":[{"line_number":1937,"context_line":""},{"line_number":1938,"context_line":"class ResizeFunctionTestCase(test.NoDBTestCase):"},{"line_number":1939,"context_line":"    def _call_get_resize_func_name(self, brand, version):"},{"line_number":1940,"context_line":"        session \u003d mock.Mock()"},{"line_number":1941,"context_line":"        session.product_brand \u003d brand"},{"line_number":1942,"context_line":"        session.product_version \u003d version"},{"line_number":1943,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAATn%2F%2BlXs%3D","line":1940,"updated":"2013-11-22 19:55:44.000000000","message":"Tip: You could also do:\n\n    session \u003d mock.Mock(product_brand\u003dbrand, product_version\u003dversion)","commit_id":"a22f8208bf534f0fbc5dbffb06a92e7f1c445731"}],"nova/virt/xenapi/vm_utils.py":[{"author":{"_account_id":7040,"name":"GuoHui LIu","email":"liuguohui@gmail.com","username":"guohliu"},"change_message_id":"b1063f0d59d8cb6a6e9eeb3172233930b10e890e","unresolved":false,"context_lines":[{"line_number":864,"context_line":"        _vdi_resize(session, vdi_ref, new_disk_size)"},{"line_number":865,"context_line":""},{"line_number":866,"context_line":"    else:"},{"line_number":867,"context_line":"        LOG.debug(_(\"Skipping unsafe resize down of VDI %s\"), vdi_ref,"},{"line_number":868,"context_line":"                  instance\u003dinstance)"},{"line_number":869,"context_line":""},{"line_number":870,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATH%2F%2FjI4%3D","line":867,"updated":"2013-10-14 17:00:15.000000000","message":"If we feel not safe to resize down, how about to throw a exception? would it be a problem that the disk size will be inconsistency with flavor? am I miss something here?","commit_id":"e7af6915bf64a57146e8989553f12c330461e3b9"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6e034462fc51d14136997fae50ffd97944ec9ef2","unresolved":false,"context_lines":[{"line_number":864,"context_line":"        _vdi_resize(session, vdi_ref, new_disk_size)"},{"line_number":865,"context_line":""},{"line_number":866,"context_line":"    else:"},{"line_number":867,"context_line":"        LOG.debug(_(\"Skipping unsafe resize down of VDI %s\"), vdi_ref,"},{"line_number":868,"context_line":"                  instance\u003dinstance)"},{"line_number":869,"context_line":""},{"line_number":870,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATH%2F%2FjFw%3D","line":867,"in_reply_to":"AAAATH%2F%2FjGY%3D","updated":"2013-10-14 17:12:04.000000000","message":"The old code didn\u0027t raise, but I think we should, updating it...","commit_id":"e7af6915bf64a57146e8989553f12c330461e3b9"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"e81d6eaad0163eed65416ca525a4457a225bdca5","unresolved":false,"context_lines":[{"line_number":864,"context_line":"        _vdi_resize(session, vdi_ref, new_disk_size)"},{"line_number":865,"context_line":""},{"line_number":866,"context_line":"    else:"},{"line_number":867,"context_line":"        LOG.debug(_(\"Skipping unsafe resize down of VDI %s\"), vdi_ref,"},{"line_number":868,"context_line":"                  instance\u003dinstance)"},{"line_number":869,"context_line":""},{"line_number":870,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATH%2F%2FjGY%3D","line":867,"in_reply_to":"AAAATH%2F%2FjI4%3D","updated":"2013-10-14 17:10:14.000000000","message":"No your not missing anything, I couldn\u0027t quite decide about that.\n\nIts a code path you shouldn\u0027t be able to reach, because we stop resize down a bit further up the stack, but I agree an exception is probably safer here.","commit_id":"e7af6915bf64a57146e8989553f12c330461e3b9"},{"author":{"_account_id":7040,"name":"GuoHui LIu","email":"liuguohui@gmail.com","username":"guohliu"},"change_message_id":"6c1633f69f01b47e3a8b17b46be281a59fc9943f","unresolved":false,"context_lines":[{"line_number":864,"context_line":"        _vdi_resize(session, vdi_ref, new_disk_size)"},{"line_number":865,"context_line":""},{"line_number":866,"context_line":"    else:"},{"line_number":867,"context_line":"        LOG.error(_(\"Found unexpected size of VDI %s during resize up\"),"},{"line_number":868,"context_line":"                  vdi_ref, instance\u003dinstance)"},{"line_number":869,"context_line":"        raise exception.ResizeError(_(\"Unexpected VDI size for resize up.\"))"},{"line_number":870,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATH%2F%2Fi9o%3D","line":867,"updated":"2013-10-14 17:46:48.000000000","message":"John, feel this might be a little bit mis leading to user, how about \"Cannot resize VDI to smaller size, old size is, new size is\"?","commit_id":"4205b1f5971b9bf603fc6860d08e3c8297ee56a9"},{"author":{"_account_id":7040,"name":"GuoHui LIu","email":"liuguohui@gmail.com","username":"guohliu"},"change_message_id":"9646f04034fdee28f60287663d98316e53dddb09","unresolved":false,"context_lines":[{"line_number":864,"context_line":"        _vdi_resize(session, vdi_ref, new_disk_size)"},{"line_number":865,"context_line":""},{"line_number":866,"context_line":"    else:"},{"line_number":867,"context_line":"        LOG.error(_(\"Found unexpected size of VDI %s during resize up\"),"},{"line_number":868,"context_line":"                  vdi_ref, instance\u003dinstance)"},{"line_number":869,"context_line":"        raise exception.ResizeError(_(\"Unexpected VDI size for resize up.\"))"},{"line_number":870,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATH%2F%2FgMk%3D","line":867,"in_reply_to":"AAAATH%2F%2Fi5o%3D","updated":"2013-10-15 10:32:50.000000000","message":"Thanks for your explanation.","commit_id":"4205b1f5971b9bf603fc6860d08e3c8297ee56a9"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"5e4b687fd062c0dfd85c78438038135cf9906719","unresolved":false,"context_lines":[{"line_number":864,"context_line":"        _vdi_resize(session, vdi_ref, new_disk_size)"},{"line_number":865,"context_line":""},{"line_number":866,"context_line":"    else:"},{"line_number":867,"context_line":"        LOG.error(_(\"Found unexpected size of VDI %s during resize up\"),"},{"line_number":868,"context_line":"                  vdi_ref, instance\u003dinstance)"},{"line_number":869,"context_line":"        raise exception.ResizeError(_(\"Unexpected VDI size for resize up.\"))"},{"line_number":870,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATH%2F%2Fi5o%3D","line":867,"in_reply_to":"AAAATH%2F%2Fi9o%3D","updated":"2013-10-14 18:09:37.000000000","message":"No, thats is covered else where in the code.\n\nWe tell the user what is happening here (line 976):\nhttps://review.openstack.org/#/c/49349/15/nova/virt/xenapi/vmops.py","commit_id":"4205b1f5971b9bf603fc6860d08e3c8297ee56a9"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"fefb57557c9d29c4d9059e8bf334baabe177b18e","unresolved":false,"context_lines":[{"line_number":823,"context_line":"        return recs.keys()[0]"},{"line_number":824,"context_line":""},{"line_number":825,"context_line":""},{"line_number":826,"context_line":"def _get_resize_func_name(session):"},{"line_number":827,"context_line":"    brand \u003d session.product_brand"},{"line_number":828,"context_line":"    version \u003d session.product_version"},{"line_number":829,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATn%2F%2FLtY%3D","line":826,"updated":"2013-11-13 09:01:00.000000000","message":"This needs a docstring.","commit_id":"b17ec02660aacf44ff3a3db4a46228018fa17e86"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"78fc739357dfa57735cba24518c64f7fc0536b79","unresolved":false,"context_lines":[{"line_number":823,"context_line":"        return recs.keys()[0]"},{"line_number":824,"context_line":""},{"line_number":825,"context_line":""},{"line_number":826,"context_line":"def _get_resize_func_name(session):"},{"line_number":827,"context_line":"    brand \u003d session.product_brand"},{"line_number":828,"context_line":"    version \u003d session.product_version"},{"line_number":829,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATn%2F9q7A%3D","line":826,"in_reply_to":"AAAATn%2F%2FLcQ%3D","updated":"2013-12-08 22:35:02.000000000","message":"Mmmmm, I don\u0027t think privacy counts towards need-to-capture-intent-and-usage. Maybe a slightly lower standard perhaps, but we\u0027re the primary consumers of such docs anyhow.","commit_id":"b17ec02660aacf44ff3a3db4a46228018fa17e86"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"55cfebbe8ecfd365e28f204248b7505e387c98bb","unresolved":false,"context_lines":[{"line_number":823,"context_line":"        return recs.keys()[0]"},{"line_number":824,"context_line":""},{"line_number":825,"context_line":""},{"line_number":826,"context_line":"def _get_resize_func_name(session):"},{"line_number":827,"context_line":"    brand \u003d session.product_brand"},{"line_number":828,"context_line":"    version \u003d session.product_version"},{"line_number":829,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATn%2F%2FLcQ%3D","line":826,"in_reply_to":"AAAATn%2F%2FLtY%3D","updated":"2013-11-13 09:59:52.000000000","message":"I am not sure it would add anything to this private function.","commit_id":"b17ec02660aacf44ff3a3db4a46228018fa17e86"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"b9495e4198213de86066ba9c34d2cb3d1426765c","unresolved":false,"context_lines":[{"line_number":823,"context_line":"        return recs.keys()[0]"},{"line_number":824,"context_line":""},{"line_number":825,"context_line":""},{"line_number":826,"context_line":"def _get_resize_func_name(session):"},{"line_number":827,"context_line":"    brand \u003d session.product_brand"},{"line_number":828,"context_line":"    version \u003d session.product_version"},{"line_number":829,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATn%2F9IuA%3D","line":826,"in_reply_to":"AAAATn%2F9q7A%3D","updated":"2013-12-16 13:21:35.000000000","message":"Agreed, I just can\u0027t think of anything worth writing that adds to the readability of this function, and prefer self-documenting code to redundant comments that need extra maintenance. But my dyslexia makes me avoid the writing, so I probably lean a little on the lacking documentation these days.","commit_id":"b17ec02660aacf44ff3a3db4a46228018fa17e86"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"fefb57557c9d29c4d9059e8bf334baabe177b18e","unresolved":false,"context_lines":[{"line_number":834,"context_line":"        r1_2_or_above \u003d ("},{"line_number":835,"context_line":"            ("},{"line_number":836,"context_line":"                version[0] \u003d\u003d 1"},{"line_number":837,"context_line":"                and version[1] \u003e 1"},{"line_number":838,"context_line":"            )"},{"line_number":839,"context_line":"            or version[0] \u003e 1)"},{"line_number":840,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATn%2F%2FLzE%3D","line":837,"updated":"2013-11-13 09:01:00.000000000","message":"These will fit on one line nicely - no need to be so complex with the layout.","commit_id":"b17ec02660aacf44ff3a3db4a46228018fa17e86"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"55cfebbe8ecfd365e28f204248b7505e387c98bb","unresolved":false,"context_lines":[{"line_number":834,"context_line":"        r1_2_or_above \u003d ("},{"line_number":835,"context_line":"            ("},{"line_number":836,"context_line":"                version[0] \u003d\u003d 1"},{"line_number":837,"context_line":"                and version[1] \u003e 1"},{"line_number":838,"context_line":"            )"},{"line_number":839,"context_line":"            or version[0] \u003e 1)"},{"line_number":840,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATn%2F%2FLcE%3D","line":837,"in_reply_to":"AAAATn%2F%2FLzE%3D","updated":"2013-11-13 09:59:52.000000000","message":"good catch, cut and paste formatting error.","commit_id":"b17ec02660aacf44ff3a3db4a46228018fa17e86"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"fefb57557c9d29c4d9059e8bf334baabe177b18e","unresolved":false,"context_lines":[{"line_number":847,"context_line":"    return \u0027VDI.resize\u0027"},{"line_number":848,"context_line":""},{"line_number":849,"context_line":""},{"line_number":850,"context_line":"def _vdi_get_virtual_size(session, vdi_ref):"},{"line_number":851,"context_line":"    size \u003d session.call_xenapi(\u0027VDI.get_virtual_size\u0027, vdi_ref)"},{"line_number":852,"context_line":"    return int(size)"},{"line_number":853,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATn%2F%2FLtc%3D","line":850,"updated":"2013-11-13 09:01:00.000000000","message":"If these two functions are going to be reused, please add docstrings. If not, please fold them into update_vdi_virtual_size as they don\u0027t add anything.","commit_id":"b17ec02660aacf44ff3a3db4a46228018fa17e86"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"78fc739357dfa57735cba24518c64f7fc0536b79","unresolved":false,"context_lines":[{"line_number":847,"context_line":"    return \u0027VDI.resize\u0027"},{"line_number":848,"context_line":""},{"line_number":849,"context_line":""},{"line_number":850,"context_line":"def _vdi_get_virtual_size(session, vdi_ref):"},{"line_number":851,"context_line":"    size \u003d session.call_xenapi(\u0027VDI.get_virtual_size\u0027, vdi_ref)"},{"line_number":852,"context_line":"    return int(size)"},{"line_number":853,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATn%2F9q68%3D","line":850,"in_reply_to":"AAAATn%2F%2FLcU%3D","updated":"2013-12-08 22:35:02.000000000","message":"I don\u0027t follow; same number of lines of mock code either way.","commit_id":"b17ec02660aacf44ff3a3db4a46228018fa17e86"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"55cfebbe8ecfd365e28f204248b7505e387c98bb","unresolved":false,"context_lines":[{"line_number":847,"context_line":"    return \u0027VDI.resize\u0027"},{"line_number":848,"context_line":""},{"line_number":849,"context_line":""},{"line_number":850,"context_line":"def _vdi_get_virtual_size(session, vdi_ref):"},{"line_number":851,"context_line":"    size \u003d session.call_xenapi(\u0027VDI.get_virtual_size\u0027, vdi_ref)"},{"line_number":852,"context_line":"    return int(size)"},{"line_number":853,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATn%2F%2FLcU%3D","line":850,"in_reply_to":"AAAATn%2F%2FLtc%3D","updated":"2013-11-13 09:59:52.000000000","message":"I am a fan of small easily testable function that add to readability.\n\nThe are plans to extract all these methods into another layer in the near future.\n\nBut the main reason is it helps with mock and returning different values for the different calls to call_xenapi with different args.","commit_id":"b17ec02660aacf44ff3a3db4a46228018fa17e86"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"b9495e4198213de86066ba9c34d2cb3d1426765c","unresolved":false,"context_lines":[{"line_number":847,"context_line":"    return \u0027VDI.resize\u0027"},{"line_number":848,"context_line":""},{"line_number":849,"context_line":""},{"line_number":850,"context_line":"def _vdi_get_virtual_size(session, vdi_ref):"},{"line_number":851,"context_line":"    size \u003d session.call_xenapi(\u0027VDI.get_virtual_size\u0027, vdi_ref)"},{"line_number":852,"context_line":"    return int(size)"},{"line_number":853,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAATn%2F9IvU%3D","line":850,"in_reply_to":"AAAATn%2F9q68%3D","updated":"2013-12-16 13:21:35.000000000","message":"The issue is multiple calls into session.call_xenapi with different arguments. Its a messier to capture those and check the calls made, and return different values depending on what gets passed in, its easier just to mock these helper functions, and it makes the code that calls these functions easier to read. But anyways, its all subjective I guess.\n\nBut the overriding thing here, is that I plan to pull all these little functions out into a more discoverable layer on top of the XenAPI library.","commit_id":"b17ec02660aacf44ff3a3db4a46228018fa17e86"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0829a3cc59708e8bb5180e645ee6f2c2cc2f8c8f","unresolved":false,"context_lines":[{"line_number":929,"context_line":"    session.call_xenapi(resize_func_name, vdi_ref, str(new_size))"},{"line_number":930,"context_line":""},{"line_number":931,"context_line":""},{"line_number":932,"context_line":"def update_vdi_virtual_size(session, instance, vdi_ref, new_gb):"},{"line_number":933,"context_line":"    virtual_size \u003d _vdi_get_virtual_size(session, vdi_ref)"},{"line_number":934,"context_line":"    new_disk_size \u003d new_gb * unit.Gi"},{"line_number":935,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAT3%2F%2FvUs%3D","line":932,"updated":"2014-01-02 16:27:57.000000000","message":"Doc strings would be good here.  I had to compare the old code and this new code to see how new_gb was being handled.  Why not just get new_gb from instance[\u0027root_gb\u0027] in this method and if it\u0027s not set you return, like the code did before?","commit_id":"6e4ce0808cf92b64cd22e3958d9f6a5563c93366"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"c20e2cead37963f6249c85bf4eb1cd83f74d1202","unresolved":false,"context_lines":[{"line_number":929,"context_line":"    session.call_xenapi(resize_func_name, vdi_ref, str(new_size))"},{"line_number":930,"context_line":""},{"line_number":931,"context_line":""},{"line_number":932,"context_line":"def update_vdi_virtual_size(session, instance, vdi_ref, new_gb):"},{"line_number":933,"context_line":"    virtual_size \u003d _vdi_get_virtual_size(session, vdi_ref)"},{"line_number":934,"context_line":"    new_disk_size \u003d new_gb * unit.Gi"},{"line_number":935,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAT3%2F%2FsB4%3D","line":932,"in_reply_to":"AAAAT3%2F%2FvUs%3D","updated":"2014-01-03 13:37:39.000000000","message":"Yeah, I am trying to avoid adding those, I like not to use instance[\u0027root_gb\u0027] down here, because its confusing during resize, etc.","commit_id":"6e4ce0808cf92b64cd22e3958d9f6a5563c93366"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0829a3cc59708e8bb5180e645ee6f2c2cc2f8c8f","unresolved":false,"context_lines":[{"line_number":947,"context_line":"        # NOTE(johngarbutt): we should never get here"},{"line_number":948,"context_line":"        # but if we don\u0027t raise an exception, a user might be able to use"},{"line_number":949,"context_line":"        # more storage than allowed by their chosen instance flavor"},{"line_number":950,"context_line":"        LOG.error(_(\"VDI %s is bigger than requested resize up size.\"),"},{"line_number":951,"context_line":"                  vdi_ref, instance\u003dinstance)"},{"line_number":952,"context_line":"        raise exception.ResizeError(_(\"VDI too big for requested resize up.\"))"},{"line_number":953,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAT3%2F%2FvTU%3D","line":950,"updated":"2014-01-02 16:27:57.000000000","message":"Would be good to also log virtual_size and new_disk_size here for context.","commit_id":"6e4ce0808cf92b64cd22e3958d9f6a5563c93366"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0829a3cc59708e8bb5180e645ee6f2c2cc2f8c8f","unresolved":false,"context_lines":[{"line_number":949,"context_line":"        # more storage than allowed by their chosen instance flavor"},{"line_number":950,"context_line":"        LOG.error(_(\"VDI %s is bigger than requested resize up size.\"),"},{"line_number":951,"context_line":"                  vdi_ref, instance\u003dinstance)"},{"line_number":952,"context_line":"        raise exception.ResizeError(_(\"VDI too big for requested resize up.\"))"},{"line_number":953,"context_line":""},{"line_number":954,"context_line":""},{"line_number":955,"context_line":"def resize_disk(session, instance, vdi_ref, instance_type):"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAT3%2F%2FvTI%3D","line":952,"updated":"2014-01-02 16:27:57.000000000","message":"And here.","commit_id":"6e4ce0808cf92b64cd22e3958d9f6a5563c93366"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"c20e2cead37963f6249c85bf4eb1cd83f74d1202","unresolved":false,"context_lines":[{"line_number":949,"context_line":"        # more storage than allowed by their chosen instance flavor"},{"line_number":950,"context_line":"        LOG.error(_(\"VDI %s is bigger than requested resize up size.\"),"},{"line_number":951,"context_line":"                  vdi_ref, instance\u003dinstance)"},{"line_number":952,"context_line":"        raise exception.ResizeError(_(\"VDI too big for requested resize up.\"))"},{"line_number":953,"context_line":""},{"line_number":954,"context_line":""},{"line_number":955,"context_line":"def resize_disk(session, instance, vdi_ref, instance_type):"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAT3%2F%2FsBs%3D","line":952,"in_reply_to":"AAAAT3%2F%2FvTI%3D","updated":"2014-01-03 13:37:39.000000000","message":"You know, I don\u0027t like this code, what if the virtual_size is equal to the new_disk_size, hmm...","commit_id":"6e4ce0808cf92b64cd22e3958d9f6a5563c93366"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0f72d19aab7bb475c86a58064968fddc76e9b522","unresolved":false,"context_lines":[{"line_number":936,"context_line":""},{"line_number":937,"context_line":"    msg \u003d _(\"Resizing up VDI %(vdi_ref)s from %(virtual_size)d \""},{"line_number":938,"context_line":"            \"to %(new_disk_size)d\")"},{"line_number":939,"context_line":"    LOG.debug(msg, {\u0027vdi_ref\u0027: vdi_ref, \u0027virtual_size\u0027: virtual_size,"},{"line_number":940,"context_line":"                    \u0027new_disk_size\u0027: new_disk_size},"},{"line_number":941,"context_line":"              instance\u003dinstance)"},{"line_number":942,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2FxhY%3D","line":939,"updated":"2014-01-08 16:27:51.000000000","message":"I would have left this in the block where you\u0027re actually doing the resize and then your elfi for \u003d\u003d check covers that case nicely, now you\u0027ll see this debug message that it\u0027s resizing even if it\u0027s \u003d\u003d or virtual_size \u003e new_disk_size, which could be confusing when you then see that it doesn\u0027t resize, but the patch is already approved now so oh well, that\u0027s my fault for not getting back to this review sooner.","commit_id":"98be70c20012e36b5b7c55175cdd17ee2c9d3d71"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"15d6ae1e31a0fde8a3fc06d5f555d54faeccd9b1","unresolved":false,"context_lines":[{"line_number":936,"context_line":""},{"line_number":937,"context_line":"    msg \u003d _(\"Resizing up VDI %(vdi_ref)s from %(virtual_size)d \""},{"line_number":938,"context_line":"            \"to %(new_disk_size)d\")"},{"line_number":939,"context_line":"    LOG.debug(msg, {\u0027vdi_ref\u0027: vdi_ref, \u0027virtual_size\u0027: virtual_size,"},{"line_number":940,"context_line":"                    \u0027new_disk_size\u0027: new_disk_size},"},{"line_number":941,"context_line":"              instance\u003dinstance)"},{"line_number":942,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2FxBE%3D","line":939,"in_reply_to":"AAAAUH%2F%2FxhY%3D","updated":"2014-01-08 18:12:14.000000000","message":"hmm, true, just seemed easier to keep it more consistent between the cases, but thats just laziness really.","commit_id":"98be70c20012e36b5b7c55175cdd17ee2c9d3d71"}],"nova/virt/xenapi/vmops.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0829a3cc59708e8bb5180e645ee6f2c2cc2f8c8f","unresolved":false,"context_lines":[{"line_number":1085,"context_line":"            resize_func_name \u003d self.check_resize_func_name()"},{"line_number":1086,"context_line":"            self._session.call_xenapi(resize_func_name, root_vdi[\u0027ref\u0027],"},{"line_number":1087,"context_line":"                    str(new_disk_size))"},{"line_number":1088,"context_line":"            LOG.debug(_(\"Resize complete\"), instance\u003dinstance)"},{"line_number":1089,"context_line":""},{"line_number":1090,"context_line":"    def check_resize_func_name(self):"},{"line_number":1091,"context_line":"        \"\"\"Check the function name used to resize an instance based"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAT3%2F%2FvRw%3D","side":"PARENT","line":1088,"updated":"2014-01-02 16:27:57.000000000","message":"Just a note but you didn\u0027t bring this over to vm_utils, not sure if that was intentional or not.","commit_id":"6a1667424a12b192402276ee81eb26a87bbb08f8"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"c20e2cead37963f6249c85bf4eb1cd83f74d1202","unresolved":false,"context_lines":[{"line_number":1085,"context_line":"            resize_func_name \u003d self.check_resize_func_name()"},{"line_number":1086,"context_line":"            self._session.call_xenapi(resize_func_name, root_vdi[\u0027ref\u0027],"},{"line_number":1087,"context_line":"                    str(new_disk_size))"},{"line_number":1088,"context_line":"            LOG.debug(_(\"Resize complete\"), instance\u003dinstance)"},{"line_number":1089,"context_line":""},{"line_number":1090,"context_line":"    def check_resize_func_name(self):"},{"line_number":1091,"context_line":"        \"\"\"Check the function name used to resize an instance based"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAT3%2F%2FsBo%3D","side":"PARENT","line":1088,"in_reply_to":"AAAAT3%2F%2FvRw%3D","updated":"2014-01-03 13:37:39.000000000","message":"Didn\u0027t seem worth keeping, given the extra logs for the error case.","commit_id":"6a1667424a12b192402276ee81eb26a87bbb08f8"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"fff0ded34aa2e40bae205c24c0b77421795e437b","unresolved":false,"context_lines":[{"line_number":1085,"context_line":"            resize_func_name \u003d self.check_resize_func_name()"},{"line_number":1086,"context_line":"            self._session.call_xenapi(resize_func_name, root_vdi[\u0027ref\u0027],"},{"line_number":1087,"context_line":"                    str(new_disk_size))"},{"line_number":1088,"context_line":"            LOG.debug(_(\"Resize complete\"), instance\u003dinstance)"},{"line_number":1089,"context_line":""},{"line_number":1090,"context_line":"    def check_resize_func_name(self):"},{"line_number":1091,"context_line":"        \"\"\"Check the function name used to resize an instance based"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAT3%2F%2FsAk%3D","side":"PARENT","line":1088,"in_reply_to":"AAAAT3%2F%2FvRw%3D","updated":"2014-01-03 13:40:17.000000000","message":"It was a tidy up really, wasn\u0027t needed.","commit_id":"6a1667424a12b192402276ee81eb26a87bbb08f8"}]}