)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"467714757c905703a499ec1659dbe79d30496d26","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"9635c76c_8e9579aa","updated":"2024-08-27 11:03:14.000000000","message":"It seems my comments are still open so I stopped my current round of review here now.","commit_id":"088b8a2434acd7f67f06a51d55f817347a251c3f"}],"nova/healthcheck/manager.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"244b1e1c02e5bb822cab77d4f258faa473ac9512","unresolved":true,"context_lines":[{"line_number":97,"context_line":"            # manually debugging/executing this module in an interactive shell."},{"line_number":98,"context_line":"            self.sleep()"},{"line_number":99,"context_line":"        else:"},{"line_number":100,"context_line":"            LOG.debug(\u0027healthcheck endpoint is not enabled\u0027)"},{"line_number":101,"context_line":"    def sleep(self, time: int \u003d 0) -\u003e None:"},{"line_number":102,"context_line":"        # NOTE(sean-k-mooney): when running this module in an interactive"},{"line_number":103,"context_line":"        # python shell you should call sleep to yield execution form the"}],"source_content_type":"text/x-python","patch_set":2,"id":"4718e757_eecc1bda","line":100,"range":{"start_line":100,"start_character":12,"end_line":100,"end_character":21},"updated":"2022-02-10 04:07:38.000000000","message":"this should be info level","commit_id":"a737b86ecc4fb672ebbbc0fc47bc0b157bdcb7e7"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"8cc6551bc4a2210f7e02e701357a1d99351fae62","unresolved":false,"context_lines":[{"line_number":97,"context_line":"            # manually debugging/executing this module in an interactive shell."},{"line_number":98,"context_line":"            self.sleep()"},{"line_number":99,"context_line":"        else:"},{"line_number":100,"context_line":"            LOG.debug(\u0027healthcheck endpoint is not enabled\u0027)"},{"line_number":101,"context_line":"    def sleep(self, time: int \u003d 0) -\u003e None:"},{"line_number":102,"context_line":"        # NOTE(sean-k-mooney): when running this module in an interactive"},{"line_number":103,"context_line":"        # python shell you should call sleep to yield execution form the"}],"source_content_type":"text/x-python","patch_set":2,"id":"703cff89_b9dac57c","line":100,"range":{"start_line":100,"start_character":12,"end_line":100,"end_character":21},"in_reply_to":"4718e757_eecc1bda","updated":"2023-01-21 00:25:26.000000000","message":"Done","commit_id":"a737b86ecc4fb672ebbbc0fc47bc0b157bdcb7e7"}],"nova/healthcheck/utils.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"6859e4b2f43094ae7aed3559ec990938db7833ab","unresolved":true,"context_lines":[{"line_number":26,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"def get_context(args: tuple) -\u003e context.RequestContext:"},{"line_number":30,"context_line":"    ctx \u003d None"},{"line_number":31,"context_line":"    if args and isinstance(args[0], context.RequestContext):"},{"line_number":32,"context_line":"        ctx \u003d args[0]"}],"source_content_type":"text/x-python","patch_set":16,"id":"204166ca_7885ebe3","line":29,"updated":"2024-02-09 10:01:33.000000000","message":"Actually we don\u0027t need this logic. You create a global healthcheck tracker for each service and that is included in the RequestContext automatically when contex.get_context() is called. As the context is only used to access the tracker you can simply access the global tracker instead or use context.get_context(). \n\nI don\u0027t think we ever want to pass around multiple different trackers via different context instances so we don\u0027t need to rely on the specific context instance the decorated function gets.\nAlso this would allow to use the context manager or decorator in a situation where context is not passed in.","commit_id":"0cac913ecac527ae92e922905a5ba6a3911470c9"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"5669112803380a3b90f229afb9e6808ec42672bc","unresolved":true,"context_lines":[{"line_number":26,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"def get_context(args: tuple) -\u003e context.RequestContext:"},{"line_number":30,"context_line":"    ctx \u003d None"},{"line_number":31,"context_line":"    if args and isinstance(args[0], context.RequestContext):"},{"line_number":32,"context_line":"        ctx \u003d args[0]"}],"source_content_type":"text/x-python","patch_set":16,"id":"9762c745_61b7958d","line":29,"in_reply_to":"204166ca_7885ebe3","updated":"2024-02-13 10:22:24.000000000","message":"context.get_context() would be ok but i am expcitly tryting to ensure we dont acess the glogal object directly.\n\ni really dont want this to be used like a global and want to pass it in via the context.\n\nif people prefer we could make it a singleton but i dont line how hat impacts testing normally so im trying to avoid that.","commit_id":"0cac913ecac527ae92e922905a5ba6a3911470c9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"10d35839a10b0e83fdbd5c0a9ab425d70ce17c12","unresolved":true,"context_lines":[{"line_number":39,"context_line":"    return ctx"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"@contextlib.contextmanager"},{"line_number":43,"context_line":"def health_cm("},{"line_number":44,"context_line":"        ctx: context.RequestContext, check_name: str,"},{"line_number":45,"context_line":"        error_set \u003d (Exception,)"}],"source_content_type":"text/x-python","patch_set":16,"id":"0af05e49_a7b44fab","line":42,"updated":"2024-02-09 09:54:43.000000000","message":"I feel that this decorator allows us to use the resulting entity both as a context manager and as a decorator. From the doc:\n\n\u003e\u003e contextmanager() uses ContextDecorator so the context managers it creates can be used as decorators as well as in with statements. When used as a decorator, a new generator instance is implicitly created on each function call (this allows the otherwise “one-shot” context managers created by contextmanager() to meet the requirement that context managers support multiple invocations in order to be used as decorators).\n\nIf this is true then the explicit decorator down below is not needed.","commit_id":"0cac913ecac527ae92e922905a5ba6a3911470c9"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"5669112803380a3b90f229afb9e6808ec42672bc","unresolved":true,"context_lines":[{"line_number":39,"context_line":"    return ctx"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"@contextlib.contextmanager"},{"line_number":43,"context_line":"def health_cm("},{"line_number":44,"context_line":"        ctx: context.RequestContext, check_name: str,"},{"line_number":45,"context_line":"        error_set \u003d (Exception,)"}],"source_content_type":"text/x-python","patch_set":16,"id":"cf98709e_162d3a51","line":42,"in_reply_to":"0af05e49_a7b44fab","updated":"2024-02-13 10:22:24.000000000","message":"hum... you might be right i have never tried that\n\nif it works then sure ill delete the other form.","commit_id":"0cac913ecac527ae92e922905a5ba6a3911470c9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ee20a020770dff34bd9a119f23498e45d883fe63","unresolved":true,"context_lines":[{"line_number":60,"context_line":"            )"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"def health_decorator("},{"line_number":64,"context_line":"        check_name: str, error_set \u003d (Exception,)"},{"line_number":65,"context_line":") -\u003e ty.Callable:"},{"line_number":66,"context_line":"    def wrapper(func):"}],"source_content_type":"text/x-python","patch_set":16,"id":"0fcebbfc_d9c7c38c","line":63,"updated":"2024-02-09 10:12:06.000000000","message":"I think the naming can be simplified as this now ended up being called as\n```\n    @healthcheck_utils.health_decorator(\n```\n\nSo calling it simply `check` would be enough","commit_id":"0cac913ecac527ae92e922905a5ba6a3911470c9"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"5669112803380a3b90f229afb9e6808ec42672bc","unresolved":true,"context_lines":[{"line_number":60,"context_line":"            )"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"def health_decorator("},{"line_number":64,"context_line":"        check_name: str, error_set \u003d (Exception,)"},{"line_number":65,"context_line":") -\u003e ty.Callable:"},{"line_number":66,"context_line":"    def wrapper(func):"}],"source_content_type":"text/x-python","patch_set":16,"id":"cbdff4d0_80282430","line":63,"in_reply_to":"0fcebbfc_d9c7c38c","updated":"2024-02-13 10:22:24.000000000","message":"yep","commit_id":"0cac913ecac527ae92e922905a5ba6a3911470c9"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"10d35839a10b0e83fdbd5c0a9ab425d70ce17c12","unresolved":true,"context_lines":[{"line_number":70,"context_line":"                result \u003d func(*args, **kwargs)"},{"line_number":71,"context_line":"            except error_set as e:"},{"line_number":72,"context_line":"                ctx \u003d get_context(args)"},{"line_number":73,"context_line":"                if ctx.health_tracker is not None:"},{"line_number":74,"context_line":"                    ctx.health_tracker.record("},{"line_number":75,"context_line":"                        check_name, hm.HealthcheckStatus.FAIL,"},{"line_number":76,"context_line":"                        output\u003dstr(e)"},{"line_number":77,"context_line":"                    )"}],"source_content_type":"text/x-python","patch_set":16,"id":"55969fa8_40a38c7b","line":74,"range":{"start_line":73,"start_character":0,"end_line":74,"end_character":46},"updated":"2024-02-09 09:54:43.000000000","message":"Can we add function directly to the NovaContext that hides this \"if\" from the caller and simply ignores the record if the tracker is not in the context? This if is not repeated at least 4 times.","commit_id":"0cac913ecac527ae92e922905a5ba6a3911470c9"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"5669112803380a3b90f229afb9e6808ec42672bc","unresolved":true,"context_lines":[{"line_number":70,"context_line":"                result \u003d func(*args, **kwargs)"},{"line_number":71,"context_line":"            except error_set as e:"},{"line_number":72,"context_line":"                ctx \u003d get_context(args)"},{"line_number":73,"context_line":"                if ctx.health_tracker is not None:"},{"line_number":74,"context_line":"                    ctx.health_tracker.record("},{"line_number":75,"context_line":"                        check_name, hm.HealthcheckStatus.FAIL,"},{"line_number":76,"context_line":"                        output\u003dstr(e)"},{"line_number":77,"context_line":"                    )"}],"source_content_type":"text/x-python","patch_set":16,"id":"8f2da117_c631aff6","line":74,"range":{"start_line":73,"start_character":0,"end_line":74,"end_character":46},"in_reply_to":"55969fa8_40a38c7b","updated":"2024-02-13 10:22:24.000000000","message":"so i was debating if i need this at all relly.\n\ni put this in in case it woudl ever not be initalised but im not sure if that will ever happen so sure i can look at how this is done or hide this in a function on the context object.\n\n\ni had to do this in other places where the decortor/contoext manager were not what i needed too so there would be addtional duplcication that could be avoided \n\ni.e. https://review.opendev.org/c/openstack/nova/+/907424/1/nova/virt/libvirt/driver.py#5307","commit_id":"0cac913ecac527ae92e922905a5ba6a3911470c9"}]}