)]}'
{"nova/tests/virt/xenapi/test_vm_utils.py":[{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"6e6bc3fa3f7a8eb0827a51071d6e1e4d13135fe6","unresolved":false,"context_lines":[{"line_number":1008,"context_line":"        # Stubbing on the session object and not class so we don\u0027t pollute"},{"line_number":1009,"context_line":"        # other tests"},{"line_number":1010,"context_line":"        self.session.VDI_add_to_other_config \u003d VDI_add_to_other_config"},{"line_number":1011,"context_line":"        self.session.VDI_get_other_config \u003d VDI_get_other_config"},{"line_number":1012,"context_line":"        self.session.call_plugin_serialized \u003d call_plugin_serialized"},{"line_number":1013,"context_line":""},{"line_number":1014,"context_line":"        self.stubs.Set(vm_utils, \u0027get_sr_path\u0027, lambda *a, **k: None)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BOAY%3D","line":1011,"updated":"2013-11-28 20:08:38.000000000","message":"Comment: Your test would be smaller (and still clear IMO) if you said\n\n self.session.VDI_get_other_config \u003d lambda: {}","commit_id":"86df0b3642f56e34feca1d0f734e85be1dd86340"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"9b2d040df359a7b83b38e85f1da9658d1626f3f0","unresolved":false,"context_lines":[{"line_number":1008,"context_line":"        # Stubbing on the session object and not class so we don\u0027t pollute"},{"line_number":1009,"context_line":"        # other tests"},{"line_number":1010,"context_line":"        self.session.VDI_add_to_other_config \u003d VDI_add_to_other_config"},{"line_number":1011,"context_line":"        self.session.VDI_get_other_config \u003d VDI_get_other_config"},{"line_number":1012,"context_line":"        self.session.call_plugin_serialized \u003d call_plugin_serialized"},{"line_number":1013,"context_line":""},{"line_number":1014,"context_line":"        self.stubs.Set(vm_utils, \u0027get_sr_path\u0027, lambda *a, **k: None)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BN4w%3D","line":1011,"in_reply_to":"AAAATn%2F%2BOAY%3D","updated":"2013-11-28 22:41:52.000000000","message":"Done","commit_id":"86df0b3642f56e34feca1d0f734e85be1dd86340"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"a4b829099f0b8daa8b25b76d63288f4803214b0e","unresolved":false,"context_lines":[{"line_number":1006,"context_line":"        # Stubbing on the session object and not class so we don\u0027t pollute"},{"line_number":1007,"context_line":"        # other tests"},{"line_number":1008,"context_line":"        self.session.VDI_add_to_other_config \u003d VDI_add_to_other_config"},{"line_number":1009,"context_line":"        self.session.VDI_get_other_config \u003d VDI_get_other_config"},{"line_number":1010,"context_line":"        self.session.call_plugin_serialized \u003d call_plugin_serialized"},{"line_number":1011,"context_line":""},{"line_number":1012,"context_line":"        self.stubs.Set(vm_utils, \u0027get_sr_path\u0027, lambda *a, **k: None)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAATn%2F9Cm4%3D","line":1009,"updated":"2013-12-17 12:15:52.000000000","message":"probably should make the same change here you did above?","commit_id":"d009f45b0295d7e062ca1fabaaec92d6183dc235"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"da79cef8968a4e083a307db9aaf8ebe27581df8e","unresolved":false,"context_lines":[{"line_number":1006,"context_line":"        # Stubbing on the session object and not class so we don\u0027t pollute"},{"line_number":1007,"context_line":"        # other tests"},{"line_number":1008,"context_line":"        self.session.VDI_add_to_other_config \u003d VDI_add_to_other_config"},{"line_number":1009,"context_line":"        self.session.VDI_get_other_config \u003d VDI_get_other_config"},{"line_number":1010,"context_line":"        self.session.call_plugin_serialized \u003d call_plugin_serialized"},{"line_number":1011,"context_line":""},{"line_number":1012,"context_line":"        self.stubs.Set(vm_utils, \u0027get_sr_path\u0027, lambda *a, **k: None)"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAATn%2F9Cmg%3D","line":1009,"in_reply_to":"AAAATn%2F9Cm4%3D","updated":"2013-12-17 12:18:59.000000000","message":"Done","commit_id":"d009f45b0295d7e062ca1fabaaec92d6183dc235"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"e7b4c5576f485b657f3c1da3af8a074d4b75c94b","unresolved":false,"context_lines":[{"line_number":1808,"context_line":"    @mock.patch.object(vm_utils, \u0027_try_strip_base_mirror_from_vdi\u0027)"},{"line_number":1809,"context_line":"    def test_strip_base_mirror_from_vdis(self, mock_strip):"},{"line_number":1810,"context_line":"        def call_xenapi(method, arg):"},{"line_number":1811,"context_line":"            if method \u003d\u003d \"VM.get_VBDs\":"},{"line_number":1812,"context_line":"                return [\u0027VBD_ref_1\u0027, \u0027VBD_ref_2\u0027]"},{"line_number":1813,"context_line":"            if method \u003d\u003d \"VBD.get_VDI\":"},{"line_number":1814,"context_line":"                return \u0027VDI\u0027 + arg[3:]"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUH%2F%2Fyh0%3D","line":1811,"updated":"2014-01-08 14:51:20.000000000","message":"I hate this bit of mock, it sucks.\n\nIts why I added the get_VBDs helper methods and stuff, but I am not sure thats a better solution.\n\nPS, I am good with this change, I just felt the need to share my anger on this subject.","commit_id":"127e55a40d4b3e3ceefa99027d72749296fc5ef8"}],"nova/virt/xenapi/fake.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"e7b4c5576f485b657f3c1da3af8a074d4b75c94b","unresolved":false,"context_lines":[{"line_number":127,"context_line":""},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"def create_vm(name_label, status, **kwargs):"},{"line_number":130,"context_line":"    if status.lower() \u003d\u003d \u0027running\u0027:"},{"line_number":131,"context_line":"        domid \u003d random.randrange(1, 1 \u003c\u003c 16)"},{"line_number":132,"context_line":"        resident_on \u003d _db_content[\u0027host\u0027].keys()[0]"},{"line_number":133,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUH%2F%2Fyhs%3D","line":130,"updated":"2014-01-08 14:51:20.000000000","message":"is this going to hide errors in the code? or does xenapi do the equivalent?","commit_id":"127e55a40d4b3e3ceefa99027d72749296fc5ef8"}],"nova/virt/xenapi/vm_utils.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"0c3b7f6dbb237762ee26289c3f5c26ff796b1bf1","unresolved":false,"context_lines":[{"line_number":747,"context_line":"def strip_base_mirror_from_vdis(session, vm_ref):"},{"line_number":748,"context_line":"    # NOTE(johngarbutt) part of workaround for XenServer bug CA-98606"},{"line_number":749,"context_line":"    vbd_refs \u003d session.call_xenapi(\"VM.get_VBDs\", vm_ref)"},{"line_number":750,"context_line":"    for vbd in vbd_refs:"},{"line_number":751,"context_line":"        vdi_ref \u003d session.call_xenapi(\"VBD.get_VDI\", vbd)"},{"line_number":752,"context_line":"        _try_strip_base_mirror_from_vdi(session, vdi_ref)"},{"line_number":753,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2BHpg%3D","line":750,"updated":"2013-12-02 11:52:28.000000000","message":"should be vbd_ref, but thats another change I guess.","commit_id":"207153ec65888e93ae1de053d15ac7d4d3b9b2a7"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"6e58ad29cb12ffe2959d93aaecddf2d086595075","unresolved":false,"context_lines":[{"line_number":747,"context_line":"def strip_base_mirror_from_vdis(session, vm_ref):"},{"line_number":748,"context_line":"    # NOTE(johngarbutt) part of workaround for XenServer bug CA-98606"},{"line_number":749,"context_line":"    vbd_refs \u003d session.call_xenapi(\"VM.get_VBDs\", vm_ref)"},{"line_number":750,"context_line":"    for vbd in vbd_refs:"},{"line_number":751,"context_line":"        vdi_ref \u003d session.call_xenapi(\"VBD.get_VDI\", vbd)"},{"line_number":752,"context_line":"        _try_strip_base_mirror_from_vdi(session, vdi_ref)"},{"line_number":753,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2BGA0%3D","line":750,"in_reply_to":"AAAATn%2F%2BHpg%3D","updated":"2013-12-02 16:49:19.000000000","message":"Done","commit_id":"207153ec65888e93ae1de053d15ac7d4d3b9b2a7"}],"nova/virt/xenapi/vmops.py":[{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"6e6bc3fa3f7a8eb0827a51071d6e1e4d13135fe6","unresolved":false,"context_lines":[{"line_number":1226,"context_line":"                    self._delete_from_xenstore(instance, location,"},{"line_number":1227,"context_line":"                                               vm_ref\u003dvm_ref)"},{"line_number":1228,"context_line":"                except exception.InstanceNotFound:"},{"line_number":1229,"context_line":"                    # catch exception for domid if instance isn\u0027t running"},{"line_number":1230,"context_line":"                    pass"},{"line_number":1231,"context_line":"            elif change[0] \u003d\u003d \u0027+\u0027:"},{"line_number":1232,"context_line":"                self._add_to_param_xenstore(vm_ref, location,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BN%2Fw%3D","line":1229,"updated":"2013-11-28 20:08:38.000000000","message":"I realise this is just a mechanical change, but I can\u0027t make head or tails of the comment: why do we catch InstanceNotFound rather than propogating it?","commit_id":"86df0b3642f56e34feca1d0f734e85be1dd86340"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"9b2d040df359a7b83b38e85f1da9658d1626f3f0","unresolved":false,"context_lines":[{"line_number":1226,"context_line":"                    self._delete_from_xenstore(instance, location,"},{"line_number":1227,"context_line":"                                               vm_ref\u003dvm_ref)"},{"line_number":1228,"context_line":"                except exception.InstanceNotFound:"},{"line_number":1229,"context_line":"                    # catch exception for domid if instance isn\u0027t running"},{"line_number":1230,"context_line":"                    pass"},{"line_number":1231,"context_line":"            elif change[0] \u003d\u003d \u0027+\u0027:"},{"line_number":1232,"context_line":"                self._add_to_param_xenstore(vm_ref, location,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAATn%2F%2BN44%3D","line":1229,"in_reply_to":"AAAATn%2F%2BN%2Fw%3D","updated":"2013-11-28 22:41:52.000000000","message":"Updated the comment to explain.  Basically the VM record has a  field which will be the contents of xenstore on next boot (the param_xenstore referenced above) - but if the VM is already running we need to push the same changes directly to the VM too.","commit_id":"86df0b3642f56e34feca1d0f734e85be1dd86340"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"0c3b7f6dbb237762ee26289c3f5c26ff796b1bf1","unresolved":false,"context_lines":[{"line_number":1226,"context_line":"                    self._delete_from_xenstore(instance, location,"},{"line_number":1227,"context_line":"                                               vm_ref\u003dvm_ref)"},{"line_number":1228,"context_line":"                except exception.InstanceNotFound:"},{"line_number":1229,"context_line":"                    # If the VM is not running then no need to update"},{"line_number":1230,"context_line":"                    # the live xenstore - the param xenstore will be"},{"line_number":1231,"context_line":"                    # used next time the VM is booted"},{"line_number":1232,"context_line":"                    pass"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2BHpM%3D","line":1229,"updated":"2013-12-02 11:52:28.000000000","message":"Can we add some unit tests for this now?","commit_id":"207153ec65888e93ae1de053d15ac7d4d3b9b2a7"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"6e58ad29cb12ffe2959d93aaecddf2d086595075","unresolved":false,"context_lines":[{"line_number":1226,"context_line":"                    self._delete_from_xenstore(instance, location,"},{"line_number":1227,"context_line":"                                               vm_ref\u003dvm_ref)"},{"line_number":1228,"context_line":"                except exception.InstanceNotFound:"},{"line_number":1229,"context_line":"                    # If the VM is not running then no need to update"},{"line_number":1230,"context_line":"                    # the live xenstore - the param xenstore will be"},{"line_number":1231,"context_line":"                    # used next time the VM is booted"},{"line_number":1232,"context_line":"                    pass"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2BF7A%3D","line":1229,"in_reply_to":"AAAATn%2F%2BHpM%3D","updated":"2013-12-02 16:49:19.000000000","message":"The code the needs unit testing is _make_plugin_call which is not tested at all currently.  The error codes make this a tricky unit test to write so I think it should be in a separate change and not bundled with the speed improvements.","commit_id":"207153ec65888e93ae1de053d15ac7d4d3b9b2a7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"0c3b7f6dbb237762ee26289c3f5c26ff796b1bf1","unresolved":false,"context_lines":[{"line_number":1790,"context_line":"    def _get_dom_id(self, instance\u003dNone, vm_ref\u003dNone, check_rescue\u003dFalse):"},{"line_number":1791,"context_line":"        vm_ref \u003d vm_ref or self._get_vm_opaque_ref(instance, check_rescue)"},{"line_number":1792,"context_line":"        domid \u003d self._session.call_xenapi(\"VM.get_domid\", vm_ref)"},{"line_number":1793,"context_line":"        if not domid or domid \u003d\u003d -1:"},{"line_number":1794,"context_line":"            raise exception.InstanceNotFound(instance_id\u003dinstance[\u0027name\u0027])"},{"line_number":1795,"context_line":"        return domid"},{"line_number":1796,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2BHpU%3D","line":1793,"updated":"2013-12-02 11:52:28.000000000","message":"I am tempted to say put this in a separate change with a nice description of what you changed.","commit_id":"207153ec65888e93ae1de053d15ac7d4d3b9b2a7"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"6e58ad29cb12ffe2959d93aaecddf2d086595075","unresolved":false,"context_lines":[{"line_number":1790,"context_line":"    def _get_dom_id(self, instance\u003dNone, vm_ref\u003dNone, check_rescue\u003dFalse):"},{"line_number":1791,"context_line":"        vm_ref \u003d vm_ref or self._get_vm_opaque_ref(instance, check_rescue)"},{"line_number":1792,"context_line":"        domid \u003d self._session.call_xenapi(\"VM.get_domid\", vm_ref)"},{"line_number":1793,"context_line":"        if not domid or domid \u003d\u003d -1:"},{"line_number":1794,"context_line":"            raise exception.InstanceNotFound(instance_id\u003dinstance[\u0027name\u0027])"},{"line_number":1795,"context_line":"        return domid"},{"line_number":1796,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2BF%2Fk%3D","line":1793,"in_reply_to":"AAAATn%2F%2BHpU%3D","updated":"2013-12-02 16:49:19.000000000","message":"I\u0027m not sure there\u0027s benefit to that, and I suspect it would be hard to do in a way that made sense.  The get_domid change depends on checking for -1, so we\u0027d have to introduce the \"-1\" change first and say \"if domid \u003d\u003d -1 then throw KeyError\" which is very weird.","commit_id":"207153ec65888e93ae1de053d15ac7d4d3b9b2a7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"e7b4c5576f485b657f3c1da3af8a074d4b75c94b","unresolved":false,"context_lines":[{"line_number":1800,"context_line":"        vm_ref \u003d vm_ref or self._get_vm_opaque_ref(instance, check_rescue)"},{"line_number":1801,"context_line":"        domid \u003d self._session.call_xenapi(\"VM.get_domid\", vm_ref)"},{"line_number":1802,"context_line":"        if not domid or domid \u003d\u003d -1:"},{"line_number":1803,"context_line":"            raise exception.InstanceNotFound(instance_id\u003dinstance[\u0027name\u0027])"},{"line_number":1804,"context_line":"        return domid"},{"line_number":1805,"context_line":""},{"line_number":1806,"context_line":"    def _add_to_param_xenstore(self, vm_ref, key, val):"}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUH%2F%2FyCA%3D","line":1803,"updated":"2014-01-08 14:51:20.000000000","message":"it could be in a separate change, but in this case that would be overkill.\n\nI like the use of the better exception.\n\nAgain, I would bring up the lack of direct unit tests, but I think this is tested using the fake object, if I remember correctly, so thats cool.","commit_id":"127e55a40d4b3e3ceefa99027d72749296fc5ef8"}]}