)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"5fd3331e9847be9519f7a4e40e1cbb4cc6c360a6","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Vasyl Saienko \u003cvsaienko@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2024-02-13 09:59:51 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Drop hardcoded dhcp agent type"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Pick up dhcp agent type value from the constants instead of"},{"line_number":10,"context_line":"hardcoded value."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ee367391_1e444146","line":7,"updated":"2024-02-16 10:20:15.000000000","message":"nit: and L3","commit_id":"af7069145b803e156c59efe9700969d4b4f887e8"},{"author":{"_account_id":14525,"name":"Vasyl Saienko","email":"vsaienko@mirantis.com","username":"vsaienko"},"change_message_id":"8234c23fd8c313d4bc0f76726945be506ef84c8f","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Vasyl Saienko \u003cvsaienko@mirantis.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2024-02-13 09:59:51 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Drop hardcoded dhcp agent type"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Pick up dhcp agent type value from the constants instead of"},{"line_number":10,"context_line":"hardcoded value."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"2ec7bebd_4bbaffaf","line":7,"in_reply_to":"ee367391_1e444146","updated":"2024-02-16 19:03:54.000000000","message":"Done","commit_id":"af7069145b803e156c59efe9700969d4b4f887e8"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"5fd3331e9847be9519f7a4e40e1cbb4cc6c360a6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"51aad7f8_60b8f958","updated":"2024-02-16 10:20:15.000000000","message":"thanks, looks ok","commit_id":"af7069145b803e156c59efe9700969d4b4f887e8"},{"author":{"_account_id":14525,"name":"Vasyl Saienko","email":"vsaienko@mirantis.com","username":"vsaienko"},"change_message_id":"8234c23fd8c313d4bc0f76726945be506ef84c8f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f836144b_f443652e","updated":"2024-02-16 19:03:54.000000000","message":"thanks, done","commit_id":"da53a55bece9a4e747afdd722f2796fcc84526b6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"72f9feabfa66315ba6650aa095cfc7bfc92afc6f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"bac047d1_0a519a2c","updated":"2024-02-21 23:09:36.000000000","message":"Vasyl - why did you add so many reviewers? Some of these people do not really work on neutron any more.","commit_id":"121855c9abdc311e28ef4e74247235c6a7c88c08"},{"author":{"_account_id":14525,"name":"Vasyl Saienko","email":"vsaienko@mirantis.com","username":"vsaienko"},"change_message_id":"c550f2d70a7a7c9254c9023999a7e4f045eef04a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"f662a4dd_5ec3b501","in_reply_to":"b8f6a19d_6cde085a","updated":"2024-02-22 17:13:21.000000000","message":"Thanks for your comments, I will take them into account when doing other patches. This looks strange if person is not a reviewer anymore why it is still in the core reviewers list?","commit_id":"121855c9abdc311e28ef4e74247235c6a7c88c08"},{"author":{"_account_id":14525,"name":"Vasyl Saienko","email":"vsaienko@mirantis.com","username":"vsaienko"},"change_message_id":"b209854b570c41a9e2d781e705bc92c9c278ec87","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"c9e2f929_f05f3723","in_reply_to":"bac047d1_0a519a2c","updated":"2024-02-22 08:31:33.000000000","message":"why they in core reviewers than?","commit_id":"121855c9abdc311e28ef4e74247235c6a7c88c08"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"54fe8042ffb1495838969b7fe4c74d04fac2dee5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"b8f6a19d_6cde085a","in_reply_to":"c9e2f929_f05f3723","updated":"2024-02-22 16:27:06.000000000","message":"They are not all in the core reviewers list [0]. It is also just not good form to add so many - there were already two that reviewed this, maybe adding one more for a tiebreaker, or ping one on irc, but please don\u0027t add 13 reviewers.\n\n[0] https://review.opendev.org/admin/groups/9b227d5d1b27471dacbe9a9161904ec115a74138,members","commit_id":"121855c9abdc311e28ef4e74247235c6a7c88c08"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"843e1c1f890f16edce23b7a29e1ab5e17be2b679","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"a6a933f8_c2e4dc4d","updated":"2024-02-26 08:02:06.000000000","message":"+1 because the change makes sense.\n\nAs Brian commented:\n1) Please don\u0027t spam a patch adding so many reviewers.\n2) Add a rationale and a LP bug link.\n3) Let\u0027s make valuable any new code; this change looks a bit unnecessary.","commit_id":"a44a06fd31ec267690c234e4e3553b170d20b76d"},{"author":{"_account_id":14525,"name":"Vasyl Saienko","email":"vsaienko@mirantis.com","username":"vsaienko"},"change_message_id":"8089d64528f7fce6c706dac0e731bbd589feb4ba","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"53fa94b6_54cbf29b","in_reply_to":"a6a933f8_c2e4dc4d","updated":"2024-03-17 13:03:02.000000000","message":"Having argument name `agent_type` aligned with value it contain is a good code pattern otherwise it is easier to do mistakes.\n\n1) I thought email is yet another form of communication between members in community. Asking for review does not look like a spam message. I didn\u0027t know that some persons may be to nervous when they receive a message with ask for review. I will take into account this.\n\n2) Ok I will\n\n3) This change actualizes value and argument name, anyone who will update related functions will have to keep in mind that argument may contain free form string which may lead to errors. I just wanted to make code more clear.","commit_id":"a44a06fd31ec267690c234e4e3553b170d20b76d"}],"neutron/db/agentschedulers_db.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"34b6e27a46e52871acc4a22dffbb97093a252826","unresolved":true,"context_lines":[{"line_number":298,"context_line":"        \"\"\""},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"        agent_dead_limit \u003d self.agent_dead_limit_seconds()"},{"line_number":301,"context_line":"        self.wait_down_agents(constants.AGENT_TYPE_DHCP, agent_dead_limit)"},{"line_number":302,"context_line":"        cutoff \u003d self.get_cutoff_time(agent_dead_limit)"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        context \u003d ncontext.get_admin_context()"}],"source_content_type":"text/x-python","patch_set":1,"id":"ca13fab6_d7429f5e","line":301,"updated":"2024-02-12 19:21:11.000000000","message":"This is wrong, these strings are not equivalent, and this will result in a warning string something like:\n\n\"Time since last DHCP agent agent reschedule check has...\"\n\nThe L3 agent code calls this via reschedule_routers_from_down_agents() and passes \u0027L3\u0027.","commit_id":"ea2a9555db6a2343683dfe26c89c123c32acc97e"},{"author":{"_account_id":14525,"name":"Vasyl Saienko","email":"vsaienko@mirantis.com","username":"vsaienko"},"change_message_id":"2c3b6b26e31db6fa29ed9505b3ff8075fc2c31b4","unresolved":true,"context_lines":[{"line_number":298,"context_line":"        \"\"\""},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"        agent_dead_limit \u003d self.agent_dead_limit_seconds()"},{"line_number":301,"context_line":"        self.wait_down_agents(constants.AGENT_TYPE_DHCP, agent_dead_limit)"},{"line_number":302,"context_line":"        cutoff \u003d self.get_cutoff_time(agent_dead_limit)"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        context \u003d ncontext.get_admin_context()"}],"source_content_type":"text/x-python","patch_set":1,"id":"1bf8ead6_dfbf5cac","line":301,"in_reply_to":"ca13fab6_d7429f5e","updated":"2024-02-13 08:54:00.000000000","message":"thanks, fixed.","commit_id":"ea2a9555db6a2343683dfe26c89c123c32acc97e"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"e2d7f8c00353ec86434f1cc50fca4ca6b3a10f6d","unresolved":true,"context_lines":[{"line_number":115,"context_line":"        tdelta \u003d timeutils.utcnow() - getattr(self, \u0027_clock_jump_canary\u0027,"},{"line_number":116,"context_line":"                                              timeutils.utcnow())"},{"line_number":117,"context_line":"        if tdelta.total_seconds() \u003e cfg.CONF.agent_down_time:"},{"line_number":118,"context_line":"            LOG.warning(\"Time since last %s reschedule check has \""},{"line_number":119,"context_line":"                        \"exceeded the interval between checks. Waiting \""},{"line_number":120,"context_line":"                        \"before check to allow agents to send a heartbeat \""},{"line_number":121,"context_line":"                        \"in case there was a clock adjustment.\","}],"source_content_type":"text/x-python","patch_set":3,"id":"cbd4834e_3fc58628","line":118,"updated":"2024-02-16 20:14:52.000000000","message":"This code was moved to this common location from the L3 scheduler in this review:\n\nhttps://review.opendev.org/c/openstack/neutron/+/131150\n\nI don\u0027t think there was ever the intention to make it accept an agent_type constant, just a string. As there are no other callers except DHCP and L3 I don\u0027t see the need to change it as it is not fixing any issue. I guess I didn\u0027t make that clear in my initial review.","commit_id":"da53a55bece9a4e747afdd722f2796fcc84526b6"},{"author":{"_account_id":14525,"name":"Vasyl Saienko","email":"vsaienko@mirantis.com","username":"vsaienko"},"change_message_id":"2539d70093cd81492b1428101a64ae2f4ba7020f","unresolved":true,"context_lines":[{"line_number":115,"context_line":"        tdelta \u003d timeutils.utcnow() - getattr(self, \u0027_clock_jump_canary\u0027,"},{"line_number":116,"context_line":"                                              timeutils.utcnow())"},{"line_number":117,"context_line":"        if tdelta.total_seconds() \u003e cfg.CONF.agent_down_time:"},{"line_number":118,"context_line":"            LOG.warning(\"Time since last %s reschedule check has \""},{"line_number":119,"context_line":"                        \"exceeded the interval between checks. Waiting \""},{"line_number":120,"context_line":"                        \"before check to allow agents to send a heartbeat \""},{"line_number":121,"context_line":"                        \"in case there was a clock adjustment.\","}],"source_content_type":"text/x-python","patch_set":3,"id":"d089a525_a15321e5","line":118,"in_reply_to":"cbd4834e_3fc58628","updated":"2024-02-17 08:55:23.000000000","message":"The functions has an argument `agent_type` but actually this is free form string that is just a part of warning message. This patch actualize the content of agent_type variable to be actually what we use everywhere for agent_type. Also we do print agent type from neutron_lib  constants in API /agents/\u003c\u003e/ endpoint. Why we should not use same when printing warning? \nhttps://github.com/openstack/neutron/blob/df13cbd1182473e34483b555e5a68c623e8fdadb/neutron/db/agentschedulers_db.py#L131\nhttps://github.com/openstack/neutron/blob/df13cbd1182473e34483b555e5a68c623e8fdadb/neutron/db/agentschedulers_db.py#L111","commit_id":"da53a55bece9a4e747afdd722f2796fcc84526b6"},{"author":{"_account_id":14525,"name":"Vasyl Saienko","email":"vsaienko@mirantis.com","username":"vsaienko"},"change_message_id":"c4fe48b70f36f8f31905f7ee37cf6c0bf46c74aa","unresolved":true,"context_lines":[{"line_number":115,"context_line":"        tdelta \u003d timeutils.utcnow() - getattr(self, \u0027_clock_jump_canary\u0027,"},{"line_number":116,"context_line":"                                              timeutils.utcnow())"},{"line_number":117,"context_line":"        if tdelta.total_seconds() \u003e cfg.CONF.agent_down_time:"},{"line_number":118,"context_line":"            LOG.warning(\"Time since last %s reschedule check has \""},{"line_number":119,"context_line":"                        \"exceeded the interval between checks. Waiting \""},{"line_number":120,"context_line":"                        \"before check to allow agents to send a heartbeat \""},{"line_number":121,"context_line":"                        \"in case there was a clock adjustment.\","}],"source_content_type":"text/x-python","patch_set":3,"id":"271711ba_c8a0e049","line":118,"in_reply_to":"cee78043_78cc9e87","updated":"2024-02-17 20:32:44.000000000","message":"thanks for review Brian, lets see other opinions I appreciate your vote.","commit_id":"da53a55bece9a4e747afdd722f2796fcc84526b6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"5d3114631ccb0f08249251df1ce0829373066cfe","unresolved":true,"context_lines":[{"line_number":115,"context_line":"        tdelta \u003d timeutils.utcnow() - getattr(self, \u0027_clock_jump_canary\u0027,"},{"line_number":116,"context_line":"                                              timeutils.utcnow())"},{"line_number":117,"context_line":"        if tdelta.total_seconds() \u003e cfg.CONF.agent_down_time:"},{"line_number":118,"context_line":"            LOG.warning(\"Time since last %s reschedule check has \""},{"line_number":119,"context_line":"                        \"exceeded the interval between checks. Waiting \""},{"line_number":120,"context_line":"                        \"before check to allow agents to send a heartbeat \""},{"line_number":121,"context_line":"                        \"in case there was a clock adjustment.\","}],"source_content_type":"text/x-python","patch_set":3,"id":"cee78043_78cc9e87","line":118,"in_reply_to":"d089a525_a15321e5","updated":"2024-02-17 15:43:56.000000000","message":"This has not been an issue in the 9 years since the change that introduced it merged, agent_type was maybe just a poor choice in a variable name but nothing else. The fact I had to find the constants in neutron-lib to notice there was going to be a double \"agent\" printed shows this will only make it more confusing.\n\nIs there a bug this is fixing? Changing code without a reason can sometimes just introduce backport issues.","commit_id":"da53a55bece9a4e747afdd722f2796fcc84526b6"}],"neutron/db/l3_agentschedulers_db.py":[{"author":{"_account_id":14525,"name":"Vasyl Saienko","email":"vsaienko@mirantis.com","username":"vsaienko"},"change_message_id":"806966bf550ba9107b11178029c25ef864a0c8fd","unresolved":true,"context_lines":[{"line_number":63,"context_line":"    def reschedule_routers_from_down_agents(self):"},{"line_number":64,"context_line":"        \"\"\"Reschedule routers from down l3 agents if admin state is up.\"\"\""},{"line_number":65,"context_line":"        self.reschedule_resources_from_down_agents("},{"line_number":66,"context_line":"            agent_type\u003dconstants.AGENT_TYPE_L3,"},{"line_number":67,"context_line":"            get_down_bindings\u003dself.get_down_router_bindings,"},{"line_number":68,"context_line":"            agent_id_attr\u003d\u0027l3_agent_id\u0027,"},{"line_number":69,"context_line":"            resource_id_attr\u003d\u0027router_id\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"4dcbec3e_983b9bc9","line":66,"updated":"2024-02-13 08:54:35.000000000","message":"now this parameter actually accepts what it should according to its name.","commit_id":"af7069145b803e156c59efe9700969d4b4f887e8"}]}