)]}'
{"nova/tests/test_hypervapi.py":[{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"35e1171f55bfa61ba0584987ff18484423721fd8","unresolved":false,"context_lines":[{"line_number":877,"context_line":""},{"line_number":878,"context_line":"        self._mox.ReplayAll()"},{"line_number":879,"context_line":"        self._spawn_instance(cow)"},{"line_number":880,"context_line":"        self._mox.VerifyAll()"},{"line_number":881,"context_line":""},{"line_number":882,"context_line":"        self.assertEquals(len(self._instance_ide_disks), expected_ide_disks)"},{"line_number":883,"context_line":"        self.assertEquals(len(self._instance_ide_dvds), expected_ide_dvds)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAN3%2F%2FtXA%3D","side":"PARENT","line":880,"updated":"2013-03-15 18:28:20.000000000","message":"David, I think this VerifyAll got lost in the change for some reason.  It should have been moved to the end of the function.  Can you check on that?","commit_id":"f14cd697c005f946886f90f7af8379c58e062ed1"},{"author":{"_account_id":3185,"name":"Alessandro Pilotti","email":"apilotti@cloudbasesolutions.com","username":"alexpilotti"},"change_message_id":"4e9a3e97485efbff435a9a0904bc8480439c8598","unresolved":false,"context_lines":[{"line_number":877,"context_line":""},{"line_number":878,"context_line":"        self._mox.ReplayAll()"},{"line_number":879,"context_line":"        self._spawn_instance(cow)"},{"line_number":880,"context_line":"        self._mox.VerifyAll()"},{"line_number":881,"context_line":""},{"line_number":882,"context_line":"        self.assertEquals(len(self._instance_ide_disks), expected_ide_disks)"},{"line_number":883,"context_line":"        self.assertEquals(len(self._instance_ide_dvds), expected_ide_dvds)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAN3%2F%2FtWY%3D","side":"PARENT","line":880,"in_reply_to":"AAAAN3%2F%2FtXA%3D","updated":"2013-03-15 18:33:56.000000000","message":"Good catch!","commit_id":"f14cd697c005f946886f90f7af8379c58e062ed1"},{"author":{"_account_id":5313,"name":"David Peraza","email":"david_peraza@persistentsys.com","username":"dperaza"},"change_message_id":"d8fdfcc12090b49183c331e45898b615f1959337","unresolved":false,"context_lines":[{"line_number":877,"context_line":""},{"line_number":878,"context_line":"        self._mox.ReplayAll()"},{"line_number":879,"context_line":"        self._spawn_instance(cow)"},{"line_number":880,"context_line":"        self._mox.VerifyAll()"},{"line_number":881,"context_line":""},{"line_number":882,"context_line":"        self.assertEquals(len(self._instance_ide_disks), expected_ide_disks)"},{"line_number":883,"context_line":"        self.assertEquals(len(self._instance_ide_dvds), expected_ide_dvds)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAN3%2F%2FtQU%3D","side":"PARENT","line":880,"in_reply_to":"AAAAN3%2F%2FtXA%3D","updated":"2013-03-15 19:47:41.000000000","message":"Yes, thanks, updating","commit_id":"f14cd697c005f946886f90f7af8379c58e062ed1"},{"author":{"_account_id":3185,"name":"Alessandro Pilotti","email":"apilotti@cloudbasesolutions.com","username":"alexpilotti"},"change_message_id":"613fc7994846c035569b8304dff15186e14cc92e","unresolved":false,"context_lines":[{"line_number":920,"context_line":"        vhd_path \u003d pathutils.PathUtils().get_vhd_path(self._test_vm_name)"},{"line_number":921,"context_line":"        self.assertEquals(vhd_path, self._instance_ide_disks[0])"},{"line_number":922,"context_line":""},{"line_number":923,"context_line":"        self._mox.VerifyAll()"},{"line_number":924,"context_line":""},{"line_number":925,"context_line":"    def _mock_get_mounted_disk_from_lun(self, target_iqn, target_lun,"},{"line_number":926,"context_line":"                                        fake_mounted_disk,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAOH%2F%2F8Ts%3D","line":923,"updated":"2013-03-20 00:00:26.000000000","message":"Can you please explain why this line has been moved after the asserts? Thanks","commit_id":"3995db60ddbdb1f29d59510d6553c61a0bfc3b53"},{"author":{"_account_id":3185,"name":"Alessandro Pilotti","email":"apilotti@cloudbasesolutions.com","username":"alexpilotti"},"change_message_id":"4a9a8ea11d4454140f5839ff227121cc3b0b066d","unresolved":false,"context_lines":[{"line_number":920,"context_line":"        vhd_path \u003d pathutils.PathUtils().get_vhd_path(self._test_vm_name)"},{"line_number":921,"context_line":"        self.assertEquals(vhd_path, self._instance_ide_disks[0])"},{"line_number":922,"context_line":""},{"line_number":923,"context_line":"        self._mox.VerifyAll()"},{"line_number":924,"context_line":""},{"line_number":925,"context_line":"    def _mock_get_mounted_disk_from_lun(self, target_iqn, target_lun,"},{"line_number":926,"context_line":"                                        fake_mounted_disk,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAOH%2F%2F7nc%3D","line":923,"in_reply_to":"AAAAOH%2F%2F7uQ%3D","updated":"2013-03-20 14:54:17.000000000","message":"Mocked classes are meant to be called only by the code being tested, not by the testing routines. Al tests should follow the same format:\n\n1) Setup mocks and stubs\n2) ReplayAll\n3) Execute tested code\n4) VerifyAll\n5) Verify asserts\n\nOne option here is to replace the call with something like:\n\nvhd_path \u003d os.path.join(self._test_instance_dir, \u0027root.vhd\u0027)\n\nget_vhd_path() didn\u0027t need to be mocked originally due to its trivial implementation. Your patch is providing a slightly more complex behavior which means that get_vhd_path() will need to be mocked as well. Since this affects other tests as well, I suggest to leave it for a separate refactoring patch.\n\nThanks.","commit_id":"3995db60ddbdb1f29d59510d6553c61a0bfc3b53"},{"author":{"_account_id":6928,"name":"Ben Nemec","email":"openstack@nemebean.com","username":"bnemec"},"change_message_id":"d098448d1da67cfd49a9a4ff3ddd0a57688b01e7","unresolved":false,"context_lines":[{"line_number":920,"context_line":"        vhd_path \u003d pathutils.PathUtils().get_vhd_path(self._test_vm_name)"},{"line_number":921,"context_line":"        self.assertEquals(vhd_path, self._instance_ide_disks[0])"},{"line_number":922,"context_line":""},{"line_number":923,"context_line":"        self._mox.VerifyAll()"},{"line_number":924,"context_line":""},{"line_number":925,"context_line":"    def _mock_get_mounted_disk_from_lun(self, target_iqn, target_lun,"},{"line_number":926,"context_line":"                                        fake_mounted_disk,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAOH%2F%2F7uQ%3D","line":923,"in_reply_to":"AAAAOH%2F%2F8Ts%3D","updated":"2013-03-20 13:21:18.000000000","message":"get_vhd_path calls get_instance_dir, and with that mocked it would throw an error if I didn\u0027t include it in the expected mock calls.  That meant the VerifyAll had to move after the get_vhd_path too.","commit_id":"3995db60ddbdb1f29d59510d6553c61a0bfc3b53"},{"author":{"_account_id":3185,"name":"Alessandro Pilotti","email":"apilotti@cloudbasesolutions.com","username":"alexpilotti"},"change_message_id":"529116e818c13ca7c05ac1e36460ccf9d394001d","unresolved":false,"context_lines":[{"line_number":1060,"context_line":"            fake_dest_ip \u003d \u002710.0.0.2\u0027"},{"line_number":1061,"context_line":""},{"line_number":1062,"context_line":"        fake_root_vhd_path \u003d \u0027C:\\\\FakePath\\\\root.vhd\u0027"},{"line_number":1063,"context_line":"        fake_revert_path \u003d (self._test_instance_dir + \u0027\\\\_revert\u0027)"},{"line_number":1064,"context_line":""},{"line_number":1065,"context_line":"        func \u003d mox.Func(self._check_instance_name)"},{"line_number":1066,"context_line":"        vmutils.VMUtils.set_vm_state(func, constants.HYPERV_VM_STATE_DISABLED)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAOH%2F%2F7E8%3D","line":1063,"updated":"2013-03-20 20:51:49.000000000","message":"Parenthesis are used for line wrapping only in this case, which is not necessary anymore as the line became shorter. \n\nEven better if you can replace it with:\nos.path.join(self._test_instance_dir, \u0027_revert\u0027)\nUnless it creates issues for testing on Linux due the different path separator of course.","commit_id":"a9c2f287a2d184200c0dd2f779b1f17e80ee260a"},{"author":{"_account_id":5313,"name":"David Peraza","email":"david_peraza@persistentsys.com","username":"dperaza"},"change_message_id":"cb465c41aae27dff9857163ac5a0a11d4f5ae31c","unresolved":false,"context_lines":[{"line_number":1060,"context_line":"            fake_dest_ip \u003d \u002710.0.0.2\u0027"},{"line_number":1061,"context_line":""},{"line_number":1062,"context_line":"        fake_root_vhd_path \u003d \u0027C:\\\\FakePath\\\\root.vhd\u0027"},{"line_number":1063,"context_line":"        fake_revert_path \u003d (self._test_instance_dir + \u0027\\\\_revert\u0027)"},{"line_number":1064,"context_line":""},{"line_number":1065,"context_line":"        func \u003d mox.Func(self._check_instance_name)"},{"line_number":1066,"context_line":"        vmutils.VMUtils.set_vm_state(func, constants.HYPERV_VM_STATE_DISABLED)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAOH%2F%2F7DE%3D","line":1063,"in_reply_to":"AAAAOH%2F%2F7E8%3D","updated":"2013-03-20 21:00:56.000000000","message":"Done","commit_id":"a9c2f287a2d184200c0dd2f779b1f17e80ee260a"}],"nova/virt/hyperv/vmops.py":[{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"bbabf2d59e2c21d5d4e3a91a54ee120ebb994e45","unresolved":false,"context_lines":[{"line_number":159,"context_line":"            raise exception.InstanceExists(name\u003dinstance_name)"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        # Make sure we\u0027re starting with a clean slate."},{"line_number":162,"context_line":"        self._delete_disk_files(instance_name)"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        if self._volumeops.ebs_root_in_block_devices(block_device_info):"},{"line_number":165,"context_line":"            root_vhd_path \u003d None"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAN3%2F%2FwF4%3D","line":162,"updated":"2013-03-13 20:14:57.000000000","message":"Is there a check somewhere to ensure that the instance name is unique? What if I have two instances with the same name (for example launching two instances in one api call)?","commit_id":"3050ffeb6bcceb679987b0854ff54fd1c4d263c0"},{"author":{"_account_id":3185,"name":"Alessandro Pilotti","email":"apilotti@cloudbasesolutions.com","username":"alexpilotti"},"change_message_id":"767064b825ca159b21176b52a02414215e25f418","unresolved":false,"context_lines":[{"line_number":159,"context_line":"            raise exception.InstanceExists(name\u003dinstance_name)"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        # Make sure we\u0027re starting with a clean slate."},{"line_number":162,"context_line":"        self._delete_disk_files(instance_name)"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        if self._volumeops.ebs_root_in_block_devices(block_device_info):"},{"line_number":165,"context_line":"            root_vhd_path \u003d None"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAN3%2F%2FwEo%3D","line":162,"in_reply_to":"AAAAN3%2F%2FwF4%3D","updated":"2013-03-13 20:24:45.000000000","message":"Sorry, I put the comment in my previos review instead of here.","commit_id":"3050ffeb6bcceb679987b0854ff54fd1c4d263c0"},{"author":{"_account_id":5313,"name":"David Peraza","email":"david_peraza@persistentsys.com","username":"dperaza"},"change_message_id":"b68e8ba5700236756b20e75668d9f01835105bd6","unresolved":false,"context_lines":[{"line_number":159,"context_line":"            raise exception.InstanceExists(name\u003dinstance_name)"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"        # Make sure we\u0027re starting with a clean slate."},{"line_number":162,"context_line":"        self._delete_disk_files(instance_name)"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        if self._volumeops.ebs_root_in_block_devices(block_device_info):"},{"line_number":165,"context_line":"            root_vhd_path \u003d None"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAN3%2F%2Fucg%3D","line":162,"in_reply_to":"AAAAN3%2F%2FwF4%3D","updated":"2013-03-14 21:18:06.000000000","message":"This check is done higher at the API and Manager Layer, from an Openstack point of view the instance name is always unique compared with the DB, the problem is when we reset the DB, for example in a test environment we switch from folsom to grizzly or back and there is not migration path, or less likely but possible when and user gets on an error state during migration and they need to cleanup DB. Some instances might have been left behind. This an attempt to fix by starting with clean directories even after possible scenarios above. Open to suggestions on other ways to solve this problem.","commit_id":"3050ffeb6bcceb679987b0854ff54fd1c4d263c0"}]}