)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":34429,"name":"Tom Weininger","email":"dienste@weinimo.de","username":"tweining"},"change_message_id":"552ee1268ed93d01c91941d3e45da0449a15f826","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"0f175eba_e73a71c4","updated":"2022-04-13 16:09:34.000000000","message":"I agree with johnsom\u0027s comments.","commit_id":"222c3889a452f2ec991a8624f113c50f14cdae8a"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"06a794cf49e93b645efbe166bdf85373c8916a2e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"238bbdc9_be725729","updated":"2022-04-13 15:50:46.000000000","message":"If you don\u0027t have shell_exec_t the PING health monitor will also fail. This should at least be called on in a release note if not in the documentation discussing the health monitors.","commit_id":"222c3889a452f2ec991a8624f113c50f14cdae8a"},{"author":{"_account_id":28223,"name":"Cedric Jeanneret","display_name":"cjeanner (Tengu)","email":"cjeanner@redhat.com","username":"cjeanner"},"change_message_id":"06069997f22cd4051398c1e7e98245918730aae1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"dd9cb771_c6cc5711","in_reply_to":"238bbdc9_be725729","updated":"2022-04-14 04:26:51.000000000","message":"not sure the context would be the same for that one. At least, Greg\u0027s test didn\u0027t show any denial, but I don\u0027t know if there were any ping involved. Further testing would be needed in order to assess this \"loss\" or not.\n\nIn any cases: shell_exec_t should not be allowed for a web server, since any flaw would allow to be dropped in a shell. And with all the existing ways to escalate privileges, we can\u0027t say \"heh, it\u0027s a limited user, it\u0027s fine\".","commit_id":"222c3889a452f2ec991a8624f113c50f14cdae8a"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"e699376aed1a58dddc7e69bdc2546b04e310f964","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"5f229c00_da22bfd5","updated":"2022-04-14 21:46:33.000000000","message":"LGTM","commit_id":"21d74c373b92f8711ff91766c2e7c8bcb811bdda"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"2e7259d6186a0b509c3bbd4a261c61888f8d4049","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"84504d43_0f8e8c82","updated":"2022-04-14 21:48:07.000000000","message":"recheck\ndiskimage-builder fix has merged","commit_id":"21d74c373b92f8711ff91766c2e7c8bcb811bdda"},{"author":{"_account_id":34429,"name":"Tom Weininger","email":"dienste@weinimo.de","username":"tweining"},"change_message_id":"750802f7ed038aaff7c7cca7d15a30a3735cc7a7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"0c2b9d01_b3de7396","updated":"2022-04-19 09:57:33.000000000","message":"recheck grenade job failed still. This issue seems to be fixed on current master.","commit_id":"21d74c373b92f8711ff91766c2e7c8bcb811bdda"}],"octavia/amphorae/backends/agent/api_server/loadbalancer.py":[{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"06a794cf49e93b645efbe166bdf85373c8916a2e","unresolved":true,"context_lines":[{"line_number":246,"context_line":"            if consts.OFFLINE \u003d\u003d self._check_haproxy_status(lb_id):"},{"line_number":247,"context_line":"                action \u003d consts.AMP_ACTION_START"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"        if action \u003d\u003d consts.AMP_ACTION_RELOAD:"},{"line_number":250,"context_line":"            # We now have the \"real\" action, so if we really reload,"},{"line_number":251,"context_line":"            # we first have to save the state."},{"line_number":252,"context_line":"            haproxy_state_file \u003d util.state_file_path(lb_id)"}],"source_content_type":"text/x-python","patch_set":5,"id":"169df851_eb79d0a7","line":249,"updated":"2022-04-13 15:50:46.000000000","message":"This should be merged with the conditional above.","commit_id":"222c3889a452f2ec991a8624f113c50f14cdae8a"},{"author":{"_account_id":28223,"name":"Cedric Jeanneret","display_name":"cjeanner (Tengu)","email":"cjeanner@redhat.com","username":"cjeanner"},"change_message_id":"a5f50a28a7f485fc79c2470a5f37ac7d2aad30d3","unresolved":false,"context_lines":[{"line_number":246,"context_line":"            if consts.OFFLINE \u003d\u003d self._check_haproxy_status(lb_id):"},{"line_number":247,"context_line":"                action \u003d consts.AMP_ACTION_START"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"        if action \u003d\u003d consts.AMP_ACTION_RELOAD:"},{"line_number":250,"context_line":"            # We now have the \"real\" action, so if we really reload,"},{"line_number":251,"context_line":"            # we first have to save the state."},{"line_number":252,"context_line":"            haproxy_state_file \u003d util.state_file_path(lb_id)"}],"source_content_type":"text/x-python","patch_set":5,"id":"e68ecb8a_16e74162","line":249,"in_reply_to":"169df851_eb79d0a7","updated":"2022-04-14 04:44:29.000000000","message":"Done","commit_id":"222c3889a452f2ec991a8624f113c50f14cdae8a"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"06a794cf49e93b645efbe166bdf85373c8916a2e","unresolved":true,"context_lines":[{"line_number":257,"context_line":"                # We accept to reload haproxy even if the state_file is"},{"line_number":258,"context_line":"                # not generated, but we probably want to know about that"},{"line_number":259,"context_line":"                # failure!"},{"line_number":260,"context_line":"                LOG.debug(\u0027Failed to save haproxy-%s state!\u0027, lb_id)"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"        cmd \u003d (\"/usr/sbin/service haproxy-{lb_id} {action}\".format("},{"line_number":263,"context_line":"            lb_id\u003dlb_id, action\u003daction))"}],"source_content_type":"text/x-python","patch_set":5,"id":"c4849db9_7cf060a8","line":260,"updated":"2022-04-13 15:50:46.000000000","message":"I think this should be at least warning level.","commit_id":"222c3889a452f2ec991a8624f113c50f14cdae8a"},{"author":{"_account_id":28223,"name":"Cedric Jeanneret","display_name":"cjeanner (Tengu)","email":"cjeanner@redhat.com","username":"cjeanner"},"change_message_id":"a5f50a28a7f485fc79c2470a5f37ac7d2aad30d3","unresolved":false,"context_lines":[{"line_number":257,"context_line":"                # We accept to reload haproxy even if the state_file is"},{"line_number":258,"context_line":"                # not generated, but we probably want to know about that"},{"line_number":259,"context_line":"                # failure!"},{"line_number":260,"context_line":"                LOG.debug(\u0027Failed to save haproxy-%s state!\u0027, lb_id)"},{"line_number":261,"context_line":""},{"line_number":262,"context_line":"        cmd \u003d (\"/usr/sbin/service haproxy-{lb_id} {action}\".format("},{"line_number":263,"context_line":"            lb_id\u003dlb_id, action\u003daction))"}],"source_content_type":"text/x-python","patch_set":5,"id":"3c5f5ca1_40ddb968","line":260,"in_reply_to":"c4849db9_7cf060a8","updated":"2022-04-14 04:44:29.000000000","message":"Done","commit_id":"222c3889a452f2ec991a8624f113c50f14cdae8a"},{"author":{"_account_id":34429,"name":"Tom Weininger","email":"dienste@weinimo.de","username":"tweining"},"change_message_id":"90c4fb81dad5712f140ee27ddefb362c61000be1","unresolved":true,"context_lines":[{"line_number":245,"context_line":"        if action \u003d\u003d consts.AMP_ACTION_RELOAD:"},{"line_number":246,"context_line":"            if consts.OFFLINE \u003d\u003d self._check_haproxy_status(lb_id):"},{"line_number":247,"context_line":"                action \u003d consts.AMP_ACTION_START"},{"line_number":248,"context_line":"            else:"},{"line_number":249,"context_line":"                # We first have to save the state when we reload"},{"line_number":250,"context_line":"                haproxy_state_file \u003d util.state_file_path(lb_id)"},{"line_number":251,"context_line":"                stat_sock_file \u003d util.haproxy_sock_path(lb_id)"}],"source_content_type":"text/x-python","patch_set":11,"id":"50e093c9_50f36175","line":248,"range":{"start_line":248,"start_character":0,"end_line":248,"end_character":17},"updated":"2022-04-14 08:56:12.000000000","message":"I just want to highlight that this else wasn\u0027t there in a previous version of this change. It seems to make sense though.","commit_id":"21d74c373b92f8711ff91766c2e7c8bcb811bdda"},{"author":{"_account_id":28223,"name":"Cedric Jeanneret","display_name":"cjeanner (Tengu)","email":"cjeanner@redhat.com","username":"cjeanner"},"change_message_id":"e90fa2cb2e2bf57ffe940f99ca1252e86cff4464","unresolved":true,"context_lines":[{"line_number":245,"context_line":"        if action \u003d\u003d consts.AMP_ACTION_RELOAD:"},{"line_number":246,"context_line":"            if consts.OFFLINE \u003d\u003d self._check_haproxy_status(lb_id):"},{"line_number":247,"context_line":"                action \u003d consts.AMP_ACTION_START"},{"line_number":248,"context_line":"            else:"},{"line_number":249,"context_line":"                # We first have to save the state when we reload"},{"line_number":250,"context_line":"                haproxy_state_file \u003d util.state_file_path(lb_id)"},{"line_number":251,"context_line":"                stat_sock_file \u003d util.haproxy_sock_path(lb_id)"}],"source_content_type":"text/x-python","patch_set":11,"id":"776f2dfe_86212eda","line":248,"range":{"start_line":248,"start_character":0,"end_line":248,"end_character":17},"in_reply_to":"50e093c9_50f36175","updated":"2022-04-14 08:59:38.000000000","message":"well, that else is the answer to another comment. We want to get that \"save state\" if and only if we\u0027re in an actual RELOAD case.\n\nBefore that, the block bellow was in a dedicated new \"if action \u003d\u003d consts.AMP_ACTION_RELOAD\". that else is the equivalent.","commit_id":"21d74c373b92f8711ff91766c2e7c8bcb811bdda"},{"author":{"_account_id":11628,"name":"Michael Johnson","email":"johnsomor@gmail.com","username":"johnsom"},"change_message_id":"e699376aed1a58dddc7e69bdc2546b04e310f964","unresolved":true,"context_lines":[{"line_number":245,"context_line":"        if action \u003d\u003d consts.AMP_ACTION_RELOAD:"},{"line_number":246,"context_line":"            if consts.OFFLINE \u003d\u003d self._check_haproxy_status(lb_id):"},{"line_number":247,"context_line":"                action \u003d consts.AMP_ACTION_START"},{"line_number":248,"context_line":"            else:"},{"line_number":249,"context_line":"                # We first have to save the state when we reload"},{"line_number":250,"context_line":"                haproxy_state_file \u003d util.state_file_path(lb_id)"},{"line_number":251,"context_line":"                stat_sock_file \u003d util.haproxy_sock_path(lb_id)"}],"source_content_type":"text/x-python","patch_set":11,"id":"fca04858_a59c5742","line":248,"range":{"start_line":248,"start_character":0,"end_line":248,"end_character":17},"in_reply_to":"776f2dfe_86212eda","updated":"2022-04-14 21:46:33.000000000","message":"Yeah, the else is correct here and in response to my previous comment about consolidating the logic.","commit_id":"21d74c373b92f8711ff91766c2e7c8bcb811bdda"},{"author":{"_account_id":34429,"name":"Tom Weininger","email":"dienste@weinimo.de","username":"tweining"},"change_message_id":"2340e33edc881e27cda655cabf436928956e7714","unresolved":false,"context_lines":[{"line_number":245,"context_line":"        if action \u003d\u003d consts.AMP_ACTION_RELOAD:"},{"line_number":246,"context_line":"            if consts.OFFLINE \u003d\u003d self._check_haproxy_status(lb_id):"},{"line_number":247,"context_line":"                action \u003d consts.AMP_ACTION_START"},{"line_number":248,"context_line":"            else:"},{"line_number":249,"context_line":"                # We first have to save the state when we reload"},{"line_number":250,"context_line":"                haproxy_state_file \u003d util.state_file_path(lb_id)"},{"line_number":251,"context_line":"                stat_sock_file \u003d util.haproxy_sock_path(lb_id)"}],"source_content_type":"text/x-python","patch_set":11,"id":"f4e4f0aa_5a67e857","line":248,"range":{"start_line":248,"start_character":0,"end_line":248,"end_character":17},"in_reply_to":"fca04858_a59c5742","updated":"2022-04-19 08:30:43.000000000","message":"Ack","commit_id":"21d74c373b92f8711ff91766c2e7c8bcb811bdda"}],"octavia/amphorae/backends/utils/haproxy_query.py":[{"author":{"_account_id":34429,"name":"Tom Weininger","email":"dienste@weinimo.de","username":"tweining"},"change_message_id":"552ee1268ed93d01c91941d3e45da0449a15f826","unresolved":true,"context_lines":[{"line_number":161,"context_line":"        except Exception:"},{"line_number":162,"context_line":"            # Catch any exception - may be socket issue, or write permission"},{"line_number":163,"context_line":"            # issue as well."},{"line_number":164,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":5,"id":"833476c4_74186ecc","line":164,"range":{"start_line":164,"start_character":12,"end_line":164,"end_character":24},"updated":"2022-04-13 16:09:34.000000000","message":"I think it would be helpful to log the exception including the type either here or let the caller handle and log the exception. It would help to know why it failed.","commit_id":"222c3889a452f2ec991a8624f113c50f14cdae8a"},{"author":{"_account_id":28223,"name":"Cedric Jeanneret","display_name":"cjeanner (Tengu)","email":"cjeanner@redhat.com","username":"cjeanner"},"change_message_id":"a5f50a28a7f485fc79c2470a5f37ac7d2aad30d3","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        except Exception:"},{"line_number":162,"context_line":"            # Catch any exception - may be socket issue, or write permission"},{"line_number":163,"context_line":"            # issue as well."},{"line_number":164,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":5,"id":"e133c4e6_c9569069","line":164,"range":{"start_line":164,"start_character":12,"end_line":164,"end_character":24},"in_reply_to":"0d4e2ac2_f9c9974e","updated":"2022-04-14 04:44:29.000000000","message":"Hopefully the edition of the class is OK for you - imho it\u0027s the best to do at this point. I\u0027ll collaborate with Greg if more work is needed.","commit_id":"222c3889a452f2ec991a8624f113c50f14cdae8a"},{"author":{"_account_id":28223,"name":"Cedric Jeanneret","display_name":"cjeanner (Tengu)","email":"cjeanner@redhat.com","username":"cjeanner"},"change_message_id":"06069997f22cd4051398c1e7e98245918730aae1","unresolved":true,"context_lines":[{"line_number":161,"context_line":"        except Exception:"},{"line_number":162,"context_line":"            # Catch any exception - may be socket issue, or write permission"},{"line_number":163,"context_line":"            # issue as well."},{"line_number":164,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":5,"id":"0d4e2ac2_f9c9974e","line":164,"range":{"start_line":164,"start_character":12,"end_line":164,"end_character":24},"in_reply_to":"833476c4_74186ecc","updated":"2022-04-14 04:26:51.000000000","message":"there\u0027s apparently no logger instance in there - but we may be able to pass on to the class, as an option? That would allow to improve the overall class and its method.","commit_id":"222c3889a452f2ec991a8624f113c50f14cdae8a"},{"author":{"_account_id":34429,"name":"Tom Weininger","email":"dienste@weinimo.de","username":"tweining"},"change_message_id":"4e5b5fa376f5729073c1f3151b65eb028147e8db","unresolved":true,"context_lines":[{"line_number":161,"context_line":"        except Exception:"},{"line_number":162,"context_line":"            # Catch any exception - may be socket issue, or write permission"},{"line_number":163,"context_line":"            # issue as well."},{"line_number":164,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":5,"id":"f2667a33_fad48a63","line":164,"range":{"start_line":164,"start_character":12,"end_line":164,"end_character":24},"in_reply_to":"e133c4e6_c9569069","updated":"2022-04-14 07:48:04.000000000","message":"You could just do this at the top of this module and use LOG everywhere in this file:\n\nfrom oslo_log import log as logging\n\nLOG \u003d logging.getLogger(__name__)\n\nThen you don\u0027t need to pass the logger during instantiation.","commit_id":"222c3889a452f2ec991a8624f113c50f14cdae8a"},{"author":{"_account_id":28223,"name":"Cedric Jeanneret","display_name":"cjeanner (Tengu)","email":"cjeanner@redhat.com","username":"cjeanner"},"change_message_id":"7ef364936cb265d6bec06fab95b37b7f57557bce","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        except Exception:"},{"line_number":162,"context_line":"            # Catch any exception - may be socket issue, or write permission"},{"line_number":163,"context_line":"            # issue as well."},{"line_number":164,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":5,"id":"2772aa5c_41c9ab7f","line":164,"range":{"start_line":164,"start_character":12,"end_line":164,"end_character":24},"in_reply_to":"f2667a33_fad48a63","updated":"2022-04-14 08:04:09.000000000","message":"Done","commit_id":"222c3889a452f2ec991a8624f113c50f14cdae8a"}]}