)]}'
{"oslo_utils/imageutils/format_inspector.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"afe36ae49cb03500c8eeae6436767b3c26e8204e","unresolved":true,"context_lines":[{"line_number":165,"context_line":"    def __init__(self, failures):"},{"line_number":166,"context_line":"        super().__init__(_(\u0027Safety checks failed: %s\u0027) % \u0027,\u0027.join("},{"line_number":167,"context_line":"            name for (name, _) in failures))"},{"line_number":168,"context_line":"        self.failures \u003d failures"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"class FileInspector(abc.ABC):"}],"source_content_type":"text/x-python","patch_set":1,"id":"982c3dbe_419ff678","line":168,"updated":"2024-07-31 14:52:59.000000000","message":"We need to keep the reason here, which is my point. We need to know more than just \"the backing_file check\" failed, we need to know why. The VMDK descriptor check is very complex and has multiple possible outcomes, and the reason why something failed matters in some of the situations we\u0027re trying to account for here. Splitting these out into multiple checks helps a little (for some callers, failing the backing_file check may be all they need to know) but not enough, IMHO.\n\nI really don\u0027t understand why we can\u0027t include the check object themselves (which could later be subclassed to provide more information than just a string reason, which would be material to the judgment about whether or not the failure is fatal or not). They\u0027re created per-inspector so it\u0027s not like there\u0027s a reentrancy concern there.\n\nBut, if you feel that strongly, I\u0027ll at least ask that we persist the full set here (names and reasons).","commit_id":"0ff708d2d1e67a00646e638e6e2b592090637246"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3dd691a915ef8dd9f3c3d05349f42a3597821926","unresolved":true,"context_lines":[{"line_number":165,"context_line":"    def __init__(self, failures):"},{"line_number":166,"context_line":"        super().__init__(_(\u0027Safety checks failed: %s\u0027) % \u0027,\u0027.join("},{"line_number":167,"context_line":"            name for (name, _) in failures))"},{"line_number":168,"context_line":"        self.failures \u003d failures"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"class FileInspector(abc.ABC):"}],"source_content_type":"text/x-python","patch_set":1,"id":"dd33d7c4_02ca5c8c","line":168,"in_reply_to":"1b8778b0_138fde90","updated":"2024-07-31 15:17:09.000000000","message":"Oh sorry, you\u0027re right, storing failures, throwing away reason for the title, so that\u0027s fine.","commit_id":"0ff708d2d1e67a00646e638e6e2b592090637246"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"88e995b331ad7678564bff205f755b722c1e3620","unresolved":true,"context_lines":[{"line_number":165,"context_line":"    def __init__(self, failures):"},{"line_number":166,"context_line":"        super().__init__(_(\u0027Safety checks failed: %s\u0027) % \u0027,\u0027.join("},{"line_number":167,"context_line":"            name for (name, _) in failures))"},{"line_number":168,"context_line":"        self.failures \u003d failures"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"class FileInspector(abc.ABC):"}],"source_content_type":"text/x-python","patch_set":1,"id":"1b8778b0_138fde90","line":168,"in_reply_to":"982c3dbe_419ff678","updated":"2024-07-31 15:11:15.000000000","message":"The main concern with adding reason to a check instance is that it breaks idempotency of a check instance.\n\n1. If check succeeds then its reason is empty\n\n2. If check fails and then succeeds then its reason is still set (and then the 2nd check is still considered as a failure)\n\nThis may cause confusions and problems in case a check instance (or probably a file inspector instance) is used multiple times.\n\nThe current implementation persists both check name and failures but I\u0027ll submit a next version to show it more explicitly.","commit_id":"0ff708d2d1e67a00646e638e6e2b592090637246"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"afe36ae49cb03500c8eeae6436767b3c26e8204e","unresolved":true,"context_lines":[{"line_number":345,"context_line":"                if result is not None:"},{"line_number":346,"context_line":"                    raise RuntimeError(\u0027check returned result\u0027)"},{"line_number":347,"context_line":"            except SafetyViolation as exc:"},{"line_number":348,"context_line":"                failures.append((check.name, str(exc)))"},{"line_number":349,"context_line":"                LOG.warning(\u0027Safety check %s on %s failed because %s\u0027,"},{"line_number":350,"context_line":"                            check.name, self, exc)"},{"line_number":351,"context_line":"        if failures:"}],"source_content_type":"text/x-python","patch_set":1,"id":"483a1377_d50f2737","line":348,"updated":"2024-07-31 14:52:59.000000000","message":"This makes it much more laborious to find a failure and reason by name, which is what cinder (the primary user of this optional failure exclusion) will be doing. Can this be a dict of name:reason at least?","commit_id":"0ff708d2d1e67a00646e638e6e2b592090637246"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"731fc1b9c83cdef40bfe4766855b3f85d13db9b1","unresolved":false,"context_lines":[{"line_number":345,"context_line":"                if result is not None:"},{"line_number":346,"context_line":"                    raise RuntimeError(\u0027check returned result\u0027)"},{"line_number":347,"context_line":"            except SafetyViolation as exc:"},{"line_number":348,"context_line":"                failures.append((check.name, str(exc)))"},{"line_number":349,"context_line":"                LOG.warning(\u0027Safety check %s on %s failed because %s\u0027,"},{"line_number":350,"context_line":"                            check.name, self, exc)"},{"line_number":351,"context_line":"        if failures:"}],"source_content_type":"text/x-python","patch_set":1,"id":"67510b84_bb741514","line":348,"in_reply_to":"483a1377_d50f2737","updated":"2024-07-31 15:31:50.000000000","message":"Done","commit_id":"0ff708d2d1e67a00646e638e6e2b592090637246"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"baeefc506f54f45e7a79556bfc11032e9936db8a","unresolved":true,"context_lines":[{"line_number":265,"context_line":"        if not isinstance(check, SafetyCheck):"},{"line_number":266,"context_line":"            raise RuntimeError(_(\u0027Unable to add safety check of type %s\u0027) % ("},{"line_number":267,"context_line":"                type(check).__name__))"},{"line_number":268,"context_line":"        if check.name in self._safety_checks_names:"},{"line_number":269,"context_line":"            raise RuntimeError(_(\u0027Duplicate check fo name %s\u0027) % check.name)"},{"line_number":270,"context_line":"        self._safety_checks.append(check)"},{"line_number":271,"context_line":"        self._safety_check_names.append(check.name)"},{"line_number":272,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"2570dba7_93cc3385","line":269,"range":{"start_line":268,"start_character":0,"end_line":269,"end_character":76},"updated":"2024-07-31 15:29:51.000000000","message":"This validation might be too-much for initial implementation but can be added (with unit tests, probably) for safety.","commit_id":"28029fc9171f7d974e40410f34457755acd0b19b"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"731fc1b9c83cdef40bfe4766855b3f85d13db9b1","unresolved":true,"context_lines":[{"line_number":265,"context_line":"        if not isinstance(check, SafetyCheck):"},{"line_number":266,"context_line":"            raise RuntimeError(_(\u0027Unable to add safety check of type %s\u0027) % ("},{"line_number":267,"context_line":"                type(check).__name__))"},{"line_number":268,"context_line":"        if check.name in self._safety_checks_names:"},{"line_number":269,"context_line":"            raise RuntimeError(_(\u0027Duplicate check fo name %s\u0027) % check.name)"},{"line_number":270,"context_line":"        self._safety_checks.append(check)"},{"line_number":271,"context_line":"        self._safety_check_names.append(check.name)"},{"line_number":272,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"791f8ad0_0ce474fe","line":269,"range":{"start_line":268,"start_character":0,"end_line":269,"end_character":76},"in_reply_to":"2570dba7_93cc3385","updated":"2024-07-31 15:31:50.000000000","message":"No, I think it\u0027s a good idea (like the equivalent for the region checking).\n\nExcept for the typo `s/fo/of/` :)","commit_id":"28029fc9171f7d974e40410f34457755acd0b19b"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"b5c23ba4ae83866bf1675fefef72f78d97cb6b9a","unresolved":true,"context_lines":[{"line_number":349,"context_line":"                if result is not None:"},{"line_number":350,"context_line":"                    raise RuntimeError(\u0027check returned result\u0027)"},{"line_number":351,"context_line":"            except SafetyViolation as exc:"},{"line_number":352,"context_line":"                exc.check \u003d check"},{"line_number":353,"context_line":"                failures[check.name] \u003d exc"},{"line_number":354,"context_line":"                LOG.warning(\u0027Safety check %s on %s failed because %s\u0027,"},{"line_number":355,"context_line":"                            check.name, self, exc)"}],"source_content_type":"text/x-python","patch_set":2,"id":"20a8ab52_ed6852c9","line":352,"range":{"start_line":352,"start_character":16,"end_line":352,"end_character":33},"updated":"2024-07-31 15:29:15.000000000","message":"This is not very necessary now but probably help users later.","commit_id":"28029fc9171f7d974e40410f34457755acd0b19b"}]}