)]}'
{"nova/compute/manager.py":[{"author":{"_account_id":4419,"name":"Matthew Sherborne","email":"msherborne+openstack@gmail.com","username":"msherborne+openstack"},"change_message_id":"78ea45d96a46eecaf6f3cd976de4021f3c3794be","unresolved":false,"context_lines":[{"line_number":2695,"context_line":"            connect_info[\u0027token\u0027] \u003d token"},{"line_number":2696,"context_line":"            connect_info[\u0027access_url\u0027] \u003d access_url"},{"line_number":2697,"context_line":"        except exception.InstanceNotFound:"},{"line_number":2698,"context_line":"            if instance[\u0027vm_state\u0027] not in [vm_states.BUILDING,"},{"line_number":2699,"context_line":"                                            vm_states.RESCUED]:"},{"line_number":2700,"context_line":"                raise"},{"line_number":2701,"context_line":"            raise exception.InstanceNotReady(instance_id\u003dinstance[\u0027uuid\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAOH%2F%2F6OM%3D","line":2698,"updated":"2013-03-21 10:12:02.000000000","message":"maybe could be a tuple instead of a list to be slightly more efficient, and make me feel good.","commit_id":"60151ba0d239bc86de3f46b3b6b47e9e71908709"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"12b0733c69d370d10e08f9006c07c9c8f76c1804","unresolved":false,"context_lines":[{"line_number":2696,"context_line":"            connect_info[\u0027access_url\u0027] \u003d access_url"},{"line_number":2697,"context_line":"        except exception.InstanceNotFound:"},{"line_number":2698,"context_line":"            if instance[\u0027vm_state\u0027] not in [vm_states.BUILDING,"},{"line_number":2699,"context_line":"                                            vm_states.RESCUED]:"},{"line_number":2700,"context_line":"                raise"},{"line_number":2701,"context_line":"            raise exception.InstanceNotReady(instance_id\u003dinstance[\u0027uuid\u0027])"},{"line_number":2702,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAOH%2F%2F6Oc%3D","line":2699,"updated":"2013-03-21 10:02:11.000000000","message":"Not sure this is correct. We could check for the task_state of \"rescuing\" which is equivalent to the building vm_state.\n\nIf the instance has been rescued, but goes away for some reason, instance not ready is incorrect.\n\nBut I agree that during the \"rescuing\" stage it *should* say InstanceNotReady, but frankly not sure it is worth the complexity, because we could start looking at loads of states, like \"migrating\", and that is really a whole separate bug you might want to raise.","commit_id":"60151ba0d239bc86de3f46b3b6b47e9e71908709"},{"author":{"_account_id":4419,"name":"Matthew Sherborne","email":"msherborne+openstack@gmail.com","username":"msherborne+openstack"},"change_message_id":"78ea45d96a46eecaf6f3cd976de4021f3c3794be","unresolved":false,"context_lines":[{"line_number":2730,"context_line":"            connect_info[\u0027token\u0027] \u003d token"},{"line_number":2731,"context_line":"            connect_info[\u0027access_url\u0027] \u003d access_url"},{"line_number":2732,"context_line":"        except exception.InstanceNotFound:"},{"line_number":2733,"context_line":"            if instance[\u0027vm_state\u0027] not in [vm_states.BUILDING,"},{"line_number":2734,"context_line":"                                            vm_states.RESCUED]:"},{"line_number":2735,"context_line":"                raise"},{"line_number":2736,"context_line":"            raise exception.InstanceNotReady(instance_id\u003dinstance[\u0027uuid\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAOH%2F%2F6OI%3D","line":2733,"updated":"2013-03-21 10:12:02.000000000","message":"ditto","commit_id":"60151ba0d239bc86de3f46b3b6b47e9e71908709"}],"nova/tests/compute/test_compute.py":[{"author":{"_account_id":4419,"name":"Matthew Sherborne","email":"msherborne+openstack@gmail.com","username":"msherborne+openstack"},"change_message_id":"78ea45d96a46eecaf6f3cd976de4021f3c3794be","unresolved":false,"context_lines":[{"line_number":1705,"context_line":"                    \u0027internal_access_path\u0027: \u0027fake_rescue_path\u0027}"},{"line_number":1706,"context_line":""},{"line_number":1707,"context_line":"        def fake_xenapi_driver_get_rescue_console(*args, **kwargs):"},{"line_number":1708,"context_line":"            #Case where virt finds the rescued instance"},{"line_number":1709,"context_line":"            if instance[\u0027vm_state\u0027] \u003d\u003d vm_states.RESCUED:"},{"line_number":1710,"context_line":"                return {\u0027host\u0027: \u0027fake_host\u0027, \u0027port\u0027: 80,"},{"line_number":1711,"context_line":"                        \u0027internal_access_path\u0027: \u0027fake_rescue_path\u0027}"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAOH%2F%2F6OE%3D","line":1708,"updated":"2013-03-21 10:12:02.000000000","message":"Space after # please","commit_id":"60151ba0d239bc86de3f46b3b6b47e9e71908709"},{"author":{"_account_id":4419,"name":"Matthew Sherborne","email":"msherborne+openstack@gmail.com","username":"msherborne+openstack"},"change_message_id":"78ea45d96a46eecaf6f3cd976de4021f3c3794be","unresolved":false,"context_lines":[{"line_number":1707,"context_line":"        def fake_xenapi_driver_get_rescue_console(*args, **kwargs):"},{"line_number":1708,"context_line":"            #Case where virt finds the rescued instance"},{"line_number":1709,"context_line":"            if instance[\u0027vm_state\u0027] \u003d\u003d vm_states.RESCUED:"},{"line_number":1710,"context_line":"                return {\u0027host\u0027: \u0027fake_host\u0027, \u0027port\u0027: 80,"},{"line_number":1711,"context_line":"                        \u0027internal_access_path\u0027: \u0027fake_rescue_path\u0027}"},{"line_number":1712,"context_line":""},{"line_number":1713,"context_line":"        self.stubs.Set(self.compute.driver, \"get_vnc_console\","}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAOH%2F%2F6N4%3D","line":1710,"updated":"2013-03-21 10:12:02.000000000","message":"I would just go here: return expected\n\nThat way you avoid code duplication and a chance of forgetting to change one.","commit_id":"60151ba0d239bc86de3f46b3b6b47e9e71908709"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"12b0733c69d370d10e08f9006c07c9c8f76c1804","unresolved":false,"context_lines":[{"line_number":1716,"context_line":"        console \u003d self.compute.get_vnc_console(self.context, \u0027xvpvnc\u0027,"},{"line_number":1717,"context_line":"                                               instance\u003dinstance)"},{"line_number":1718,"context_line":""},{"line_number":1719,"context_line":"        self.assert_(console)"},{"line_number":1720,"context_line":"        self.assertEqual(console[\u0027internal_access_path\u0027],"},{"line_number":1721,"context_line":"                         expected[\u0027internal_access_path\u0027])"},{"line_number":1722,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAOH%2F%2F6PQ%3D","line":1719,"updated":"2013-03-21 10:02:11.000000000","message":"not sure you need this, but I don\u0027t mind.","commit_id":"60151ba0d239bc86de3f46b3b6b47e9e71908709"},{"author":{"_account_id":4419,"name":"Matthew Sherborne","email":"msherborne+openstack@gmail.com","username":"msherborne+openstack"},"change_message_id":"78ea45d96a46eecaf6f3cd976de4021f3c3794be","unresolved":false,"context_lines":[{"line_number":1716,"context_line":"        console \u003d self.compute.get_vnc_console(self.context, \u0027xvpvnc\u0027,"},{"line_number":1717,"context_line":"                                               instance\u003dinstance)"},{"line_number":1718,"context_line":""},{"line_number":1719,"context_line":"        self.assert_(console)"},{"line_number":1720,"context_line":"        self.assertEqual(console[\u0027internal_access_path\u0027],"},{"line_number":1721,"context_line":"                         expected[\u0027internal_access_path\u0027])"},{"line_number":1722,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAOH%2F%2F6N0%3D","line":1719,"in_reply_to":"AAAAOH%2F%2F6PQ%3D","updated":"2013-03-21 10:12:02.000000000","message":"I think it\u0027s good to check it, it\u0027ll catch if someone removes the return statement in get_vnc_console","commit_id":"60151ba0d239bc86de3f46b3b6b47e9e71908709"},{"author":{"_account_id":4419,"name":"Matthew Sherborne","email":"msherborne+openstack@gmail.com","username":"msherborne+openstack"},"change_message_id":"78ea45d96a46eecaf6f3cd976de4021f3c3794be","unresolved":false,"context_lines":[{"line_number":1717,"context_line":"                                               instance\u003dinstance)"},{"line_number":1718,"context_line":""},{"line_number":1719,"context_line":"        self.assert_(console)"},{"line_number":1720,"context_line":"        self.assertEqual(console[\u0027internal_access_path\u0027],"},{"line_number":1721,"context_line":"                         expected[\u0027internal_access_path\u0027])"},{"line_number":1722,"context_line":""},{"line_number":1723,"context_line":"        def fake_xenapi_driver_rescue_not_ready(*args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAOH%2F%2F6OA%3D","line":1720,"updated":"2013-03-21 10:12:02.000000000","message":"expected should go as the first param, because if they\u0027re different and non-simple types, the output calls the first one \u0027expected\u0027 and the 2nd actual.","commit_id":"60151ba0d239bc86de3f46b3b6b47e9e71908709"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"12b0733c69d370d10e08f9006c07c9c8f76c1804","unresolved":false,"context_lines":[{"line_number":1720,"context_line":"        self.assertEqual(console[\u0027internal_access_path\u0027],"},{"line_number":1721,"context_line":"                         expected[\u0027internal_access_path\u0027])"},{"line_number":1722,"context_line":""},{"line_number":1723,"context_line":"        def fake_xenapi_driver_rescue_not_ready(*args, **kwargs):"},{"line_number":1724,"context_line":"            #Case where rescue instance is not there yet"},{"line_number":1725,"context_line":"            if instance[\u0027vm_state\u0027] \u003d\u003d vm_states.RESCUED:"},{"line_number":1726,"context_line":"                raise exception.InstanceNotFound(instance_id\u003dinstance[\u0027uuid\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAOH%2F%2F6PY%3D","line":1723,"updated":"2013-03-21 10:02:11.000000000","message":"from here down should be a separate test, one for each call to get_vnc_console. I know that could involves a bit of duplication, but I would create an \"_enable_vnc\" helper method.\n\nMuch easier to see what you are testing and spot what fails if you do this.","commit_id":"60151ba0d239bc86de3f46b3b6b47e9e71908709"},{"author":{"_account_id":4419,"name":"Matthew Sherborne","email":"msherborne+openstack@gmail.com","username":"msherborne+openstack"},"change_message_id":"78ea45d96a46eecaf6f3cd976de4021f3c3794be","unresolved":false,"context_lines":[{"line_number":1721,"context_line":"                         expected[\u0027internal_access_path\u0027])"},{"line_number":1722,"context_line":""},{"line_number":1723,"context_line":"        def fake_xenapi_driver_rescue_not_ready(*args, **kwargs):"},{"line_number":1724,"context_line":"            #Case where rescue instance is not there yet"},{"line_number":1725,"context_line":"            if instance[\u0027vm_state\u0027] \u003d\u003d vm_states.RESCUED:"},{"line_number":1726,"context_line":"                raise exception.InstanceNotFound(instance_id\u003dinstance[\u0027uuid\u0027])"},{"line_number":1727,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAOH%2F%2F6Nw%3D","line":1724,"updated":"2013-03-21 10:12:02.000000000","message":"space please:\n\n# Case where rescue instance is not there yet","commit_id":"60151ba0d239bc86de3f46b3b6b47e9e71908709"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"12b0733c69d370d10e08f9006c07c9c8f76c1804","unresolved":false,"context_lines":[{"line_number":1722,"context_line":""},{"line_number":1723,"context_line":"        def fake_xenapi_driver_rescue_not_ready(*args, **kwargs):"},{"line_number":1724,"context_line":"            #Case where rescue instance is not there yet"},{"line_number":1725,"context_line":"            if instance[\u0027vm_state\u0027] \u003d\u003d vm_states.RESCUED:"},{"line_number":1726,"context_line":"                raise exception.InstanceNotFound(instance_id\u003dinstance[\u0027uuid\u0027])"},{"line_number":1727,"context_line":""},{"line_number":1728,"context_line":"        self.stubs.Set(self.compute.driver, \"get_vnc_console\","}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAOH%2F%2F6PA%3D","line":1725,"updated":"2013-03-21 10:02:11.000000000","message":"this is basically \"if true:\" so I would remove it.","commit_id":"60151ba0d239bc86de3f46b3b6b47e9e71908709"}],"nova/tests/test_xenapi.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"724b31dbc7b48f299f5e8fd4193952eb5c46fef2","unresolved":false,"context_lines":[{"line_number":425,"context_line":"                fake.FakeVirtAPI())"},{"line_number":426,"context_line":"        conn \u003d xenapi_conn.XenAPIDriver(fake.FakeVirtAPI(), False)"},{"line_number":427,"context_line":"        vm_ref \u003d vm_utils.lookup(session, instance[\u0027name\u0027])"},{"line_number":428,"context_line":"        expected_path \u003d \"/console?ref\u003d%s\" % str(vm_ref)"},{"line_number":429,"context_line":""},{"line_number":430,"context_line":"        console \u003d conn.get_vnc_console(instance)"},{"line_number":431,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOH%2F%2F4ZA%3D","line":428,"updated":"2013-03-22 11:20:25.000000000","message":"you have the session object above, why not add in the session id? For readability, I would move it just above the assert statement below.","commit_id":"5185e61a163de1bb413db61facc461d47da5c0a4"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"614cf5f0b9118e51125e06347b221f654b9d76a9","unresolved":false,"context_lines":[{"line_number":425,"context_line":"                fake.FakeVirtAPI())"},{"line_number":426,"context_line":"        conn \u003d xenapi_conn.XenAPIDriver(fake.FakeVirtAPI(), False)"},{"line_number":427,"context_line":"        vm_ref \u003d vm_utils.lookup(session, instance[\u0027name\u0027])"},{"line_number":428,"context_line":"        expected_path \u003d \"/console?ref\u003d%s\" % str(vm_ref)"},{"line_number":429,"context_line":""},{"line_number":430,"context_line":"        console \u003d conn.get_vnc_console(instance)"},{"line_number":431,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOH%2F%2F4Tw%3D","line":428,"in_reply_to":"AAAAOH%2F%2F4ZA%3D","updated":"2013-03-22 12:23:01.000000000","message":"Intentionally skipped as it is not needed. I am stripping out  session-id from console return as well because they would always be different. Basically just testing that vm_ref is correct.","commit_id":"5185e61a163de1bb413db61facc461d47da5c0a4"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"724b31dbc7b48f299f5e8fd4193952eb5c46fef2","unresolved":false,"context_lines":[{"line_number":444,"context_line":"                                          \u0027Running\u0027)"},{"line_number":445,"context_line":"        # Set instance state to rescued"},{"line_number":446,"context_line":"        instance[\u0027vm_state\u0027] \u003d \u0027rescued\u0027"},{"line_number":447,"context_line":"        vm_ref \u003d vm_utils.lookup(session, instance[\u0027name\u0027] + \u0027-rescue\u0027)"},{"line_number":448,"context_line":"        expected_path \u003d \"/console?ref\u003d%s\" % str(vm_ref)"},{"line_number":449,"context_line":""},{"line_number":450,"context_line":"        console \u003d conn.get_vnc_console(instance)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOH%2F%2F4Y4%3D","line":447,"updated":"2013-03-22 11:20:25.000000000","message":"isn\u0027t rescue_vm the vm_ref you need?","commit_id":"5185e61a163de1bb413db61facc461d47da5c0a4"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"614cf5f0b9118e51125e06347b221f654b9d76a9","unresolved":false,"context_lines":[{"line_number":444,"context_line":"                                          \u0027Running\u0027)"},{"line_number":445,"context_line":"        # Set instance state to rescued"},{"line_number":446,"context_line":"        instance[\u0027vm_state\u0027] \u003d \u0027rescued\u0027"},{"line_number":447,"context_line":"        vm_ref \u003d vm_utils.lookup(session, instance[\u0027name\u0027] + \u0027-rescue\u0027)"},{"line_number":448,"context_line":"        expected_path \u003d \"/console?ref\u003d%s\" % str(vm_ref)"},{"line_number":449,"context_line":""},{"line_number":450,"context_line":"        console \u003d conn.get_vnc_console(instance)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOH%2F%2F4Ts%3D","line":447,"in_reply_to":"AAAAOH%2F%2F4Y4%3D","updated":"2013-03-22 12:23:01.000000000","message":"Yep, thanks for spotting that.","commit_id":"5185e61a163de1bb413db61facc461d47da5c0a4"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"724b31dbc7b48f299f5e8fd4193952eb5c46fef2","unresolved":false,"context_lines":[{"line_number":445,"context_line":"        # Set instance state to rescued"},{"line_number":446,"context_line":"        instance[\u0027vm_state\u0027] \u003d \u0027rescued\u0027"},{"line_number":447,"context_line":"        vm_ref \u003d vm_utils.lookup(session, instance[\u0027name\u0027] + \u0027-rescue\u0027)"},{"line_number":448,"context_line":"        expected_path \u003d \"/console?ref\u003d%s\" % str(vm_ref)"},{"line_number":449,"context_line":""},{"line_number":450,"context_line":"        console \u003d conn.get_vnc_console(instance)"},{"line_number":451,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOH%2F%2F4Ys%3D","line":448,"updated":"2013-03-22 11:20:25.000000000","message":"I would be tempted to share the code to generate this path with the above test in a helper method.","commit_id":"5185e61a163de1bb413db61facc461d47da5c0a4"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"724b31dbc7b48f299f5e8fd4193952eb5c46fef2","unresolved":false,"context_lines":[{"line_number":454,"context_line":"        actual_path \u003d console[\u0027internal_access_path\u0027].split(\u0027\u0026\u0027)[0]"},{"line_number":455,"context_line":""},{"line_number":456,"context_line":"        self.assertEqual(expected_path, actual_path)"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"    def test_instance_snapshot_fails_with_no_primary_vdi(self):"},{"line_number":459,"context_line":""},{"line_number":460,"context_line":"        def create_bad_vbd(session, vm_ref, vdi_ref, userdevice,"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOH%2F%2F4Yw%3D","line":457,"updated":"2013-03-22 11:20:25.000000000","message":"There should be two more tests for those exceptional cases.","commit_id":"5185e61a163de1bb413db61facc461d47da5c0a4"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"614cf5f0b9118e51125e06347b221f654b9d76a9","unresolved":false,"context_lines":[{"line_number":454,"context_line":"        actual_path \u003d console[\u0027internal_access_path\u0027].split(\u0027\u0026\u0027)[0]"},{"line_number":455,"context_line":""},{"line_number":456,"context_line":"        self.assertEqual(expected_path, actual_path)"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"    def test_instance_snapshot_fails_with_no_primary_vdi(self):"},{"line_number":459,"context_line":""},{"line_number":460,"context_line":"        def create_bad_vbd(session, vm_ref, vdi_ref, userdevice,"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOH%2F%2F4To%3D","line":457,"in_reply_to":"AAAAOH%2F%2F4Yw%3D","updated":"2013-03-22 12:23:01.000000000","message":"I truly apologize for my sloppiness on this one, should have taken care of this.","commit_id":"5185e61a163de1bb413db61facc461d47da5c0a4"},{"author":{"_account_id":4419,"name":"Matthew Sherborne","email":"msherborne+openstack@gmail.com","username":"msherborne+openstack"},"change_message_id":"e26a9d4107a813680166687da0c201a590d1b90e","unresolved":false,"context_lines":[{"line_number":422,"context_line":"    def test_get_vnc_console(self):"},{"line_number":423,"context_line":"        instance \u003d self._create_instance()"},{"line_number":424,"context_line":"        session \u003d xenapi_conn.XenAPISession(\u0027test_url\u0027, \u0027root\u0027, \u0027test_pass\u0027,"},{"line_number":425,"context_line":"                fake.FakeVirtAPI())"},{"line_number":426,"context_line":"        conn \u003d xenapi_conn.XenAPIDriver(fake.FakeVirtAPI(), False)"},{"line_number":427,"context_line":"        vm_ref \u003d vm_utils.lookup(session, instance[\u0027name\u0027])"},{"line_number":428,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAOH%2F%2F3JM%3D","line":425,"updated":"2013-03-23 03:16:58.000000000","message":"Probably could have the same indentation as the one in the func below. Like up to the opening bracket of the line above.","commit_id":"e136de1aea8d1469465272942fa3d2769cbe3a80"}],"nova/virt/xenapi/vmops.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"70e552998e07c430c4289c71719afdb0cce11656","unresolved":false,"context_lines":[{"line_number":1365,"context_line":""},{"line_number":1366,"context_line":"    def get_vnc_console(self, instance):"},{"line_number":1367,"context_line":"        \"\"\"Return connection info for a vnc console.\"\"\""},{"line_number":1368,"context_line":"        if instance[\u0027vm_state\u0027] \u003d\u003d vm_state.RESCUED:"},{"line_number":1369,"context_line":"            instance[\u0027name\u0027] \u003d instance[\u0027name\u0027] + \u0027-rescue\u0027"},{"line_number":1370,"context_line":""},{"line_number":1371,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAOH%2F%2F73s%3D","line":1368,"updated":"2013-03-20 10:32:40.000000000","message":"Missing:\nfrom nova.compute import vm_states","commit_id":"8d9dc1326272a285c2722dc4070dbad6627d7c69"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"988c7463e0eebf6a058d5755fd53bb3516fd5571","unresolved":false,"context_lines":[{"line_number":1365,"context_line":""},{"line_number":1366,"context_line":"    def get_vnc_console(self, instance):"},{"line_number":1367,"context_line":"        \"\"\"Return connection info for a vnc console.\"\"\""},{"line_number":1368,"context_line":"        if instance[\u0027vm_state\u0027] \u003d\u003d vm_state.RESCUED:"},{"line_number":1369,"context_line":"            instance[\u0027name\u0027] \u003d instance[\u0027name\u0027] + \u0027-rescue\u0027"},{"line_number":1370,"context_line":""},{"line_number":1371,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAOH%2F%2F7fA%3D","line":1368,"in_reply_to":"AAAAOH%2F%2F73s%3D","updated":"2013-03-20 16:09:36.000000000","message":"yes, my bad :), will be corrected in the next patch.","commit_id":"8d9dc1326272a285c2722dc4070dbad6627d7c69"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"70e552998e07c430c4289c71719afdb0cce11656","unresolved":false,"context_lines":[{"line_number":1366,"context_line":"    def get_vnc_console(self, instance):"},{"line_number":1367,"context_line":"        \"\"\"Return connection info for a vnc console.\"\"\""},{"line_number":1368,"context_line":"        if instance[\u0027vm_state\u0027] \u003d\u003d vm_state.RESCUED:"},{"line_number":1369,"context_line":"            instance[\u0027name\u0027] \u003d instance[\u0027name\u0027] + \u0027-rescue\u0027"},{"line_number":1370,"context_line":""},{"line_number":1371,"context_line":"        try:"},{"line_number":1372,"context_line":"            vm_ref \u003d self._get_vm_opaque_ref(instance)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAOH%2F%2F73g%3D","line":1369,"updated":"2013-03-20 10:32:40.000000000","message":"It would be better to have an instance_name variable rather than changing this copy of the DB data.\n\nThis change might affect other code outside this function. It probably doesn\u0027t in this case, but it looks bad.","commit_id":"8d9dc1326272a285c2722dc4070dbad6627d7c69"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"988c7463e0eebf6a058d5755fd53bb3516fd5571","unresolved":false,"context_lines":[{"line_number":1366,"context_line":"    def get_vnc_console(self, instance):"},{"line_number":1367,"context_line":"        \"\"\"Return connection info for a vnc console.\"\"\""},{"line_number":1368,"context_line":"        if instance[\u0027vm_state\u0027] \u003d\u003d vm_state.RESCUED:"},{"line_number":1369,"context_line":"            instance[\u0027name\u0027] \u003d instance[\u0027name\u0027] + \u0027-rescue\u0027"},{"line_number":1370,"context_line":""},{"line_number":1371,"context_line":"        try:"},{"line_number":1372,"context_line":"            vm_ref \u003d self._get_vm_opaque_ref(instance)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAOH%2F%2F7e8%3D","line":1369,"in_reply_to":"AAAAOH%2F%2F73g%3D","updated":"2013-03-20 16:09:36.000000000","message":"will need to pass instance to get vm opqref so we can either do what you suggested below can call vm_utils straight or change the name in the dict like here.","commit_id":"8d9dc1326272a285c2722dc4070dbad6627d7c69"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"70e552998e07c430c4289c71719afdb0cce11656","unresolved":false,"context_lines":[{"line_number":1369,"context_line":"            instance[\u0027name\u0027] \u003d instance[\u0027name\u0027] + \u0027-rescue\u0027"},{"line_number":1370,"context_line":""},{"line_number":1371,"context_line":"        try:"},{"line_number":1372,"context_line":"            vm_ref \u003d self._get_vm_opaque_ref(instance)"},{"line_number":1373,"context_line":"        except exception.NotFound:"},{"line_number":1374,"context_line":"            # The compute manager expects InstanceNotFound for this case."},{"line_number":1375,"context_line":"            raise exception.InstanceNotFound(instance_id\u003dinstance[\u0027uuid\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAOH%2F%2F73I%3D","line":1372,"updated":"2013-03-20 10:32:40.000000000","message":"Given this different exception, and the messiness of the above, maybe it would be better to call vm_utils directly and deal with the null here?","commit_id":"8d9dc1326272a285c2722dc4070dbad6627d7c69"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"988c7463e0eebf6a058d5755fd53bb3516fd5571","unresolved":false,"context_lines":[{"line_number":1369,"context_line":"            instance[\u0027name\u0027] \u003d instance[\u0027name\u0027] + \u0027-rescue\u0027"},{"line_number":1370,"context_line":""},{"line_number":1371,"context_line":"        try:"},{"line_number":1372,"context_line":"            vm_ref \u003d self._get_vm_opaque_ref(instance)"},{"line_number":1373,"context_line":"        except exception.NotFound:"},{"line_number":1374,"context_line":"            # The compute manager expects InstanceNotFound for this case."},{"line_number":1375,"context_line":"            raise exception.InstanceNotFound(instance_id\u003dinstance[\u0027uuid\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAOH%2F%2F7es%3D","line":1372,"in_reply_to":"AAAAOH%2F%2F73I%3D","updated":"2013-03-20 16:09:36.000000000","message":"Yeah i thought about that, but this seemed straight forward. If you call the vm_utils.lookup then we\u0027d have to raise NotFound for non existent instance and incorporate that also. But if you feel that is a clear better way of handling this iam happy to do so.","commit_id":"8d9dc1326272a285c2722dc4070dbad6627d7c69"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"12b0733c69d370d10e08f9006c07c9c8f76c1804","unresolved":false,"context_lines":[{"line_number":1366,"context_line":""},{"line_number":1367,"context_line":"    def get_vnc_console(self, instance):"},{"line_number":1368,"context_line":"        \"\"\"Return connection info for a vnc console.\"\"\""},{"line_number":1369,"context_line":"        if instance[\u0027vm_state\u0027] \u003d\u003d vm_states.RESCUED:"},{"line_number":1370,"context_line":"            instance[\u0027name\u0027] \u003d instance[\u0027name\u0027] + \u0027-rescue\u0027"},{"line_number":1371,"context_line":""},{"line_number":1372,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAOH%2F%2F6PI%3D","line":1369,"updated":"2013-03-21 10:02:11.000000000","message":"I still don\u0027t see any tests to cover this if statement.\n\nI would add a test for this in test_xenapi.py, hopefully near existing tests that cover this method.","commit_id":"60151ba0d239bc86de3f46b3b6b47e9e71908709"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"12b0733c69d370d10e08f9006c07c9c8f76c1804","unresolved":false,"context_lines":[{"line_number":1367,"context_line":"    def get_vnc_console(self, instance):"},{"line_number":1368,"context_line":"        \"\"\"Return connection info for a vnc console.\"\"\""},{"line_number":1369,"context_line":"        if instance[\u0027vm_state\u0027] \u003d\u003d vm_states.RESCUED:"},{"line_number":1370,"context_line":"            instance[\u0027name\u0027] \u003d instance[\u0027name\u0027] + \u0027-rescue\u0027"},{"line_number":1371,"context_line":""},{"line_number":1372,"context_line":"        try:"},{"line_number":1373,"context_line":"            vm_ref \u003d self._get_vm_opaque_ref(instance)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAOH%2F%2F6O4%3D","line":1370,"updated":"2013-03-21 10:02:11.000000000","message":"My issue with this statement is that your change is visible outside of this method. I could live with it, but I don\u0027t like it.\n\nAs mentioned before, I would change the try except below to explicitly call vm_utils function that is called inside _get_vm_opaque_ref passing the name and handling the null case below.","commit_id":"60151ba0d239bc86de3f46b3b6b47e9e71908709"},{"author":{"_account_id":5652,"name":"David Ripton","email":"dripton@redhat.com","username":"dripton"},"change_message_id":"9602e53e5f763745fcf7411baa031cfc0950734b","unresolved":false,"context_lines":[{"line_number":1367,"context_line":"    def get_vnc_console(self, instance):"},{"line_number":1368,"context_line":"        \"\"\"Return connection info for a vnc console.\"\"\""},{"line_number":1369,"context_line":"        if instance[\u0027vm_state\u0027] \u003d\u003d vm_states.RESCUED:"},{"line_number":1370,"context_line":"            instance[\u0027name\u0027] \u003d instance[\u0027name\u0027] + \u0027-rescue\u0027"},{"line_number":1371,"context_line":""},{"line_number":1372,"context_line":"        try:"},{"line_number":1373,"context_line":"            vm_ref \u003d self._get_vm_opaque_ref(instance)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAOH%2F%2F7X0%3D","line":1370,"updated":"2013-03-20 17:31:25.000000000","message":"instance[\u0027name\u0027] +\u003d \u0027-rescue\u0027 is a bit shorter and cleaner","commit_id":"60151ba0d239bc86de3f46b3b6b47e9e71908709"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"724b31dbc7b48f299f5e8fd4193952eb5c46fef2","unresolved":false,"context_lines":[{"line_number":1371,"context_line":"            vm_ref \u003d vm_utils.lookup(self._session, name)"},{"line_number":1372,"context_line":"            if vm_ref is None:"},{"line_number":1373,"context_line":"                # The rescue instance might not be ready at this point"},{"line_number":1374,"context_line":"                raise exception.InstanceNotReady(instance_id\u003dinstance[\u0027uuid\u0027])"},{"line_number":1375,"context_line":"        else:"},{"line_number":1376,"context_line":"            try:"},{"line_number":1377,"context_line":"                vm_ref \u003d self._get_vm_opaque_ref(instance)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOH%2F%2F4ZU%3D","line":1374,"updated":"2013-03-22 11:20:25.000000000","message":"Need to test the correct exceptions are raised.","commit_id":"5185e61a163de1bb413db61facc461d47da5c0a4"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"614cf5f0b9118e51125e06347b221f654b9d76a9","unresolved":false,"context_lines":[{"line_number":1371,"context_line":"            vm_ref \u003d vm_utils.lookup(self._session, name)"},{"line_number":1372,"context_line":"            if vm_ref is None:"},{"line_number":1373,"context_line":"                # The rescue instance might not be ready at this point"},{"line_number":1374,"context_line":"                raise exception.InstanceNotReady(instance_id\u003dinstance[\u0027uuid\u0027])"},{"line_number":1375,"context_line":"        else:"},{"line_number":1376,"context_line":"            try:"},{"line_number":1377,"context_line":"                vm_ref \u003d self._get_vm_opaque_ref(instance)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOH%2F%2F4Tk%3D","line":1374,"in_reply_to":"AAAAOH%2F%2F4ZU%3D","updated":"2013-03-22 12:23:01.000000000","message":"coming up .. thanks agin","commit_id":"5185e61a163de1bb413db61facc461d47da5c0a4"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"724b31dbc7b48f299f5e8fd4193952eb5c46fef2","unresolved":false,"context_lines":[{"line_number":1374,"context_line":"                raise exception.InstanceNotReady(instance_id\u003dinstance[\u0027uuid\u0027])"},{"line_number":1375,"context_line":"        else:"},{"line_number":1376,"context_line":"            try:"},{"line_number":1377,"context_line":"                vm_ref \u003d self._get_vm_opaque_ref(instance)"},{"line_number":1378,"context_line":"            except exception.NotFound:"},{"line_number":1379,"context_line":"                # The compute manager expects InstanceNotFound for this case."},{"line_number":1380,"context_line":"                raise exception.InstanceNotFound(instance_id\u003dinstance[\u0027uuid\u0027])"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAOH%2F%2F4ZI%3D","line":1377,"updated":"2013-03-22 11:20:25.000000000","message":"I would be tempted to share the code with the above logic, its an odd use of _get_vm_opaque_ref anyways.","commit_id":"5185e61a163de1bb413db61facc461d47da5c0a4"}]}