)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"1a6f79e60e615ec3aa57a368f3811954b4039ba4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f6ccb5ec_be76d042","updated":"2024-07-18 15:10:36.000000000","message":"recheck holding a node for debugging, this looks weird from the outside","commit_id":"3e5d7c50904ed5735d0f7e775fcac333733098e4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c5436bd5d2af283b76bb27b6f51c05268f1e7cb5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6eb17fbc_60d9ad1b","updated":"2024-07-18 16:26:26.000000000","message":"recheck post failure from pep8 with no results","commit_id":"3e5d7c50904ed5735d0f7e775fcac333733098e4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"355ee3da7a287dbb4c056b026f5298feec8f1c79","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"79c77396_0a2e7dd6","updated":"2024-07-18 13:44:51.000000000","message":"recheck still unrelated but maybe it has cleared up? I saw 64 hits yesterday and none so far today according to opensearch","commit_id":"3e5d7c50904ed5735d0f7e775fcac333733098e4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c26d493c30f1cd6ea4bfb305e576a1cb6190641d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"25c78b13_87321865","updated":"2024-07-17 22:17:25.000000000","message":"recheck um, some swift installation error?","commit_id":"3e5d7c50904ed5735d0f7e775fcac333733098e4"}],"oslo_utils/imageutils/format_inspector.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f84dec059032d4095f3e692b5c3e58e97b5f2d88","unresolved":true,"context_lines":[{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        @name should be a short name of the check (ideally no spaces)"},{"line_number":95,"context_line":"        @target_fn is the implementation we run (no args) which returns either"},{"line_number":96,"context_line":"                   None if the check passes, or a string reason why it failed."},{"line_number":97,"context_line":"        @description is a optional longer-format human-readable string that"},{"line_number":98,"context_line":"                     describes the check."},{"line_number":99,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"f612259a_102133d7","line":96,"updated":"2024-07-19 18:02:23.000000000","message":"I think I want to add another exception and have these raise instead of return a string.","commit_id":"3e5d7c50904ed5735d0f7e775fcac333733098e4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"92f25d060fada8af4811d02a36e3036c68edf05c","unresolved":false,"context_lines":[{"line_number":93,"context_line":""},{"line_number":94,"context_line":"        @name should be a short name of the check (ideally no spaces)"},{"line_number":95,"context_line":"        @target_fn is the implementation we run (no args) which returns either"},{"line_number":96,"context_line":"                   None if the check passes, or a string reason why it failed."},{"line_number":97,"context_line":"        @description is a optional longer-format human-readable string that"},{"line_number":98,"context_line":"                     describes the check."},{"line_number":99,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"cef77171_8aa4c5c5","line":96,"in_reply_to":"f612259a_102133d7","updated":"2024-07-23 16:35:04.000000000","message":"Done","commit_id":"3e5d7c50904ed5735d0f7e775fcac333733098e4"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"24e78253e67b2884a749bbfb2474a998f8934453","unresolved":true,"context_lines":[{"line_number":100,"context_line":"        self.name \u003d name"},{"line_number":101,"context_line":"        self.target_fn \u003d target_fn"},{"line_number":102,"context_line":"        self.description \u003d description"},{"line_number":103,"context_line":"        self.reason \u003d None"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def __call__(self):"},{"line_number":106,"context_line":"        \"\"\"Executes the target check function, records the result."}],"source_content_type":"text/x-python","patch_set":2,"id":"707de140_6162213b","line":103,"range":{"start_line":103,"start_character":8,"end_line":103,"end_character":26},"updated":"2024-07-23 14:23:07.000000000","message":"I wonder if we can somehow get rid of `self.reason` here, because this is specific to single check execution, not an attribute of the check logic itself. I see __call__ raises an exception and I guess we can just use exception directly in upper layer ?","commit_id":"03b7bcedca11dc54d8c9157949c849457552d2dd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8e9b98a22eddebd85165550e388ec99fb6b4436c","unresolved":true,"context_lines":[{"line_number":100,"context_line":"        self.name \u003d name"},{"line_number":101,"context_line":"        self.target_fn \u003d target_fn"},{"line_number":102,"context_line":"        self.description \u003d description"},{"line_number":103,"context_line":"        self.reason \u003d None"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def __call__(self):"},{"line_number":106,"context_line":"        \"\"\"Executes the target check function, records the result."}],"source_content_type":"text/x-python","patch_set":2,"id":"f9fbd1cc_4833b186","line":103,"range":{"start_line":103,"start_character":8,"end_line":103,"end_character":26},"in_reply_to":"2d714100_b441dba7","updated":"2024-07-31 14:56:34.000000000","message":"I commented on the other, but let me reinforce here that there\u0027s a lot more information we may need to pull out here in the future and I was hoping that making and linking these objects will help there. For example, the backing file check may need to actually communicate the backing_file name itself, and perhaps the format or scheme. We may also want to allow the caller to *provide* a check with a regex or pass/fail acceptance function at some point, and so it just feels to me like the way these are created per-use allows for that without much concern.\n\nAnyway, getting this merged sooner than later is most important and we can work out how to do those things when we get there regardless of what we do here. I\u0027ll wait for your replies on the example and then fold that in here.","commit_id":"03b7bcedca11dc54d8c9157949c849457552d2dd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"92f25d060fada8af4811d02a36e3036c68edf05c","unresolved":true,"context_lines":[{"line_number":100,"context_line":"        self.name \u003d name"},{"line_number":101,"context_line":"        self.target_fn \u003d target_fn"},{"line_number":102,"context_line":"        self.description \u003d description"},{"line_number":103,"context_line":"        self.reason \u003d None"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def __call__(self):"},{"line_number":106,"context_line":"        \"\"\"Executes the target check function, records the result."}],"source_content_type":"text/x-python","patch_set":2,"id":"de06c8b3_d2ebf7ca","line":103,"range":{"start_line":103,"start_character":8,"end_line":103,"end_character":26},"in_reply_to":"707de140_6162213b","updated":"2024-07-23 16:35:04.000000000","message":"The reason I wanted it this way is because of conditional skipping. Cinder has a need to ignore the backing file check in certain circumstances but not others. Thus they need the safety check to be more granular than all-or-nothing, and they need to be able to examine the failures and the reasons for each and decide when it\u0027s appropriate to let a failure go and when to call it fatal. So my goal here was to capture the failure reasons tied to the short name of the check so that Cinder (and others in the future) can look at the top-level exception and step through the actual fails to make that decision.","commit_id":"03b7bcedca11dc54d8c9157949c849457552d2dd"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"5f27107f550e872f8171ad61a0a277f255341e53","unresolved":true,"context_lines":[{"line_number":100,"context_line":"        self.name \u003d name"},{"line_number":101,"context_line":"        self.target_fn \u003d target_fn"},{"line_number":102,"context_line":"        self.description \u003d description"},{"line_number":103,"context_line":"        self.reason \u003d None"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":"    def __call__(self):"},{"line_number":106,"context_line":"        \"\"\"Executes the target check function, records the result."}],"source_content_type":"text/x-python","patch_set":2,"id":"2d714100_b441dba7","line":103,"range":{"start_line":103,"start_character":8,"end_line":103,"end_character":26},"in_reply_to":"de06c8b3_d2ebf7ca","updated":"2024-07-31 14:44:00.000000000","message":"OK I understand the motivation but I still think we can decouple a check instance and its failure, (see https://review.opendev.org/c/openstack/oslo.utils/+/925403/ ).\n\nMy initial concern with the current implementation is that it may cause a wired result in case a check instance is somehow reused (I know it may not happen but I\u0027m more concerned with any future change). I also see a slight confusion between \"failure\" and \"failed_check\" in L171. I hope the decoupling can ease these concerns.","commit_id":"03b7bcedca11dc54d8c9157949c849457552d2dd"}]}
