)]}'
{"/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":"9744fb3deaaecbd2d81b4721035fa7ff5dc003fb","unresolved":true,"context_lines":[{"line_number":11,"context_line":"such that the agent can continue its operation for possibly"},{"line_number":12,"context_line":"other haproxy processes"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"rhbz: #2229779"},{"line_number":15,"context_line":"Change-Id: I6da1b135c83ecfc41ec91e907ebf8500325a7a80"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"f7bd1e61_b78320b7","line":14,"range":{"start_line":14,"start_character":0,"end_line":14,"end_character":14},"updated":"2023-08-22 16:04:20.000000000","message":"Do you have a launchpad bug for it perhaps? This bug is open as I see, but no link is added to the commit msg","commit_id":"e6f4b3305dce1a1781bef304561cbef584af5e75"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"00988fe38028c60e1f773d1a52ff2ad5427d7000","unresolved":false,"context_lines":[{"line_number":11,"context_line":"such that the agent can continue its operation for possibly"},{"line_number":12,"context_line":"other haproxy processes"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"rhbz: #2229779"},{"line_number":15,"context_line":"Change-Id: I6da1b135c83ecfc41ec91e907ebf8500325a7a80"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"c6d9b6a6_27f4d7e2","line":14,"range":{"start_line":14,"start_character":0,"end_line":14,"end_character":14},"in_reply_to":"f7bd1e61_b78320b7","updated":"2023-08-24 17:47:25.000000000","message":"Done","commit_id":"e6f4b3305dce1a1781bef304561cbef584af5e75"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"204aebdcabe210f50ac4fa3c740df7557132cacf","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Miro Tomaska \u003cmtomaska@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2023-08-24 13:44:41 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"OVN Metadata handle process execeptions"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"OVN metadata agent should handle process exeptions incase"},{"line_number":10,"context_line":"an exeption happens when spawning haproxy processes"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"e34391fc_0c82752c","line":7,"range":{"start_line":7,"start_character":28,"end_line":7,"end_character":39},"updated":"2023-08-24 20:19:22.000000000","message":"nit: exceptions, same below twice","commit_id":"bb62d1bbb772e55831fe4fda0e6a358026ef5707"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"b99868beb753679f43785dada6a7f0ddb33b7f0b","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Miro Tomaska \u003cmtomaska@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2023-08-24 13:44:41 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"OVN Metadata handle process execeptions"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"OVN metadata agent should handle process exeptions incase"},{"line_number":10,"context_line":"an exeption happens when spawning haproxy processes"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"47a02f9a_d0c2c7ca","line":7,"range":{"start_line":7,"start_character":28,"end_line":7,"end_character":39},"in_reply_to":"e34391fc_0c82752c","updated":"2023-08-28 16:50:10.000000000","message":"Done","commit_id":"bb62d1bbb772e55831fe4fda0e6a358026ef5707"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"204aebdcabe210f50ac4fa3c740df7557132cacf","unresolved":true,"context_lines":[{"line_number":11,"context_line":"such that the agent can continue its operation for possibly"},{"line_number":12,"context_line":"other haproxy processes"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"rhbz: https://bugzilla.redhat.com/show_bug.cgi?id\u003d2229779"},{"line_number":15,"context_line":"Change-Id: I6da1b135c83ecfc41ec91e907ebf8500325a7a80"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"2a0d851d_6167f1e6","line":14,"updated":"2023-08-24 20:19:22.000000000","message":"Can you also open a launchpad bug and link it here? Seems like something we will want to backport.","commit_id":"bb62d1bbb772e55831fe4fda0e6a358026ef5707"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"b99868beb753679f43785dada6a7f0ddb33b7f0b","unresolved":false,"context_lines":[{"line_number":11,"context_line":"such that the agent can continue its operation for possibly"},{"line_number":12,"context_line":"other haproxy processes"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"rhbz: https://bugzilla.redhat.com/show_bug.cgi?id\u003d2229779"},{"line_number":15,"context_line":"Change-Id: I6da1b135c83ecfc41ec91e907ebf8500325a7a80"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"097b7155_6161b6cd","line":14,"in_reply_to":"2a0d851d_6167f1e6","updated":"2023-08-28 16:50:10.000000000","message":"Done","commit_id":"bb62d1bbb772e55831fe4fda0e6a358026ef5707"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"41c790421ffea56ba889a16a58898dd2fcf7eeab","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"OVN Metadata handle process exceptions"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"OVN metadata agent should handle process exceptions incase"},{"line_number":10,"context_line":"an exception happens when spawning haproxy processes"},{"line_number":11,"context_line":"such that the agent can continue its operation for possibly"},{"line_number":12,"context_line":"other haproxy processes"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"7cd505e1_34ed91b5","line":9,"range":{"start_line":9,"start_character":52,"end_line":9,"end_character":58},"updated":"2023-09-06 16:36:16.000000000","message":"typo?","commit_id":"409a664935fe40cbe7030aca22bc579347b34981"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"182bc7c1f427b76e055a660e71853ca1d4fbc971","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"OVN Metadata handle process exceptions"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"OVN metadata agent should handle process exceptions incase"},{"line_number":10,"context_line":"an exception happens when spawning haproxy processes"},{"line_number":11,"context_line":"such that the agent can continue its operation for possibly"},{"line_number":12,"context_line":"other haproxy processes"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"8f5cfb0c_2e4ff242","line":9,"range":{"start_line":9,"start_character":52,"end_line":9,"end_character":58},"in_reply_to":"7cd505e1_34ed91b5","updated":"2023-09-19 15:13:17.000000000","message":"Done","commit_id":"409a664935fe40cbe7030aca22bc579347b34981"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"54540a6831e1507cd14c4f6ed75d6299f1525100","unresolved":true,"context_lines":[{"line_number":11,"context_line":"such that the agent can continue its operation for possibly"},{"line_number":12,"context_line":"other haproxy processes"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"rhbz: https://bugzilla.redhat.com/show_bug.cgi?id\u003d2229779"},{"line_number":15,"context_line":"Change-Id: I6da1b135c83ecfc41ec91e907ebf8500325a7a80"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"e87e8912_51ec247d","line":14,"updated":"2023-09-19 19:07:50.000000000","message":"Has this been changed from linking the LP bug 2033305 to rhbz on purpose?","commit_id":"81bbe2e0ed20c85bb0e00fd4c1fd7390366d970e"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"b4d1bcb8643f2f33640f6b7b63d23bda674c2478","unresolved":false,"context_lines":[{"line_number":11,"context_line":"such that the agent can continue its operation for possibly"},{"line_number":12,"context_line":"other haproxy processes"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"rhbz: https://bugzilla.redhat.com/show_bug.cgi?id\u003d2229779"},{"line_number":15,"context_line":"Change-Id: I6da1b135c83ecfc41ec91e907ebf8500325a7a80"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"b4dac96a_e62c1874","line":14,"in_reply_to":"e87e8912_51ec247d","updated":"2023-09-20 14:50:07.000000000","message":"Done","commit_id":"81bbe2e0ed20c85bb0e00fd4c1fd7390366d970e"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"e87360ca672ec39d0140bc2627c4758f2d9a182f","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Brian Haley \u003chaleyb.dev@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2023-10-27 12:21:35 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Metadata: handle process execeptions"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Both metadata agents (OVN and non-OVN) should handle"},{"line_number":10,"context_line":"process exceptions when spawning haproxy processes"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"c4015b0a_28571292","line":7,"range":{"start_line":7,"start_character":25,"end_line":7,"end_character":36},"updated":"2023-10-27 17:31:31.000000000","message":"If we respin should fix my typo","commit_id":"3e67b3c4b59f91ce572992dad59d356d61e39dc1"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"d30d65142bb13fb27e20c94bf4a11b3a8446de4b","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Brian Haley \u003chaleyb.dev@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2023-10-27 12:21:35 -0400"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Metadata: handle process execeptions"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Both metadata agents (OVN and non-OVN) should handle"},{"line_number":10,"context_line":"process exceptions when spawning haproxy processes"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"33d32919_6b0115e3","line":7,"range":{"start_line":7,"start_character":25,"end_line":7,"end_character":36},"in_reply_to":"c4015b0a_28571292","updated":"2023-11-29 20:23:30.000000000","message":"Done","commit_id":"3e67b3c4b59f91ce572992dad59d356d61e39dc1"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"7a24dbc7428bb6a891f3d455b3f30bd78e376067","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Both metadata agents (OVN and non-OVN) should handle"},{"line_number":10,"context_line":"process exceptions when spawning haproxy processes"},{"line_number":11,"context_line":"such that the agent can continue its operation for"},{"line_number":12,"context_line":"other haproxy processes."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Closes-Bug: #2033305"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"fce90c4c_fcfef015","line":11,"range":{"start_line":11,"start_character":24,"end_line":11,"end_character":32},"updated":"2023-11-30 18:27:21.000000000","message":"that\u0027s a noble goal.\n\nI wonder: what happens with the failed metadata? will we retry later, or will metadata stay broken until the router / network get another update?","commit_id":"ed0515737be75745933f4994b5e6b652f9e0a2be"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"9ca15280337583b900b8e086449e126a267d9f34","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Both metadata agents (OVN and non-OVN) should handle"},{"line_number":10,"context_line":"process exceptions when spawning haproxy processes"},{"line_number":11,"context_line":"such that the agent can continue its operation for"},{"line_number":12,"context_line":"other haproxy processes."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Closes-Bug: #2033305"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"b4305ab7_6a828e89","line":11,"range":{"start_line":11,"start_character":24,"end_line":11,"end_character":32},"in_reply_to":"fce90c4c_fcfef015","updated":"2023-12-05 14:46:52.000000000","message":"Yes, this is just general exception handling. We just log any process exception which could happen when spawning a process. We just want to prevent metadata agent from stopping completely if there was an exception and at least continue to process other work that is the agent is responsible for.\nIf the exception is just one off exception then metadata agent will try to start the process again(either because of agent restart, or some qualifying DB event). I imagine if there is some persistant issue on the system then the user will just find bunch of logs every time the agent tried to start the problematic process. Hope is that the log will provide enough info for the user/developer to debug the system further.","commit_id":"ed0515737be75745933f4994b5e6b652f9e0a2be"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"2195b4ef3ff173c5d9b766d20ebeb7fdaebbea55","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"8a0109ea_c3886cf5","updated":"2023-08-21 13:20:04.000000000","message":"recheck rally job should be now fixed","commit_id":"5ab44c52bbd62997b0f320c1f4b6baaac85add0f"},{"author":{"_account_id":21798,"name":"Bernard Cafarelli","email":"bcafarel@redhat.com","username":"bcafarel"},"change_message_id":"fa974d7c6c2af19ea43dec5ea988e3c4fcf56cb7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"fbcfdd3a_2f047445","updated":"2023-08-21 13:18:23.000000000","message":"recheck rally nova: not found","commit_id":"5ab44c52bbd62997b0f320c1f4b6baaac85add0f"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"79895a1e6b5677e5a0b595fb386e65f1efbcc714","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"ee4786cc_d9dba4b2","updated":"2023-08-22 19:12:10.000000000","message":"I think you should add a test for this throwing an exception.","commit_id":"e6f4b3305dce1a1781bef304561cbef584af5e75"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"41c790421ffea56ba889a16a58898dd2fcf7eeab","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"cda3b230_0688f60a","updated":"2023-09-06 16:36:16.000000000","message":"Looking at the ProcessMonitor, I don\u0027t think this is correct. The ProcessMonitor is able to handle case where the monitored process already exists. Looking at the linked BZ from the LP bug it looks like a TripleO related problem where haproxy can\u0027t be spawned because the side-car container already exists. I\u0027m wondering if the right fix shouldn\u0027t be by adding \"--rm\" parameter to the \"podman run\" command. Putting -1 tentatively until we understand it better.","commit_id":"409a664935fe40cbe7030aca22bc579347b34981"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"e74100c1d0bb91591793073a35664626d2ed3b81","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"3be63239_7a57b219","in_reply_to":"5abbb717_030936f7","updated":"2023-09-13 18:32:43.000000000","message":"FYI. Proposed fix for the specific \"container already exists\" exception\nhttps://review.opendev.org/c/openstack/puppet-tripleo/+/894931","commit_id":"409a664935fe40cbe7030aca22bc579347b34981"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"2b5d1a19925abd87d2e1dbb0de599f886ea80c4a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"5abbb717_030936f7","in_reply_to":"cda3b230_0688f60a","updated":"2023-09-06 18:25:10.000000000","message":"Hi. As we discussed offline. I think the `-rm` is a very good idea, I was not aware of it, it would certainly take care of the specific scenario from this bug. I.e. haproxy wrapper trying to start a container that is already running with that name. In this BZ the process was actually in this weird state where `pm.enable` was returning False but the container was still running. So in general, I think it\u0027s a good idea to keep this patch for general error handling. Many things can happen when a process is getting started and the agent should be able to handle it. I think this patch is still good to have.\n\nI will follow up with another patch in tripleo for adding the `-rm` argument into the podman run in the haproxy wrapper.","commit_id":"409a664935fe40cbe7030aca22bc579347b34981"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"aea58e15585ef7f3aea0fb887fba03b279ba4a4b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"f6336ad5_2c09b7ed","updated":"2023-09-26 18:38:17.000000000","message":"Question, otherwise LGTM","commit_id":"502df507ed7c15d67e38776f54d55f90ba6395a7"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"e504396794efc8501b43ccb94a5497981f62e5da","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"fb7d556d_6178471b","updated":"2023-10-04 21:25:18.000000000","message":"So this seems like it would apply to the non-OVN metadata driver as well?\n\nThis is another reason I want to make a common class so we don\u0027t continue to diverge (sorry, plugging my own patch).\n\n  https://review.opendev.org/c/openstack/neutron/+/894399\n\nMy other comment is more at the number of changes to the metadata agents and drivers in flight, I think there are four different changes? We should maybe pick one and get it done, or even put them in an order since there will be a number of rebases. The IPv6 one should maybe be the first?\n\n  https://review.opendev.org/c/openstack/neutron/+/894026","commit_id":"dab794c8bd20ad3f14f0e7d778057e83dcc95a35"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"ffceb01fb9c8a58a08654112ef123dd0aa632d72","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"1565078d_796746ca","updated":"2023-10-04 20:43:39.000000000","message":"Thanks!","commit_id":"dab794c8bd20ad3f14f0e7d778057e83dcc95a35"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"7dcd3e81ab451b8317b7e8fdbdaa7286a1e3ecb1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"f6365ed9_fd991df8","in_reply_to":"fb7d556d_6178471b","updated":"2023-10-06 03:03:50.000000000","message":"+1 on getting common code base for the agents. I also agree with getting the ovn agent ipv6 support in first[1]. Then, lets get this review[2] in since its ready to be merged. Then complete the the common code base patch[3]. This change(890986) and this [4] wip change can go after[3] or be done as part of [3] (if it makes sense). Lastly, this change[5] can be just rebased depending when its ready to get merged. What do you think? I see you have [3] as WIP. Do you think you will have time to complete it anytime soon? Can I offer help on [3]? \n\n\n[1] https://review.opendev.org/c/openstack/neutron/+/894026\n[2] https://review.opendev.org/c/openstack/neutron/+/890339\n[3] https://review.opendev.org/c/openstack/neutron/+/894399\n[4] https://review.opendev.org/c/openstack/neutron/+/893966\n[5] https://review.opendev.org/c/openstack/neutron/+/895402","commit_id":"dab794c8bd20ad3f14f0e7d778057e83dcc95a35"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"1a562258cf0af6c115824bf606ee5a194106fc90","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"1dae2d89_0a3b4183","updated":"2023-10-27 01:01:45.000000000","message":"Updated based on PTG discussions to also add try/except to non-OVN agent as well.","commit_id":"c9e890ed237d69471e7903bb7af288ce0a9198c6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"b633299d73c1b99e354e02aa84857aaed42475a1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"fbbeab93_ee933194","updated":"2023-10-29 04:05:00.000000000","message":"recheck timed out","commit_id":"16f79c450607e4d893e48a690a43c1f21e3573da"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"b9b64670c8a24daa20a0f28c3f8e9cccf2965435","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"8a9ca1f8_aad9c63d","updated":"2023-10-29 00:14:44.000000000","message":"recheck unrelated fullstack failure","commit_id":"16f79c450607e4d893e48a690a43c1f21e3573da"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"d71bb243c11f423c035d6c6def55862491ad8bc6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"982f7385_16d05d8b","updated":"2023-10-30 20:33:01.000000000","message":"recheck unrelated timeouts","commit_id":"16f79c450607e4d893e48a690a43c1f21e3573da"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"4f3eb50046b2756feb1d3ad206b5683e77e32824","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":20,"id":"5f14b7b6_e1f0889f","updated":"2023-11-07 23:46:28.000000000","message":"recheck sqlalchemy test non-voting now","commit_id":"853191d8f05a90635414afed2cf48a939cb13ab0"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"5099f19238faca1af34af2f6ee63af22c70e9985","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":22,"id":"3c30d59c_90517d4c","updated":"2023-11-21 13:52:32.000000000","message":"The overall patch makes sense to me. Just one question in the tests.","commit_id":"c2820f1628f1688385513d5205042fc0ad7c20f6"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"ff773024694c7f948bf678eeb4ceccffaef884f9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"460165ac_f7a6a13d","updated":"2023-12-05 18:08:52.000000000","message":"I think this is good to go, should be merged before https://review.opendev.org/c/openstack/neutron/+/894399","commit_id":"ed0515737be75745933f4994b5e6b652f9e0a2be"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"c0916070632cf2eeba0bd3d7201d7ca2918a2e88","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"ec37cb7b_b654d3d4","updated":"2023-12-05 20:54:11.000000000","message":"It should be a simple rebase for the other change Kuba mentioned.","commit_id":"ed0515737be75745933f4994b5e6b652f9e0a2be"}],"neutron/agent/metadata/driver.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"8fc9b32cbec491d021c933cf1096264da3490647","unresolved":true,"context_lines":[{"line_number":280,"context_line":"        callback \u003d cls._get_metadata_proxy_callback("},{"line_number":281,"context_line":"            bind_address, port, conf,"},{"line_number":282,"context_line":"            network_id\u003dnetwork_id, router_id\u003drouter_id,"},{"line_number":283,"context_line":"            bind_address_v6\u003dbind_address_v6, bind_interface\u003dbind_interface)"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"        try:"},{"line_number":286,"context_line":"            pm.enable(cmd_callback\u003dcallback)"}],"source_content_type":"text/x-python","patch_set":17,"id":"bd8f2128_2e2ef563","line":283,"updated":"2023-10-27 15:42:07.000000000","message":"So moving this change after creating the process manager (and not passing as the callback argument) is causing the functional test failures, and I\u0027m not sure why it\u0027s necessary. The callback won\u0027t be called unless pm.enable() is called, so it should be Ok to do it first. I\u0027m going to move it back in both drivers.","commit_id":"c9e890ed237d69471e7903bb7af288ce0a9198c6"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"23600482b83067ca27007b9937059c3d064ca2a3","unresolved":false,"context_lines":[{"line_number":280,"context_line":"        callback \u003d cls._get_metadata_proxy_callback("},{"line_number":281,"context_line":"            bind_address, port, conf,"},{"line_number":282,"context_line":"            network_id\u003dnetwork_id, router_id\u003drouter_id,"},{"line_number":283,"context_line":"            bind_address_v6\u003dbind_address_v6, bind_interface\u003dbind_interface)"},{"line_number":284,"context_line":""},{"line_number":285,"context_line":"        try:"},{"line_number":286,"context_line":"            pm.enable(cmd_callback\u003dcallback)"}],"source_content_type":"text/x-python","patch_set":17,"id":"3690da1b_be9ee369","line":283,"in_reply_to":"bd8f2128_2e2ef563","updated":"2023-10-27 16:22:42.000000000","message":"Done","commit_id":"c9e890ed237d69471e7903bb7af288ce0a9198c6"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"50059e803c91ba81cfca5f0328fdb83ed17cc387","unresolved":true,"context_lines":[{"line_number":278,"context_line":"        pm \u003d cls._get_metadata_proxy_process_manager(uuid, conf,"},{"line_number":279,"context_line":"                                                     ns_name\u003dns_name,"},{"line_number":280,"context_line":"                                                     callback\u003dcallback)"},{"line_number":281,"context_line":"        if pm.active:"},{"line_number":282,"context_line":"            return"},{"line_number":283,"context_line":"        try:"},{"line_number":284,"context_line":"            pm.enable()"},{"line_number":285,"context_line":"        except exceptions.ProcessExecutionError as exec_err:"}],"source_content_type":"text/x-python","patch_set":22,"id":"ae16c78a_dd5794be","line":282,"range":{"start_line":281,"start_character":0,"end_line":282,"end_character":18},"updated":"2023-11-21 02:56:11.000000000","message":"this check looks redundant. The enable method should do nothing in case the process is active (and the other arguments are not passed)\n\nhttps://github.com/openstack/neutron/blob/d853996d8717487aeaf19fbf9c948cdbafe344e7/neutron/agent/linux/external_process.py#L87","commit_id":"c2820f1628f1688385513d5205042fc0ad7c20f6"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"d30d65142bb13fb27e20c94bf4a11b3a8446de4b","unresolved":true,"context_lines":[{"line_number":278,"context_line":"        pm \u003d cls._get_metadata_proxy_process_manager(uuid, conf,"},{"line_number":279,"context_line":"                                                     ns_name\u003dns_name,"},{"line_number":280,"context_line":"                                                     callback\u003dcallback)"},{"line_number":281,"context_line":"        if pm.active:"},{"line_number":282,"context_line":"            return"},{"line_number":283,"context_line":"        try:"},{"line_number":284,"context_line":"            pm.enable()"},{"line_number":285,"context_line":"        except exceptions.ProcessExecutionError as exec_err:"}],"source_content_type":"text/x-python","patch_set":22,"id":"b53da0f4_12d5386c","line":282,"range":{"start_line":281,"start_character":0,"end_line":282,"end_character":18},"in_reply_to":"ae16c78a_dd5794be","updated":"2023-11-29 20:23:30.000000000","message":"That is correct, it was here to prevent unnecessary call to `cls._get_metadata_proxy_process_manager` which creates haproxy config file. If the `pm.active` is already active then we can prevent that call(it was just a small optimization). But Looking at the history, in patch 17-\u003e18[1] it got removed and thus yes it makes the current implementation redundant. \n\nI am going to see what functional test issue it was causing, to see if its worth fixing the test or removing this line from the code.\n[1] https://review.opendev.org/c/openstack/neutron/+/890986/17/neutron/agent/metadata/driver.py#283","commit_id":"c2820f1628f1688385513d5205042fc0ad7c20f6"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"3ff43cf640332df2fa578aca20714b398cca1f5c","unresolved":false,"context_lines":[{"line_number":278,"context_line":"        pm \u003d cls._get_metadata_proxy_process_manager(uuid, conf,"},{"line_number":279,"context_line":"                                                     ns_name\u003dns_name,"},{"line_number":280,"context_line":"                                                     callback\u003dcallback)"},{"line_number":281,"context_line":"        if pm.active:"},{"line_number":282,"context_line":"            return"},{"line_number":283,"context_line":"        try:"},{"line_number":284,"context_line":"            pm.enable()"},{"line_number":285,"context_line":"        except exceptions.ProcessExecutionError as exec_err:"}],"source_content_type":"text/x-python","patch_set":22,"id":"e1753054_ef56595b","line":282,"range":{"start_line":281,"start_character":0,"end_line":282,"end_character":18},"in_reply_to":"b53da0f4_12d5386c","updated":"2023-11-29 21:20:23.000000000","message":"Done","commit_id":"c2820f1628f1688385513d5205042fc0ad7c20f6"}],"neutron/agent/ovn/metadata/driver.py":[{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"3576382b3830d66d3a19a29ec171c21a3eeb6e66","unresolved":true,"context_lines":[{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        pm \u003d cls._get_metadata_proxy_process_manager(uuid, conf,"},{"line_number":190,"context_line":"                                                     ns_name\u003dns_name)"},{"line_number":191,"context_line":"        if pm.active:"},{"line_number":192,"context_line":"            return"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"        callback \u003d cls._get_metadata_proxy_callback("}],"source_content_type":"text/x-python","patch_set":3,"id":"6237bcc6_40f51e69","line":191,"updated":"2023-08-16 17:00:47.000000000","message":"Just noticed this solution is not the best. Since the enable function will perform similar `is active` check. [1] I think maybe catching error would be the best\nhttps://opendev.org/openstack/neutron/src/commit/186e87e3894e8ac1151425f93d51b819002c02cb/neutron/agent/linux/external_process.py#L87","commit_id":"6fb6ee0f26f520f15d89dd1066c51e4ad6d81135"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"2a4b07158d0f82ab7c967af961bae93049cf7b91","unresolved":false,"context_lines":[{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        pm \u003d cls._get_metadata_proxy_process_manager(uuid, conf,"},{"line_number":190,"context_line":"                                                     ns_name\u003dns_name)"},{"line_number":191,"context_line":"        if pm.active:"},{"line_number":192,"context_line":"            return"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"        callback \u003d cls._get_metadata_proxy_callback("}],"source_content_type":"text/x-python","patch_set":3,"id":"610920e2_58df3a1f","line":191,"in_reply_to":"6237bcc6_40f51e69","updated":"2023-08-16 17:38:10.000000000","message":"Done","commit_id":"6fb6ee0f26f520f15d89dd1066c51e4ad6d81135"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"79895a1e6b5677e5a0b595fb386e65f1efbcc714","unresolved":true,"context_lines":[{"line_number":200,"context_line":"            cls.monitors[router_id] \u003d pm"},{"line_number":201,"context_line":"        except exceptions.ProcessExecutionError as exec_err:"},{"line_number":202,"context_line":"            LOG.error(\"Encountered process execution error %(err)s while \""},{"line_number":203,"context_line":"                      \"starting process on namespace %(ns)s\","},{"line_number":204,"context_line":"                      {\u0027err\u0027: exec_err, \"ns\": ns_name})"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"324e5169_4ead9ace","line":203,"range":{"start_line":203,"start_character":40,"end_line":203,"end_character":42},"updated":"2023-08-22 19:12:10.000000000","message":"s/in","commit_id":"e6f4b3305dce1a1781bef304561cbef584af5e75"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"00988fe38028c60e1f773d1a52ff2ad5427d7000","unresolved":false,"context_lines":[{"line_number":200,"context_line":"            cls.monitors[router_id] \u003d pm"},{"line_number":201,"context_line":"        except exceptions.ProcessExecutionError as exec_err:"},{"line_number":202,"context_line":"            LOG.error(\"Encountered process execution error %(err)s while \""},{"line_number":203,"context_line":"                      \"starting process on namespace %(ns)s\","},{"line_number":204,"context_line":"                      {\u0027err\u0027: exec_err, \"ns\": ns_name})"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"8c7973da_347da3d7","line":203,"range":{"start_line":203,"start_character":40,"end_line":203,"end_character":42},"in_reply_to":"324e5169_4ead9ace","updated":"2023-08-24 17:47:25.000000000","message":"Done","commit_id":"e6f4b3305dce1a1781bef304561cbef584af5e75"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"8bc6c7299b821791ad3970d616823ab437637562","unresolved":true,"context_lines":[{"line_number":201,"context_line":"        except exceptions.ProcessExecutionError as exec_err:"},{"line_number":202,"context_line":"            LOG.error(\"Encountered process execution error %(err)s while \""},{"line_number":203,"context_line":"                      \"starting process on namespace %(ns)s\","},{"line_number":204,"context_line":"                      {\u0027err\u0027: exec_err, \"ns\": ns_name})"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    @classmethod"},{"line_number":207,"context_line":"    def destroy_monitored_metadata_proxy(cls, monitor, uuid, conf, ns_name):"}],"source_content_type":"text/x-python","patch_set":6,"id":"26d30423_ff7ddd48","line":204,"range":{"start_line":204,"start_character":23,"end_line":204,"end_character":28},"updated":"2023-08-22 15:09:29.000000000","message":"nit consistency. either \" or \u0027","commit_id":"e6f4b3305dce1a1781bef304561cbef584af5e75"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"00988fe38028c60e1f773d1a52ff2ad5427d7000","unresolved":false,"context_lines":[{"line_number":201,"context_line":"        except exceptions.ProcessExecutionError as exec_err:"},{"line_number":202,"context_line":"            LOG.error(\"Encountered process execution error %(err)s while \""},{"line_number":203,"context_line":"                      \"starting process on namespace %(ns)s\","},{"line_number":204,"context_line":"                      {\u0027err\u0027: exec_err, \"ns\": ns_name})"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"    @classmethod"},{"line_number":207,"context_line":"    def destroy_monitored_metadata_proxy(cls, monitor, uuid, conf, ns_name):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3dd6fba7_ae211c3b","line":204,"range":{"start_line":204,"start_character":23,"end_line":204,"end_character":28},"in_reply_to":"26d30423_ff7ddd48","updated":"2023-08-24 17:47:25.000000000","message":"Done","commit_id":"e6f4b3305dce1a1781bef304561cbef584af5e75"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"069b180332f86f78b44d548cf8c92776d80e7c92","unresolved":true,"context_lines":[{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        pm \u003d cls._get_metadata_proxy_process_manager(uuid, conf,"},{"line_number":190,"context_line":"                                                     ns_name\u003dns_name)"},{"line_number":191,"context_line":"        if pm.active:"},{"line_number":192,"context_line":"            return"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"        callback \u003d cls._get_metadata_proxy_callback("},{"line_number":195,"context_line":"            bind_address, port, conf, network_id\u003dnetwork_id,"}],"source_content_type":"text/x-python","patch_set":8,"id":"dbcb8355_ec475a2d","line":192,"range":{"start_line":191,"start_character":8,"end_line":192,"end_character":18},"updated":"2023-08-29 07:45:37.000000000","message":"Why the metadata service should be active?\nIf it is already active, what configuration is using?\nIn this case, is \"cls.monitors[router_id] \u003d pm\" correctly populated?","commit_id":"409a664935fe40cbe7030aca22bc579347b34981"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"22a2cf8d1b8114f35f234ba2a00157ecc42ba34a","unresolved":true,"context_lines":[{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        pm \u003d cls._get_metadata_proxy_process_manager(uuid, conf,"},{"line_number":190,"context_line":"                                                     ns_name\u003dns_name)"},{"line_number":191,"context_line":"        if pm.active:"},{"line_number":192,"context_line":"            return"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"        callback \u003d cls._get_metadata_proxy_callback("},{"line_number":195,"context_line":"            bind_address, port, conf, network_id\u003dnetwork_id,"}],"source_content_type":"text/x-python","patch_set":8,"id":"8353a610_a3f4f664","line":192,"range":{"start_line":191,"start_character":8,"end_line":192,"end_character":18},"in_reply_to":"1f380b64_e82ae085","updated":"2023-08-29 15:34:56.000000000","message":"*** The formating got messed up in the response above . Here it is again ***\n\nThis is for the case when the metadata agent is running and gets restarted but haproxies(processes) are already running. In this case `cls.monitors[\u0027router_id\u0027]` should be identical. \nIf you actually look at previous implementation and the `enable` function[1] I did not change this logic much. Except that now, I am checking pm.enable explicitly in this function. This way, I dont have to create a callback(and config file) if it is not needed. In the previous implementation, we always created a config, passed it to the constructor and then it was not used at all when the `enable` was called but service was active.\nHowever, If you think it would be safer to leave it as it was, then I could do that.\n\n[1] https://opendev.org/openstack/neutron/src/commit/03e07ade2ecb99a9e63b138622e3f3fcc35205d0/neutron/agent/linux/external_process.py#L86-L98","commit_id":"409a664935fe40cbe7030aca22bc579347b34981"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"c958c4673a88bfb8f84ccee70190f914ea13cb8c","unresolved":false,"context_lines":[{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        pm \u003d cls._get_metadata_proxy_process_manager(uuid, conf,"},{"line_number":190,"context_line":"                                                     ns_name\u003dns_name)"},{"line_number":191,"context_line":"        if pm.active:"},{"line_number":192,"context_line":"            return"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"        callback \u003d cls._get_metadata_proxy_callback("},{"line_number":195,"context_line":"            bind_address, port, conf, network_id\u003dnetwork_id,"}],"source_content_type":"text/x-python","patch_set":8,"id":"aebbbddb_c55591f9","line":192,"range":{"start_line":191,"start_character":8,"end_line":192,"end_character":18},"in_reply_to":"8353a610_a3f4f664","updated":"2023-09-06 14:23:48.000000000","message":"Ok, let\u0027s use this implementation then. We should keep an eye on the CI during the next weeks, to check if that is breaking something, that I don\u0027t expect.","commit_id":"409a664935fe40cbe7030aca22bc579347b34981"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"5f01f949ccd4c30a04d0c2974dcb852ed8d5f566","unresolved":true,"context_lines":[{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        pm \u003d cls._get_metadata_proxy_process_manager(uuid, conf,"},{"line_number":190,"context_line":"                                                     ns_name\u003dns_name)"},{"line_number":191,"context_line":"        if pm.active:"},{"line_number":192,"context_line":"            return"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"        callback \u003d cls._get_metadata_proxy_callback("},{"line_number":195,"context_line":"            bind_address, port, conf, network_id\u003dnetwork_id,"}],"source_content_type":"text/x-python","patch_set":8,"id":"1f380b64_e82ae085","line":192,"range":{"start_line":191,"start_character":8,"end_line":192,"end_character":18},"in_reply_to":"dbcb8355_ec475a2d","updated":"2023-08-29 15:29:56.000000000","message":"This is for the case when the metadata agent is running and gets restarted but haproxies(processes) are already running. In this case `cls.monitors[\u0027router_id\u0027]\" should be identical. \nIf you actually look at previous implementation and the `enable`[1] function I did not change this logic much. Except that now that I am checking `pm.enable` explicitly here. This way, I dont have to create a callback(and config file) if not needed. In previous implementation we always create config, passed it to constructor and then it was not used at all when the `enable` was called but service was active.\nHowever, If you think it would be safer to leave it as it was, then I could do that. NP\n\n\n[1] https://opendev.org/openstack/neutron/src/commit/03e07ade2ecb99a9e63b138622e3f3fcc35205d0/neutron/agent/linux/external_process.py#L86-L98","commit_id":"409a664935fe40cbe7030aca22bc579347b34981"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0c78b1767bc42349f1ed7c4002f1319e5393903c","unresolved":true,"context_lines":[{"line_number":198,"context_line":"            pm.enable(cmd_callback\u003dcallback)"},{"line_number":199,"context_line":"            monitor.register(uuid, METADATA_SERVICE_NAME, pm)"},{"line_number":200,"context_line":"            cls.monitors[router_id] \u003d pm"},{"line_number":201,"context_line":"        except exceptions.ProcessExecutionError as exec_err:"},{"line_number":202,"context_line":"            LOG.error(\"Encountered process execution error %(err)s while \""},{"line_number":203,"context_line":"                      \"starting process in namespace %(ns)s\","},{"line_number":204,"context_line":"                      {\"err\": exec_err, \"ns\": ns_name})"}],"source_content_type":"text/x-python","patch_set":9,"id":"05cff240_aeb5a8e0","line":201,"range":{"start_line":201,"start_character":8,"end_line":201,"end_character":14},"updated":"2023-09-18 14:32:25.000000000","message":"Should this wrap only pm.enable() ?","commit_id":"6ce6f40cd6f1fd25620f54fac2b8e2e74e61cf15"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"b3d526065fed338d766b695fd1a789bb7a1ce31d","unresolved":true,"context_lines":[{"line_number":198,"context_line":"            pm.enable(cmd_callback\u003dcallback)"},{"line_number":199,"context_line":"            monitor.register(uuid, METADATA_SERVICE_NAME, pm)"},{"line_number":200,"context_line":"            cls.monitors[router_id] \u003d pm"},{"line_number":201,"context_line":"        except exceptions.ProcessExecutionError as exec_err:"},{"line_number":202,"context_line":"            LOG.error(\"Encountered process execution error %(err)s while \""},{"line_number":203,"context_line":"                      \"starting process in namespace %(ns)s\","},{"line_number":204,"context_line":"                      {\"err\": exec_err, \"ns\": ns_name})"}],"source_content_type":"text/x-python","patch_set":9,"id":"e8ce9545_dfd3f90e","line":201,"range":{"start_line":201,"start_character":8,"end_line":201,"end_character":14},"in_reply_to":"05cff240_aeb5a8e0","updated":"2023-09-18 17:38:22.000000000","message":"if the `pm.enable` raises an exception then we dont want to call register `monitor.register(uuid, METADATA_SERVICE_NAME, pm)`, am I wrong? In the exception case we just want to log the exception and move on.\nThe `cls.monitors[router_id] \u003d pm` is a dead code as you already pointed out in your patch[1]. I am just trying to keep this patch in the scope of the bug so I am treating it as if the line did something 😊\n\n[1] https://review.opendev.org/c/openstack/neutron/+/893966","commit_id":"6ce6f40cd6f1fd25620f54fac2b8e2e74e61cf15"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"182bc7c1f427b76e055a660e71853ca1d4fbc971","unresolved":false,"context_lines":[{"line_number":198,"context_line":"            pm.enable(cmd_callback\u003dcallback)"},{"line_number":199,"context_line":"            monitor.register(uuid, METADATA_SERVICE_NAME, pm)"},{"line_number":200,"context_line":"            cls.monitors[router_id] \u003d pm"},{"line_number":201,"context_line":"        except exceptions.ProcessExecutionError as exec_err:"},{"line_number":202,"context_line":"            LOG.error(\"Encountered process execution error %(err)s while \""},{"line_number":203,"context_line":"                      \"starting process in namespace %(ns)s\","},{"line_number":204,"context_line":"                      {\"err\": exec_err, \"ns\": ns_name})"}],"source_content_type":"text/x-python","patch_set":9,"id":"26a9f7d3_9a2cd733","line":201,"range":{"start_line":201,"start_character":8,"end_line":201,"end_character":14},"in_reply_to":"76f8d897_c834218b","updated":"2023-09-19 15:13:17.000000000","message":"Done","commit_id":"6ce6f40cd6f1fd25620f54fac2b8e2e74e61cf15"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"8e5c6993b4d7f7506a962e611847d237563489a3","unresolved":true,"context_lines":[{"line_number":198,"context_line":"            pm.enable(cmd_callback\u003dcallback)"},{"line_number":199,"context_line":"            monitor.register(uuid, METADATA_SERVICE_NAME, pm)"},{"line_number":200,"context_line":"            cls.monitors[router_id] \u003d pm"},{"line_number":201,"context_line":"        except exceptions.ProcessExecutionError as exec_err:"},{"line_number":202,"context_line":"            LOG.error(\"Encountered process execution error %(err)s while \""},{"line_number":203,"context_line":"                      \"starting process in namespace %(ns)s\","},{"line_number":204,"context_line":"                      {\"err\": exec_err, \"ns\": ns_name})"}],"source_content_type":"text/x-python","patch_set":9,"id":"76f8d897_c834218b","line":201,"range":{"start_line":201,"start_character":8,"end_line":201,"end_character":14},"in_reply_to":"e8ce9545_dfd3f90e","updated":"2023-09-18 19:22:19.000000000","message":"I see what you mean. The try-catch context should be as minimal as possible because we use specific exception class here, you can either return in the except block or use else\n\n```\n  try:\n      pm.enable(cmd_callback\u003dcallback)\n  except exceptions.ProcessExecutionError as exec_err:\n      LOG.error(...)\n      return\n```      \nor \n```\n try:\n     pm.enable(cmd_callback\u003dcallback)\n except exceptions.ProcessExecutionError as exec_err:\n     LOG.error(...)\n else:\n     monitor.register(uuid, METADATA_SERVICE_NAME, pm)\n     cls.monitors[router_id] \u003d pm\n```\nthis way it\u0027s clear where the error is coming from and what to do if the enable() was successful","commit_id":"6ce6f40cd6f1fd25620f54fac2b8e2e74e61cf15"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"50059e803c91ba81cfca5f0328fdb83ed17cc387","unresolved":true,"context_lines":[{"line_number":191,"context_line":"        pm \u003d cls._get_metadata_proxy_process_manager(uuid, conf,"},{"line_number":192,"context_line":"                                                     ns_name\u003dns_name,"},{"line_number":193,"context_line":"                                                     callback\u003dcallback)"},{"line_number":194,"context_line":"        if pm.active:"},{"line_number":195,"context_line":"            return"},{"line_number":196,"context_line":"        try:"},{"line_number":197,"context_line":"            pm.enable()"},{"line_number":198,"context_line":"        except exceptions.ProcessExecutionError as exec_err:"}],"source_content_type":"text/x-python","patch_set":22,"id":"c77d7a48_205cf417","line":195,"range":{"start_line":194,"start_character":0,"end_line":195,"end_character":18},"updated":"2023-11-21 02:56:11.000000000","message":"see my comment in the other file.","commit_id":"c2820f1628f1688385513d5205042fc0ad7c20f6"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"3ff43cf640332df2fa578aca20714b398cca1f5c","unresolved":false,"context_lines":[{"line_number":191,"context_line":"        pm \u003d cls._get_metadata_proxy_process_manager(uuid, conf,"},{"line_number":192,"context_line":"                                                     ns_name\u003dns_name,"},{"line_number":193,"context_line":"                                                     callback\u003dcallback)"},{"line_number":194,"context_line":"        if pm.active:"},{"line_number":195,"context_line":"            return"},{"line_number":196,"context_line":"        try:"},{"line_number":197,"context_line":"            pm.enable()"},{"line_number":198,"context_line":"        except exceptions.ProcessExecutionError as exec_err:"}],"source_content_type":"text/x-python","patch_set":22,"id":"df6dab33_6a769884","line":195,"range":{"start_line":194,"start_character":0,"end_line":195,"end_character":18},"in_reply_to":"c77d7a48_205cf417","updated":"2023-11-29 21:20:23.000000000","message":"Done","commit_id":"c2820f1628f1688385513d5205042fc0ad7c20f6"}],"neutron/tests/unit/agent/dhcp/test_agent.py":[{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"5099f19238faca1af34af2f6ee63af22c70e9985","unresolved":true,"context_lines":[{"line_number":855,"context_line":"            process_instance.assert_has_calls(["},{"line_number":856,"context_line":"                mock.call.enable(),"},{"line_number":857,"context_line":"                mock.call.disable(sig\u003dstr(int(signal.SIGTERM)))],"},{"line_number":858,"context_line":"                any_order\u003dTrue)"},{"line_number":859,"context_line":"        else:"},{"line_number":860,"context_line":"            process_instance.assert_has_calls(["},{"line_number":861,"context_line":"                mock.call.disable(sig\u003dstr(int(signal.SIGTERM)))])"}],"source_content_type":"text/x-python","patch_set":22,"id":"58dbf71b_484a2ba8","line":858,"range":{"start_line":858,"start_character":16,"end_line":858,"end_character":30},"updated":"2023-11-21 13:52:32.000000000","message":"Why does the order not matter?","commit_id":"c2820f1628f1688385513d5205042fc0ad7c20f6"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"3ff43cf640332df2fa578aca20714b398cca1f5c","unresolved":false,"context_lines":[{"line_number":855,"context_line":"            process_instance.assert_has_calls(["},{"line_number":856,"context_line":"                mock.call.enable(),"},{"line_number":857,"context_line":"                mock.call.disable(sig\u003dstr(int(signal.SIGTERM)))],"},{"line_number":858,"context_line":"                any_order\u003dTrue)"},{"line_number":859,"context_line":"        else:"},{"line_number":860,"context_line":"            process_instance.assert_has_calls(["},{"line_number":861,"context_line":"                mock.call.disable(sig\u003dstr(int(signal.SIGTERM)))])"}],"source_content_type":"text/x-python","patch_set":22,"id":"f4cb665c_940e44a7","line":858,"range":{"start_line":858,"start_character":16,"end_line":858,"end_character":30},"in_reply_to":"58dbf71b_484a2ba8","updated":"2023-11-29 21:20:23.000000000","message":"Good call, it was introduced in 16-17 and later modified. But enforcing order is better and makes more sense bacause of[2]\n\n[1] https://review.opendev.org/c/openstack/neutron/+/890986/16..17\n[2] https://opendev.org/openstack/neutron/src/branch/master/neutron/agent/dhcp/agent.py#L771-L773","commit_id":"c2820f1628f1688385513d5205042fc0ad7c20f6"}],"neutron/tests/unit/agent/ovn/metadata/test_driver.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0c78b1767bc42349f1ed7c4002f1319e5393903c","unresolved":true,"context_lines":[{"line_number":158,"context_line":"    @mock.patch(\u0027neutron.agent.linux.external_process.ProcessManager\u0027)"},{"line_number":159,"context_line":"    def test_spawn_metadata_proxy_handles_process_exception(self,"},{"line_number":160,"context_line":"            error_log, process_manager):"},{"line_number":161,"context_line":"        process_instance \u003d mock.MagicMock()"},{"line_number":162,"context_line":"        process_instance.active \u003d False"},{"line_number":163,"context_line":"        process_instance.enable.side_effect \u003d lib_exceptions.\\"},{"line_number":164,"context_line":"            ProcessExecutionError(\u0027Something happened\u0027, -1)"}],"source_content_type":"text/x-python","patch_set":9,"id":"e2575dbc_2ba1ec47","line":161,"updated":"2023-09-18 14:32:25.000000000","message":"You can pass the defined attributes already during initialization:\n\n process_instance \u003d mock.MagicMock(active\u003dFalse)","commit_id":"6ce6f40cd6f1fd25620f54fac2b8e2e74e61cf15"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"b3d526065fed338d766b695fd1a789bb7a1ce31d","unresolved":false,"context_lines":[{"line_number":158,"context_line":"    @mock.patch(\u0027neutron.agent.linux.external_process.ProcessManager\u0027)"},{"line_number":159,"context_line":"    def test_spawn_metadata_proxy_handles_process_exception(self,"},{"line_number":160,"context_line":"            error_log, process_manager):"},{"line_number":161,"context_line":"        process_instance \u003d mock.MagicMock()"},{"line_number":162,"context_line":"        process_instance.active \u003d False"},{"line_number":163,"context_line":"        process_instance.enable.side_effect \u003d lib_exceptions.\\"},{"line_number":164,"context_line":"            ProcessExecutionError(\u0027Something happened\u0027, -1)"}],"source_content_type":"text/x-python","patch_set":9,"id":"063d1ca0_9d12476b","line":161,"in_reply_to":"e2575dbc_2ba1ec47","updated":"2023-09-18 17:38:22.000000000","message":"Done","commit_id":"6ce6f40cd6f1fd25620f54fac2b8e2e74e61cf15"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0c78b1767bc42349f1ed7c4002f1319e5393903c","unresolved":true,"context_lines":[{"line_number":160,"context_line":"            error_log, process_manager):"},{"line_number":161,"context_line":"        process_instance \u003d mock.MagicMock()"},{"line_number":162,"context_line":"        process_instance.active \u003d False"},{"line_number":163,"context_line":"        process_instance.enable.side_effect \u003d lib_exceptions.\\"},{"line_number":164,"context_line":"            ProcessExecutionError(\u0027Something happened\u0027, -1)"},{"line_number":165,"context_line":"        process_manager.return_value \u003d process_instance"},{"line_number":166,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"a158816a_80a27754","line":163,"range":{"start_line":163,"start_character":61,"end_line":163,"end_character":62},"updated":"2023-09-18 14:32:25.000000000","message":"nit: \"It is preferred to wrap long lines in parentheses and not a backslash for line continuation.\"\n\n -- https://docs.openstack.org/hacking/latest/user/hacking.html","commit_id":"6ce6f40cd6f1fd25620f54fac2b8e2e74e61cf15"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"b3d526065fed338d766b695fd1a789bb7a1ce31d","unresolved":false,"context_lines":[{"line_number":160,"context_line":"            error_log, process_manager):"},{"line_number":161,"context_line":"        process_instance \u003d mock.MagicMock()"},{"line_number":162,"context_line":"        process_instance.active \u003d False"},{"line_number":163,"context_line":"        process_instance.enable.side_effect \u003d lib_exceptions.\\"},{"line_number":164,"context_line":"            ProcessExecutionError(\u0027Something happened\u0027, -1)"},{"line_number":165,"context_line":"        process_manager.return_value \u003d process_instance"},{"line_number":166,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"f1d13858_23397ef6","line":163,"range":{"start_line":163,"start_character":61,"end_line":163,"end_character":62},"in_reply_to":"a158816a_80a27754","updated":"2023-09-18 17:38:22.000000000","message":"Done","commit_id":"6ce6f40cd6f1fd25620f54fac2b8e2e74e61cf15"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"7a78dfe4ec87b67da2d1591fe6006900ad0dfd4f","unresolved":true,"context_lines":[{"line_number":160,"context_line":"            error_log, process_manager):"},{"line_number":161,"context_line":"        process_instance \u003d mock.MagicMock(active\u003dFalse)"},{"line_number":162,"context_line":"        process_instance.enable.side_effect \u003d ("},{"line_number":163,"context_line":"            lib_exceptions. ProcessExecutionError(\u0027Something happened\u0027, -1))"},{"line_number":164,"context_line":"        process_manager.return_value \u003d process_instance"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"        process_monitor \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":12,"id":"b92a920b_d40942b6","line":163,"range":{"start_line":163,"start_character":27,"end_line":163,"end_character":28},"updated":"2023-09-19 19:29:19.000000000","message":"whitespace :) Interestingly I thought this would raise an error but it does not","commit_id":"90669276c7a8eb446de8c5f5e8f9e10e629c7dc6"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"b4d1bcb8643f2f33640f6b7b63d23bda674c2478","unresolved":false,"context_lines":[{"line_number":160,"context_line":"            error_log, process_manager):"},{"line_number":161,"context_line":"        process_instance \u003d mock.MagicMock(active\u003dFalse)"},{"line_number":162,"context_line":"        process_instance.enable.side_effect \u003d ("},{"line_number":163,"context_line":"            lib_exceptions. ProcessExecutionError(\u0027Something happened\u0027, -1))"},{"line_number":164,"context_line":"        process_manager.return_value \u003d process_instance"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"        process_monitor \u003d mock.Mock()"}],"source_content_type":"text/x-python","patch_set":12,"id":"6be32541_aba890d3","line":163,"range":{"start_line":163,"start_character":27,"end_line":163,"end_character":28},"in_reply_to":"b92a920b_d40942b6","updated":"2023-09-20 14:50:07.000000000","message":"Done","commit_id":"90669276c7a8eb446de8c5f5e8f9e10e629c7dc6"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"aea58e15585ef7f3aea0fb887fba03b279ba4a4b","unresolved":true,"context_lines":[{"line_number":158,"context_line":"    @mock.patch(\u0027neutron.agent.linux.external_process.ProcessManager\u0027)"},{"line_number":159,"context_line":"    def test_spawn_metadata_proxy_handles_process_exception(self,"},{"line_number":160,"context_line":"            error_log, process_manager):"},{"line_number":161,"context_line":"        process_instance \u003d mock.MagicMock(active\u003dFalse)"},{"line_number":162,"context_line":"        process_instance.enable.side_effect \u003d ("},{"line_number":163,"context_line":"            lib_exceptions.ProcessExecutionError(\u0027Something happened\u0027, -1))"},{"line_number":164,"context_line":"        process_manager.return_value \u003d process_instance"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"        process_monitor \u003d mock.Mock()"},{"line_number":167,"context_line":"        network_id \u003d 123"}],"source_content_type":"text/x-python","patch_set":14,"id":"f0f9dcaa_820162a2","line":164,"range":{"start_line":161,"start_character":0,"end_line":164,"end_character":55},"updated":"2023-09-26 18:38:17.000000000","message":"Why not mocking `MetadataDriver._get_metadata_proxy_process_manager()` instead?","commit_id":"502df507ed7c15d67e38776f54d55f90ba6395a7"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"9ad8c4b4561e9399153f9e52e0f01b556b783082","unresolved":false,"context_lines":[{"line_number":158,"context_line":"    @mock.patch(\u0027neutron.agent.linux.external_process.ProcessManager\u0027)"},{"line_number":159,"context_line":"    def test_spawn_metadata_proxy_handles_process_exception(self,"},{"line_number":160,"context_line":"            error_log, process_manager):"},{"line_number":161,"context_line":"        process_instance \u003d mock.MagicMock(active\u003dFalse)"},{"line_number":162,"context_line":"        process_instance.enable.side_effect \u003d ("},{"line_number":163,"context_line":"            lib_exceptions.ProcessExecutionError(\u0027Something happened\u0027, -1))"},{"line_number":164,"context_line":"        process_manager.return_value \u003d process_instance"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"        process_monitor \u003d mock.Mock()"},{"line_number":167,"context_line":"        network_id \u003d 123"}],"source_content_type":"text/x-python","patch_set":14,"id":"a1113277_aac17b42","line":164,"range":{"start_line":161,"start_character":0,"end_line":164,"end_character":55},"in_reply_to":"f0f9dcaa_820162a2","updated":"2023-10-02 15:07:35.000000000","message":"Done","commit_id":"502df507ed7c15d67e38776f54d55f90ba6395a7"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"e0457ab913a6587939cde199ef7884ec02750d71","unresolved":true,"context_lines":[{"line_number":160,"context_line":"        process_instance.enable.side_effect \u003d ("},{"line_number":161,"context_line":"            lib_exceptions.ProcessExecutionError(\u0027Something happened\u0027, -1))"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        metadata_driver.MetadataDriver._get_metadata_proxy_process_manager \u003d ("},{"line_number":164,"context_line":"            mock.Mock(return_value\u003dprocess_instance))"},{"line_number":165,"context_line":"        process_monitor \u003d mock.Mock()"},{"line_number":166,"context_line":"        network_id \u003d 123"},{"line_number":167,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"4049d0de_f8057647","line":164,"range":{"start_line":163,"start_character":0,"end_line":164,"end_character":53},"updated":"2023-10-04 18:04:19.000000000","message":"This will change the method in memory. Likely each test may reinstantiate but it\u0027s dangerous. It\u0027s better to use a context manager for this, so it\u0027s restored to its previous state after it\u0027s finished or if anything happens.\n```\n with mock.patch.object(metadata_driver.MetadataDriver, \u0027_get_metadata_proxy_process_manager\u0027):\n     \u003cdo stuff here\u003e\n \u003chere the method would be the original one and not mocked\u003e\n```","commit_id":"c4c220f014ee229a27ec10e4fbe3641dc662d59a"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"e0926ff5e7600b8227a1c8aed8b0bbc612f2000a","unresolved":true,"context_lines":[{"line_number":160,"context_line":"        process_instance.enable.side_effect \u003d ("},{"line_number":161,"context_line":"            lib_exceptions.ProcessExecutionError(\u0027Something happened\u0027, -1))"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        metadata_driver.MetadataDriver._get_metadata_proxy_process_manager \u003d ("},{"line_number":164,"context_line":"            mock.Mock(return_value\u003dprocess_instance))"},{"line_number":165,"context_line":"        process_monitor \u003d mock.Mock()"},{"line_number":166,"context_line":"        network_id \u003d 123"},{"line_number":167,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"893f2c3e_bfb9fadf","line":164,"range":{"start_line":163,"start_character":0,"end_line":164,"end_character":53},"in_reply_to":"4049d0de_f8057647","updated":"2023-10-04 18:05:37.000000000","message":"you can pass `return_value` in the `object()` function too","commit_id":"c4c220f014ee229a27ec10e4fbe3641dc662d59a"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"aa15fbdcb8d8187813aef05a037fa83fa2cbcdef","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        process_instance.enable.side_effect \u003d ("},{"line_number":161,"context_line":"            lib_exceptions.ProcessExecutionError(\u0027Something happened\u0027, -1))"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        metadata_driver.MetadataDriver._get_metadata_proxy_process_manager \u003d ("},{"line_number":164,"context_line":"            mock.Mock(return_value\u003dprocess_instance))"},{"line_number":165,"context_line":"        process_monitor \u003d mock.Mock()"},{"line_number":166,"context_line":"        network_id \u003d 123"},{"line_number":167,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"85974902_9cb23610","line":164,"range":{"start_line":163,"start_character":0,"end_line":164,"end_character":53},"in_reply_to":"893f2c3e_bfb9fadf","updated":"2023-10-04 18:48:06.000000000","message":"Done","commit_id":"c4c220f014ee229a27ec10e4fbe3641dc662d59a"}]}