)]}'
{"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"346747166a32662a48432f772f2e5673e59e6b65","unresolved":false,"context_lines":[{"line_number":10828,"context_line":"        with test.nested("},{"line_number":10829,"context_line":"                mock.patch.object(drvr, \u0027_get_console_log_path\u0027),"},{"line_number":10830,"context_line":"                mock.patch.object(fake_libvirt_utils, \u0027file_open\u0027,"},{"line_number":10831,"context_line":"                                  side_effect\u003dIOError)"},{"line_number":10832,"context_line":"            ) as (mock_path, mock_open):"},{"line_number":10833,"context_line":"            drvr._ensure_console_log_for_instance(mock.ANY)"},{"line_number":10834,"context_line":"            mock_path.assert_called_once()"}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_de1e286f","line":10831,"updated":"2017-01-23 11:11:30.000000000","message":"Again, if you wanted to gold-plate this, you raise an IOError with errno set to EPERM.","commit_id":"0ff2960257af265cd6a46fa73bcf4a8cab2d0254"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"64dad62ef797d151d6a1ec68d46e4f0aea9f34c4","unresolved":false,"context_lines":[{"line_number":2865,"context_line":"            libvirt_utils.file_open(console_file, \u0027a\u0027).close()"},{"line_number":2866,"context_line":"        # NOTE(sfinucan): We can safely ignore permission issues here and"},{"line_number":2867,"context_line":"        # assume that it is libvirt that has taken ownership of this file."},{"line_number":2868,"context_line":"        except IOError:"},{"line_number":2869,"context_line":"            pass"},{"line_number":2870,"context_line":""},{"line_number":2871,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5da102_35f823c3","line":2868,"updated":"2016-11-02 13:17:25.000000000","message":"Should we log a warning here ?","commit_id":"7063d143bab0ef8cec80b798e555889e3c6cde14"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"bbb1abb979d38a7d2c7cc106126da4ac4718f299","unresolved":false,"context_lines":[{"line_number":2865,"context_line":"            libvirt_utils.file_open(console_file, \u0027a\u0027).close()"},{"line_number":2866,"context_line":"        # NOTE(sfinucan): We can safely ignore permission issues here and"},{"line_number":2867,"context_line":"        # assume that it is libvirt that has taken ownership of this file."},{"line_number":2868,"context_line":"        except IOError:"},{"line_number":2869,"context_line":"            pass"},{"line_number":2870,"context_line":""},{"line_number":2871,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5da102_b6499a62","line":2868,"in_reply_to":"ba5da102_35f823c3","updated":"2016-11-02 13:49:48.000000000","message":"Not a bad idea","commit_id":"7063d143bab0ef8cec80b798e555889e3c6cde14"},{"author":{"_account_id":20191,"name":"Dinesh Bhor","email":"bhordinesh07@gmail.com","username":"dineshbhor"},"change_message_id":"cdeadaea3733fe04a0f66a6de235fea83b0c7c9b","unresolved":false,"context_lines":[{"line_number":2866,"context_line":"        # NOTE(sfinucan): We can safely ignore permission issues here and"},{"line_number":2867,"context_line":"        # assume that it is libvirt that has taken ownership of this file."},{"line_number":2868,"context_line":"        except IOError:"},{"line_number":2869,"context_line":"            LOG.warning(\u0027Failed to create console file: %s.\u0027, console_file)"},{"line_number":2870,"context_line":"            pass"},{"line_number":2871,"context_line":""},{"line_number":2872,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_223448fa","line":2869,"range":{"start_line":2869,"start_character":24,"end_line":2869,"end_character":58},"updated":"2016-11-03 06:43:05.000000000","message":"Pep8 is failing because of this:\n\"N330  LOG.warning, LOG.warn messages require translations `_LW()`.\"","commit_id":"0a3d3a6bd8c6b51fbdd7689177da66d4a253ccee"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"b105ff2603317f7e7c8b6cde490836e7de4a5d98","unresolved":false,"context_lines":[{"line_number":2867,"context_line":"        # assume that it is libvirt that has taken ownership of this file."},{"line_number":2868,"context_line":"        except IOError:"},{"line_number":2869,"context_line":"            LOG.warning(\u0027Failed to create console file: %s.\u0027, console_file)"},{"line_number":2870,"context_line":"            pass"},{"line_number":2871,"context_line":""},{"line_number":2872,"context_line":"    @staticmethod"},{"line_number":2873,"context_line":"    def _get_disk_config_path(instance, suffix\u003d\u0027\u0027):"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5da102_6298006b","line":2870,"range":{"start_line":2870,"start_character":12,"end_line":2870,"end_character":16},"updated":"2016-11-03 06:24:49.000000000","message":"no need to add pass statement","commit_id":"0a3d3a6bd8c6b51fbdd7689177da66d4a253ccee"},{"author":{"_account_id":16282,"name":"YaZug","email":"jschluet@redhat.com","username":"yazug"},"change_message_id":"ed62de02e2b69178928952766c8b40fdfb8fe042","unresolved":false,"context_lines":[{"line_number":2862,"context_line":"        LOG.debug(\u0027Ensure instance console log exists: %s\u0027, console_file,"},{"line_number":2863,"context_line":"                  instance\u003dinstance)"},{"line_number":2864,"context_line":"        try:"},{"line_number":2865,"context_line":"            libvirt_utils.file_open(console_file, \u0027a\u0027).close()"},{"line_number":2866,"context_line":"        # NOTE(sfinucan): We can safely ignore permission issues here and"},{"line_number":2867,"context_line":"        # assume that it is libvirt that has taken ownership of this file."},{"line_number":2868,"context_line":"        except IOError:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_de051b14","line":2865,"updated":"2017-01-06 16:52:00.000000000","message":"might be able to wrap a unittest around direct calling into _ensure_console_log_for_instance() ... with a mocked out libvirt_utils.file_open() which throws and IOException.  \n\nIf you want to try to go higher level and can repeatably trigger IOException on libvirt_utils.file_open() then you could mockk out self._get_console_log_path()","commit_id":"5c9c159f18de0fcddbfcb35734dbb14f557df6b6"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"769e2d877962775ccf0db49d7c3b70c6ddaa2565","unresolved":false,"context_lines":[{"line_number":2862,"context_line":"        LOG.debug(\u0027Ensure instance console log exists: %s\u0027, console_file,"},{"line_number":2863,"context_line":"                  instance\u003dinstance)"},{"line_number":2864,"context_line":"        try:"},{"line_number":2865,"context_line":"            libvirt_utils.file_open(console_file, \u0027a\u0027).close()"},{"line_number":2866,"context_line":"        # NOTE(sfinucan): We can safely ignore permission issues here and"},{"line_number":2867,"context_line":"        # assume that it is libvirt that has taken ownership of this file."},{"line_number":2868,"context_line":"        except IOError:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba5201f7_c411cc53","line":2865,"in_reply_to":"ba5201f7_de051b14","updated":"2017-01-06 17:06:18.000000000","message":"Done","commit_id":"5c9c159f18de0fcddbfcb35734dbb14f557df6b6"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"346747166a32662a48432f772f2e5673e59e6b65","unresolved":false,"context_lines":[{"line_number":2930,"context_line":"            libvirt_utils.file_open(console_file, \u0027a\u0027).close()"},{"line_number":2931,"context_line":"        # NOTE(sfinucan): We can safely ignore permission issues here and"},{"line_number":2932,"context_line":"        # assume that it is libvirt that has taken ownership of this file."},{"line_number":2933,"context_line":"        except IOError:"},{"line_number":2934,"context_line":"            LOG.warning(_LW(\u0027Failed to create console file: %s.\u0027),"},{"line_number":2935,"context_line":"                        console_file)"},{"line_number":2936,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5a3905b3_fe412464","line":2933,"updated":"2017-01-23 11:11:30.000000000","message":"Ideally this would be a bit more specific. Check for EPERM, maybe, and re-raise anything else?\n\nThat said, this is well commented and any other source of IO error would soon be apparent from other errors, so I\u0027d call this \u0027nice to have\u0027.","commit_id":"0ff2960257af265cd6a46fa73bcf4a8cab2d0254"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"9fd0ffb19ddaa079ed890de34c23c3c5efc01697","unresolved":false,"context_lines":[{"line_number":2931,"context_line":"        # NOTE(sfinucan): We can safely ignore permission issues here and"},{"line_number":2932,"context_line":"        # assume that it is libvirt that has taken ownership of this file."},{"line_number":2933,"context_line":"        except IOError:"},{"line_number":2934,"context_line":"            LOG.warning(_LW(\u0027Failed to create console file: %s.\u0027),"},{"line_number":2935,"context_line":"                        console_file)"},{"line_number":2936,"context_line":""},{"line_number":2937,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a461143_41aed281","line":2934,"updated":"2017-01-30 18:46:00.000000000","message":"I don\u0027t like this log warning because it\u0027s either:\n\n* Irrelevant - console.log exists with qemu user permissions and we can\u0027t append to it, which is fine\n\n* Actually important - for whatever reason we can\u0027t create the file, and so libvirt won\u0027t be able to either\n\nWhat would be the best way to log this only if relevant? Check that the file exists before attempting to open it in append mode?","commit_id":"0ff2960257af265cd6a46fa73bcf4a8cab2d0254"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"7f077ee90843113d74140ffd0dacedc85d04caf2","unresolved":false,"context_lines":[{"line_number":2931,"context_line":"        # NOTE(sfinucan): We can safely ignore permission issues here and"},{"line_number":2932,"context_line":"        # assume that it is libvirt that has taken ownership of this file."},{"line_number":2933,"context_line":"        except IOError:"},{"line_number":2934,"context_line":"            LOG.warning(_LW(\u0027Failed to create console file: %s.\u0027),"},{"line_number":2935,"context_line":"                        console_file)"},{"line_number":2936,"context_line":""},{"line_number":2937,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a461143_f48cc5db","line":2934,"in_reply_to":"3a461143_41aed281","updated":"2017-01-31 10:26:50.000000000","message":"See above: check for EPERM. No need for 2 IO operations as 1 already gives you all you need (and there\u0027s no race between them).\n\n  except IOError as ex:\n    if ex.errno !\u003d errno.EPERM:\n      raise\n\nAs I said above, I\u0027d prefer to see us checking for EPERM here. That said, if there\u0027s some other error qemu will explode later in a relatively obvious manner. This will have a similar effect to raising here, so despite that I still think this code is:\n\n1. An improvement.\n2. Not a maintenance issue.\n\nI wouldn\u0027t block it myself (but please update if you have time).","commit_id":"0ff2960257af265cd6a46fa73bcf4a8cab2d0254"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a6931245030226082362b72d4d79a7adb8fab813","unresolved":false,"context_lines":[{"line_number":2944,"context_line":"        LOG.debug(\u0027Ensure instance console log exists: %s\u0027, console_file,"},{"line_number":2945,"context_line":"                  instance\u003dinstance)"},{"line_number":2946,"context_line":"        try:"},{"line_number":2947,"context_line":"            libvirt_utils.file_open(console_file, \u0027a\u0027).close()"},{"line_number":2948,"context_line":"        # NOTE(sfinucan): We can safely ignore permission issues here and"},{"line_number":2949,"context_line":"        # assume that it is libvirt that has taken ownership of this file."},{"line_number":2950,"context_line":"        except IOError as ex:"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a461143_d256c4cd","line":2947,"updated":"2017-02-01 13:32:12.000000000","message":"An alternative approach could have been to synchronously lookup at the console file and then opening it if not there, right?\n\nBut meh, fine by me.","commit_id":"1848860fd4fb6dfb2ead1f8271050d36df592205"}]}