)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"84d5df19e2625cb206e40dc7c7334e541576c9a3","unresolved":true,"context_lines":[{"line_number":12,"context_line":"a result there are only Port Binding Update and Port Binding Delete"},{"line_number":13,"context_line":"events that either provision or teardown the resources."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Related bug: #2036118"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: I13f4e95eacb3cbdd3170f9707b2310bfae13a2bc"},{"line_number":18,"context_line":"Signed-off-by: Jakub Libosvar \u003clibosvar@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"03c12843_c895a8d3","line":15,"updated":"2023-10-25 11:36:10.000000000","message":"How is this bug related? My first guess is that this patch is not a refactor in the strict sense of the word, because it changes behavior a bit, like timing. Can you please add a sentence how this patch contributes to the bug referred?","commit_id":"6801589510242affc78497660d34377603774074"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"292c1f77a4e3e7e107dd8ae1c3fd6375884a5c12","unresolved":true,"context_lines":[{"line_number":12,"context_line":"a result there are only Port Binding Update and Port Binding Delete"},{"line_number":13,"context_line":"events that either provision or teardown the resources."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Related bug: #2036118"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: I13f4e95eacb3cbdd3170f9707b2310bfae13a2bc"},{"line_number":18,"context_line":"Signed-off-by: Jakub Libosvar \u003clibosvar@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"bbd0238b_d2e33b44","line":15,"in_reply_to":"03c12843_c895a8d3","updated":"2023-11-08 15:07:24.000000000","message":"Oops, sorry. I missed this comment. The patch should not change any behavior, if there is a change in timing then it was not intentional and I may revisit it. It is related to the bug in a way that adding new code to the existing codebase was no longer possible because of wrong composition of the code. So after I added the new code it was a mess.\n\nCan you please provide more information about your concern about changed the timing?","commit_id":"6801589510242affc78497660d34377603774074"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"c93f967489c595620bf8944f1420660b25080563","unresolved":false,"context_lines":[{"line_number":12,"context_line":"a result there are only Port Binding Update and Port Binding Delete"},{"line_number":13,"context_line":"events that either provision or teardown the resources."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Related bug: #2036118"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: I13f4e95eacb3cbdd3170f9707b2310bfae13a2bc"},{"line_number":18,"context_line":"Signed-off-by: Jakub Libosvar \u003clibosvar@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"e41b3446_bfdac5db","line":15,"in_reply_to":"bbd0238b_d2e33b44","updated":"2023-11-21 09:48:45.000000000","message":"Sorry for the late answer. I was just not aware of the context of this patch and wanted to understand that better. Which happened on the PTG.","commit_id":"6801589510242affc78497660d34377603774074"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"e4e1f4ac352ef0250fcebc2a7f83cd4edfa9429d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"c84ea098_1d5fa667","updated":"2023-09-22 00:35:14.000000000","message":"Looks good, mostly nits","commit_id":"ae9eef0e6fea444257ff646b2d3d9948eedabe69"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"ac174bbce32c02ce6391f6fd5fa7acea2e656914","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"dc9b973a_4c4eba7b","updated":"2023-09-22 12:38:44.000000000","message":"Thanks Brian, let me know about the `EVENTS`","commit_id":"ae9eef0e6fea444257ff646b2d3d9948eedabe69"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"5e5ae4eedb1297cd5285f45b45c0c257b4ffb10d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"0e2e71e8_ad34cbfa","updated":"2023-09-22 15:21:25.000000000","message":"lgtm, thanks Kuba!","commit_id":"734c6b9aae4e723998868194fa7624d8cd5ed3e7"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"7fd7a24b044f0df05a4c98989283b536adfdc9b9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"c00a00b6_b3bed879","updated":"2023-09-22 20:17:53.000000000","message":"Rally failure seems related\n\nhttps://400f965ef0004f72c44e-23a7172f5dc1e58445390ec61883d218.ssl.cf5.rackcdn.com/896163/6/check/neutron-ovn-rally-task/9d9af0d/controller/logs/screen-q-ovn-metadata-agent.txt","commit_id":"0b805b89f8ad06469170408fd7416f5a0d8a4ae0"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"bc8f523a506bd475d2c7580f2f23277537db916b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"18d50651_be70fcb0","updated":"2023-10-03 12:20:43.000000000","message":"I am fine with this refactor, otherwiseguy, do you have any more comments?","commit_id":"5f4e3ff0667751072b2b618a28e6c2dedcc809be"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"0b28ca0ab4a196a95727a37c964bed48a5a37fc2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"b841c5fb_df416771","updated":"2023-09-25 14:06:52.000000000","message":"minor nit, good refactor","commit_id":"5f4e3ff0667751072b2b618a28e6c2dedcc809be"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"8597d32d79932cac033adda5da9b377624318bcc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"f87f6bd7_64b0a539","updated":"2023-10-03 16:43:39.000000000","message":"Thanks.","commit_id":"881dad49aa245573b9fe37cc1d712d95fd412fce"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"74443da116b1da23610ba40c8cdeb2b868ca6c4e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"5be14e90_8c2ec721","updated":"2023-10-26 19:44:04.000000000","message":"Refactor needed to get dependent bug fix merged.","commit_id":"6801589510242affc78497660d34377603774074"}],"neutron/agent/ovn/metadata/agent.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"e4e1f4ac352ef0250fcebc2a7f83cd4edfa9429d","unresolved":true,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"class PortBindingUpdatedEvent(PortBindingEvent):"},{"line_number":108,"context_line":"    EVENTS \u003d (PortBindingEvent.ROW_UPDATE,)"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":111,"context_line":"        super().__init__(*args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":3,"id":"f8da4ac5_e7b21982","line":108,"updated":"2023-09-22 00:35:14.000000000","message":"nit: I guess since there is only one event type now in all these classes this can be \"EVENT\". And a super-nit: can remove the trailing comma","commit_id":"ae9eef0e6fea444257ff646b2d3d9948eedabe69"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"01c0b6e678d723e4bce19d00424e2d16b9fdd180","unresolved":false,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"class PortBindingUpdatedEvent(PortBindingEvent):"},{"line_number":108,"context_line":"    EVENTS \u003d (PortBindingEvent.ROW_UPDATE,)"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":111,"context_line":"        super().__init__(*args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":3,"id":"f5a6c3eb_a73250ad","line":108,"in_reply_to":"1067ec69_3dcf9310","updated":"2023-09-22 14:33:01.000000000","message":"Done","commit_id":"ae9eef0e6fea444257ff646b2d3d9948eedabe69"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"ac174bbce32c02ce6391f6fd5fa7acea2e656914","unresolved":true,"context_lines":[{"line_number":105,"context_line":""},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"class PortBindingUpdatedEvent(PortBindingEvent):"},{"line_number":108,"context_line":"    EVENTS \u003d (PortBindingEvent.ROW_UPDATE,)"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    def __init__(self, *args, **kwargs):"},{"line_number":111,"context_line":"        super().__init__(*args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1067ec69_3dcf9310","line":108,"in_reply_to":"f8da4ac5_e7b21982","updated":"2023-09-22 12:38:44.000000000","message":"I originally thought you don\u0027t like the uppercase. So you want to have single type on the class and making it a tuple when passing down to RowEvent superclass?","commit_id":"ae9eef0e6fea444257ff646b2d3d9948eedabe69"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"e4e1f4ac352ef0250fcebc2a7f83cd4edfa9429d","unresolved":true,"context_lines":[{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def _is_localport_ext_ids_update(self, row, old):"},{"line_number":125,"context_line":"        if row.type \u003d\u003d ovn_const.LSP_TYPE_LOCALPORT:"},{"line_number":126,"context_line":"            if hasattr(old, \u0027external_ids\u0027):"},{"line_number":127,"context_line":"                device_id \u003d row.external_ids.get("},{"line_number":128,"context_line":"                    ovn_const.OVN_DEVID_EXT_ID_KEY, \"\")"},{"line_number":129,"context_line":"                if device_id.startswith(NS_PREFIX):"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a525ecd_2dec3581","line":126,"updated":"2023-09-22 00:35:14.000000000","message":"The original was:\n\nif hasattr(row, \u0027external_ids\u0027) and hasattr(old, \u0027external_ids\u0027):\n\nIs it Ok to drop the first check?","commit_id":"ae9eef0e6fea444257ff646b2d3d9948eedabe69"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"ac174bbce32c02ce6391f6fd5fa7acea2e656914","unresolved":true,"context_lines":[{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def _is_localport_ext_ids_update(self, row, old):"},{"line_number":125,"context_line":"        if row.type \u003d\u003d ovn_const.LSP_TYPE_LOCALPORT:"},{"line_number":126,"context_line":"            if hasattr(old, \u0027external_ids\u0027):"},{"line_number":127,"context_line":"                device_id \u003d row.external_ids.get("},{"line_number":128,"context_line":"                    ovn_const.OVN_DEVID_EXT_ID_KEY, \"\")"},{"line_number":129,"context_line":"                if device_id.startswith(NS_PREFIX):"}],"source_content_type":"text/x-python","patch_set":3,"id":"d858544f_c415bd91","line":126,"in_reply_to":"7a525ecd_2dec3581","updated":"2023-09-22 12:38:44.000000000","message":"Good eyes :) Yes, `row` has always all columns available so the first condition was always True","commit_id":"ae9eef0e6fea444257ff646b2d3d9948eedabe69"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"01c0b6e678d723e4bce19d00424e2d16b9fdd180","unresolved":false,"context_lines":[{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    def _is_localport_ext_ids_update(self, row, old):"},{"line_number":125,"context_line":"        if row.type \u003d\u003d ovn_const.LSP_TYPE_LOCALPORT:"},{"line_number":126,"context_line":"            if hasattr(old, \u0027external_ids\u0027):"},{"line_number":127,"context_line":"                device_id \u003d row.external_ids.get("},{"line_number":128,"context_line":"                    ovn_const.OVN_DEVID_EXT_ID_KEY, \"\")"},{"line_number":129,"context_line":"                if device_id.startswith(NS_PREFIX):"}],"source_content_type":"text/x-python","patch_set":3,"id":"a9d0ec6e_82a0e0e7","line":126,"in_reply_to":"d858544f_c415bd91","updated":"2023-09-22 14:33:01.000000000","message":"Done","commit_id":"ae9eef0e6fea444257ff646b2d3d9948eedabe69"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"e4e1f4ac352ef0250fcebc2a7f83cd4edfa9429d","unresolved":true,"context_lines":[{"line_number":127,"context_line":"                device_id \u003d row.external_ids.get("},{"line_number":128,"context_line":"                    ovn_const.OVN_DEVID_EXT_ID_KEY, \"\")"},{"line_number":129,"context_line":"                if device_id.startswith(NS_PREFIX):"},{"line_number":130,"context_line":"                    new_ext_ids \u003d row.external_ids"},{"line_number":131,"context_line":"                    old_ext_ids \u003d old.external_ids"},{"line_number":132,"context_line":"                    new_cidrs \u003d new_ext_ids.get("},{"line_number":133,"context_line":"                        ovn_const.OVN_CIDRS_EXT_ID_KEY, \"\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"9934b113_74ad3aec","line":130,"updated":"2023-09-22 00:35:14.000000000","message":"nit: could be above L127 and used there as well","commit_id":"ae9eef0e6fea444257ff646b2d3d9948eedabe69"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"01c0b6e678d723e4bce19d00424e2d16b9fdd180","unresolved":false,"context_lines":[{"line_number":127,"context_line":"                device_id \u003d row.external_ids.get("},{"line_number":128,"context_line":"                    ovn_const.OVN_DEVID_EXT_ID_KEY, \"\")"},{"line_number":129,"context_line":"                if device_id.startswith(NS_PREFIX):"},{"line_number":130,"context_line":"                    new_ext_ids \u003d row.external_ids"},{"line_number":131,"context_line":"                    old_ext_ids \u003d old.external_ids"},{"line_number":132,"context_line":"                    new_cidrs \u003d new_ext_ids.get("},{"line_number":133,"context_line":"                        ovn_const.OVN_CIDRS_EXT_ID_KEY, \"\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"43fb9b77_574dd595","line":130,"in_reply_to":"33ea4545_a0034449","updated":"2023-09-22 14:33:01.000000000","message":"Done","commit_id":"ae9eef0e6fea444257ff646b2d3d9948eedabe69"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"ac174bbce32c02ce6391f6fd5fa7acea2e656914","unresolved":true,"context_lines":[{"line_number":127,"context_line":"                device_id \u003d row.external_ids.get("},{"line_number":128,"context_line":"                    ovn_const.OVN_DEVID_EXT_ID_KEY, \"\")"},{"line_number":129,"context_line":"                if device_id.startswith(NS_PREFIX):"},{"line_number":130,"context_line":"                    new_ext_ids \u003d row.external_ids"},{"line_number":131,"context_line":"                    old_ext_ids \u003d old.external_ids"},{"line_number":132,"context_line":"                    new_cidrs \u003d new_ext_ids.get("},{"line_number":133,"context_line":"                        ovn_const.OVN_CIDRS_EXT_ID_KEY, \"\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"33ea4545_a0034449","line":130,"in_reply_to":"9934b113_74ad3aec","updated":"2023-09-22 12:38:44.000000000","message":"I didn\u0027t plan to touch on this code much as I would have to rewrite the whole https://github.com/openstack/neutron/commit/686698284b3553001398f65614c6521359bc45aa#diff-95903c989a1d043a90abe006cedd7ec20bd7a36855c3219cd74580cfa125c82fL96 patch. My goal of this refactor was to avoid having multiple events all running the same `run()` method","commit_id":"ae9eef0e6fea444257ff646b2d3d9948eedabe69"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"e4e1f4ac352ef0250fcebc2a7f83cd4edfa9429d","unresolved":true,"context_lines":[{"line_number":165,"context_line":"    def match_fn(self, event, row, old):"},{"line_number":166,"context_line":"        try:"},{"line_number":167,"context_line":"            if row.chassis[0].name \u003d\u003d self.agent.chassis:"},{"line_number":168,"context_line":"                if row.type !\u003d \"external\":"},{"line_number":169,"context_line":"                    LOG.warning("},{"line_number":170,"context_line":"                        \u0027Removing non-external type port %(port_id)s with \u0027"},{"line_number":171,"context_line":"                        \u0027type \"%(type)s\"\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"6f6ca3f4_90130e92","line":168,"range":{"start_line":168,"start_character":31,"end_line":168,"end_character":41},"updated":"2023-09-22 00:35:14.000000000","message":"Can be ovn_const.LSP_TYPE_EXTERNAL as well","commit_id":"ae9eef0e6fea444257ff646b2d3d9948eedabe69"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"ac174bbce32c02ce6391f6fd5fa7acea2e656914","unresolved":false,"context_lines":[{"line_number":165,"context_line":"    def match_fn(self, event, row, old):"},{"line_number":166,"context_line":"        try:"},{"line_number":167,"context_line":"            if row.chassis[0].name \u003d\u003d self.agent.chassis:"},{"line_number":168,"context_line":"                if row.type !\u003d \"external\":"},{"line_number":169,"context_line":"                    LOG.warning("},{"line_number":170,"context_line":"                        \u0027Removing non-external type port %(port_id)s with \u0027"},{"line_number":171,"context_line":"                        \u0027type \"%(type)s\"\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"69f467e1_2f61329b","line":168,"range":{"start_line":168,"start_character":31,"end_line":168,"end_character":41},"in_reply_to":"6f6ca3f4_90130e92","updated":"2023-09-22 12:38:44.000000000","message":"Done","commit_id":"ae9eef0e6fea444257ff646b2d3d9948eedabe69"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"a311c18f959400d33586a975bbc057ee52f88e8a","unresolved":true,"context_lines":[{"line_number":79,"context_line":"    def __init__(self, metadata_agent):"},{"line_number":80,"context_line":"        self.agent \u003d metadata_agent"},{"line_number":81,"context_line":"        table \u003d \u0027Port_Binding\u0027"},{"line_number":82,"context_line":"        super().__init__(tuple(self.__class__.EVENT), table, None)"},{"line_number":83,"context_line":"        self.event_name \u003d self.__class__.__name__"},{"line_number":84,"context_line":"        self.log_msg \u003d None"},{"line_number":85,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"64cc2261_e0eb8144","line":82,"range":{"start_line":82,"start_character":25,"end_line":82,"end_character":52},"updated":"2023-09-22 16:43:00.000000000","message":"This is wrong, it generates a tuple of each letter","commit_id":"734c6b9aae4e723998868194fa7624d8cd5ed3e7"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"d8adb7d6227e64efdd2a7c82bf3cec00e25dd1c9","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    def __init__(self, metadata_agent):"},{"line_number":80,"context_line":"        self.agent \u003d metadata_agent"},{"line_number":81,"context_line":"        table \u003d \u0027Port_Binding\u0027"},{"line_number":82,"context_line":"        super().__init__(tuple(self.__class__.EVENT), table, None)"},{"line_number":83,"context_line":"        self.event_name \u003d self.__class__.__name__"},{"line_number":84,"context_line":"        self.log_msg \u003d None"},{"line_number":85,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"d81d1d4c_d8e101ba","line":82,"range":{"start_line":82,"start_character":25,"end_line":82,"end_character":52},"in_reply_to":"64cc2261_e0eb8144","updated":"2023-09-22 16:46:51.000000000","message":"Done","commit_id":"734c6b9aae4e723998868194fa7624d8cd5ed3e7"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"e364e28c68f149458cf65a7f5c757f3546953d46","unresolved":true,"context_lines":[{"line_number":84,"context_line":"        self.log_msg \u003d None"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def match_fn(self, event, row, old):"},{"line_number":87,"context_line":"        return row.type in OVN_VIF_PORT_TYPES:"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    def run(self, event, row, old):"},{"line_number":90,"context_line":"        # Check if the port has been bound/unbound to our chassis and update"}],"source_content_type":"text/x-python","patch_set":7,"id":"b84237f9_2075986d","line":87,"range":{"start_line":87,"start_character":45,"end_line":87,"end_character":46},"updated":"2023-09-22 22:16:14.000000000","message":"syntax error","commit_id":"e51c71bbcc53b41184e004d5a5fc6a00f45547cd"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"d5e4c593b8e66a0bb46f1aee5da38863de6e2527","unresolved":false,"context_lines":[{"line_number":84,"context_line":"        self.log_msg \u003d None"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def match_fn(self, event, row, old):"},{"line_number":87,"context_line":"        return row.type in OVN_VIF_PORT_TYPES:"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    def run(self, event, row, old):"},{"line_number":90,"context_line":"        # Check if the port has been bound/unbound to our chassis and update"}],"source_content_type":"text/x-python","patch_set":7,"id":"fd92227e_b57f2083","line":87,"range":{"start_line":87,"start_character":45,"end_line":87,"end_character":46},"in_reply_to":"930c820f_651715c9","updated":"2023-09-25 13:19:45.000000000","message":"Done","commit_id":"e51c71bbcc53b41184e004d5a5fc6a00f45547cd"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"3ee314a71f19733c09799d5f88da13896192bdf5","unresolved":true,"context_lines":[{"line_number":84,"context_line":"        self.log_msg \u003d None"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def match_fn(self, event, row, old):"},{"line_number":87,"context_line":"        return row.type in OVN_VIF_PORT_TYPES:"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    def run(self, event, row, old):"},{"line_number":90,"context_line":"        # Check if the port has been bound/unbound to our chassis and update"}],"source_content_type":"text/x-python","patch_set":7,"id":"930c820f_651715c9","line":87,"range":{"start_line":87,"start_character":45,"end_line":87,"end_character":46},"in_reply_to":"b84237f9_2075986d","updated":"2023-09-25 13:15:57.000000000","message":"that\u0027s what happens when I do a trivial change using the webui. Thanks:)","commit_id":"e51c71bbcc53b41184e004d5a5fc6a00f45547cd"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"dc52690001d9e16a7465a76db8df9d8f33c9781f","unresolved":true,"context_lines":[{"line_number":167,"context_line":"        if not super().match_fn(event, row, old):"},{"line_number":168,"context_line":"            return False"},{"line_number":169,"context_line":"        try:"},{"line_number":170,"context_line":"            if row.chassis[0].name \u003d\u003d self.agent.chassis:"},{"line_number":171,"context_line":"                if row.type !\u003d ovn_const.LSP_TYPE_EXTERNAL:"},{"line_number":172,"context_line":"                    LOG.warning("},{"line_number":173,"context_line":"                        \u0027Removing non-external type port %(port_id)s with \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"a7bad42d_7a67a7b0","line":170,"updated":"2023-09-22 23:27:47.000000000","message":"I\u0027m not seeing `PortBindingDeletedEvent` happening locally when I play with this. By the time the `Port_Binding` is deleted, it seems the `chassis\u003d[]`. Looking with ovsdb-client monitor, I see an UPDATE that clears the chassis before the DELETE happens. So I wouldn\u0027t expect that this will ever match.","commit_id":"e51c71bbcc53b41184e004d5a5fc6a00f45547cd"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"e6fd62783ad961c84d223d691a6fa431c49be896","unresolved":true,"context_lines":[{"line_number":167,"context_line":"        if not super().match_fn(event, row, old):"},{"line_number":168,"context_line":"            return False"},{"line_number":169,"context_line":"        try:"},{"line_number":170,"context_line":"            if row.chassis[0].name \u003d\u003d self.agent.chassis:"},{"line_number":171,"context_line":"                if row.type !\u003d ovn_const.LSP_TYPE_EXTERNAL:"},{"line_number":172,"context_line":"                    LOG.warning("},{"line_number":173,"context_line":"                        \u0027Removing non-external type port %(port_id)s with \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"4171ea8e_4abaa468","line":170,"in_reply_to":"37669bcd_5e8cc3b4","updated":"2023-09-25 19:26:57.000000000","message":"Aha, true. Before this patch, we matched the event but actually returned in `run()` and did nothing. Now we return on L167. From `ovsdb-client monitor` I can see this event was triggered when removing external gateway info from a router:\n\n```\nrow                                  action chassis                              datapath                             encap external_ids                                                                                                                                                  \u003e\n------------------------------------ ------ ------------------------------------ ------------------------------------ ----- --------------------------------------------------------------------------------------------------------------------------------------------------------------\u003e\ne081284e-b28d-4d6a-a647-969e6fea53e6 delete a08d3f4a-7db9-492b-8cd0-a410b30309dc 2e6159f7-df84-4951-96cb-7b4fb94847f1 []    {\"neutron:network_name\"\u003dneutron-0dcfaeb0-1f1a-41db-8f89-2c9ca0cc7d12, \"neutron:revision_number\"\u003d\"4\", \"neutron:router_name\"\u003d\"d4a3dd33-8cf0-4518-8c75-a93f02860e\u003e\n\n```","commit_id":"e51c71bbcc53b41184e004d5a5fc6a00f45547cd"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"1b3a13d578157b2faf8de244e60c21d672fb5b18","unresolved":true,"context_lines":[{"line_number":167,"context_line":"        if not super().match_fn(event, row, old):"},{"line_number":168,"context_line":"            return False"},{"line_number":169,"context_line":"        try:"},{"line_number":170,"context_line":"            if row.chassis[0].name \u003d\u003d self.agent.chassis:"},{"line_number":171,"context_line":"                if row.type !\u003d ovn_const.LSP_TYPE_EXTERNAL:"},{"line_number":172,"context_line":"                    LOG.warning("},{"line_number":173,"context_line":"                        \u0027Removing non-external type port %(port_id)s with \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"a605cf6d_47937d83","line":170,"in_reply_to":"4171ea8e_4abaa468","updated":"2023-09-25 19:44:18.000000000","message":"I found out this DELETE event is for SRIOV external port: https://opendev.org/openstack/neutron/commit/76a2f8b33e3f673b96feeb2724ae55567a3b6520","commit_id":"e51c71bbcc53b41184e004d5a5fc6a00f45547cd"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"412c61d03f74f75ec9b4e9111e9cc700e35f0c19","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        if not super().match_fn(event, row, old):"},{"line_number":168,"context_line":"            return False"},{"line_number":169,"context_line":"        try:"},{"line_number":170,"context_line":"            if row.chassis[0].name \u003d\u003d self.agent.chassis:"},{"line_number":171,"context_line":"                if row.type !\u003d ovn_const.LSP_TYPE_EXTERNAL:"},{"line_number":172,"context_line":"                    LOG.warning("},{"line_number":173,"context_line":"                        \u0027Removing non-external type port %(port_id)s with \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"fb66dc1d_805c65cb","line":170,"in_reply_to":"a605cf6d_47937d83","updated":"2023-10-03 16:03:19.000000000","message":"Done","commit_id":"e51c71bbcc53b41184e004d5a5fc6a00f45547cd"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"d5e4c593b8e66a0bb46f1aee5da38863de6e2527","unresolved":true,"context_lines":[{"line_number":167,"context_line":"        if not super().match_fn(event, row, old):"},{"line_number":168,"context_line":"            return False"},{"line_number":169,"context_line":"        try:"},{"line_number":170,"context_line":"            if row.chassis[0].name \u003d\u003d self.agent.chassis:"},{"line_number":171,"context_line":"                if row.type !\u003d ovn_const.LSP_TYPE_EXTERNAL:"},{"line_number":172,"context_line":"                    LOG.warning("},{"line_number":173,"context_line":"                        \u0027Removing non-external type port %(port_id)s with \u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"37669bcd_5e8cc3b4","line":170,"in_reply_to":"a7bad42d_7a67a7b0","updated":"2023-09-25 13:19:45.000000000","message":"I saw in the logs that cr-lrp ports get deleted without chassis being unset first.","commit_id":"e51c71bbcc53b41184e004d5a5fc6a00f45547cd"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"a79866bdad0f6299b691e621c78dca8e95711d0a","unresolved":true,"context_lines":[{"line_number":95,"context_line":"            try:"},{"line_number":96,"context_line":"                net_name \u003d ovn_utils.get_network_name_from_datapath("},{"line_number":97,"context_line":"                    row.datapath)"},{"line_number":98,"context_line":"                if self.log_msg:"},{"line_number":99,"context_line":"                    LOG.info(self.log_msg, row.logical_port, net_name)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"                self.agent.provision_datapath(row.datapath)"}],"source_content_type":"text/x-python","patch_set":8,"id":"2cf49586_90df9222","line":98,"updated":"2023-10-03 14:27:51.000000000","message":"(No action required, putting this here only since you are in the refactoring mood) Looks like a very specific format for the log message is assumed. This coupling could be more explicit if log_msg would be a callable and not a string, that receives two arguments (lport and net_name), and that - in its default implementation - does nothing. But in child classes, it could return a formatted string.","commit_id":"5f4e3ff0667751072b2b618a28e6c2dedcc809be"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0d76606f63c06afcc5c4cb792075bbf00d4e2db7","unresolved":false,"context_lines":[{"line_number":95,"context_line":"            try:"},{"line_number":96,"context_line":"                net_name \u003d ovn_utils.get_network_name_from_datapath("},{"line_number":97,"context_line":"                    row.datapath)"},{"line_number":98,"context_line":"                if self.log_msg:"},{"line_number":99,"context_line":"                    LOG.info(self.log_msg, row.logical_port, net_name)"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"                self.agent.provision_datapath(row.datapath)"}],"source_content_type":"text/x-python","patch_set":8,"id":"88338819_c748bee3","line":98,"in_reply_to":"2cf49586_90df9222","updated":"2023-10-03 15:05:54.000000000","message":"Done","commit_id":"5f4e3ff0667751072b2b618a28e6c2dedcc809be"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"a79866bdad0f6299b691e621c78dca8e95711d0a","unresolved":true,"context_lines":[{"line_number":125,"context_line":"        return any(check(row, old) for check in self._match_checks)"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"    def _is_localport_ext_ids_update(self, row, old):"},{"line_number":128,"context_line":"        if row.type \u003d\u003d ovn_const.LSP_TYPE_LOCALPORT:"},{"line_number":129,"context_line":"            if hasattr(old, \u0027external_ids\u0027):"},{"line_number":130,"context_line":"                device_id \u003d row.external_ids.get("},{"line_number":131,"context_line":"                    ovn_const.OVN_DEVID_EXT_ID_KEY, \"\")"}],"source_content_type":"text/x-python","patch_set":8,"id":"b0253360_38e494cd","line":128,"updated":"2023-10-03 14:27:51.000000000","message":"(Not required, refactoring mode on) it\u0027s usually easier on eyes when code is structured as:\n\nif not X:\n  return False\n...proceed...\n\nthan:\n\nif X:\n   ...proceed...\nreturn False\n\nsince it saves one indent level (and you have four here already)","commit_id":"5f4e3ff0667751072b2b618a28e6c2dedcc809be"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"0b28ca0ab4a196a95727a37c964bed48a5a37fc2","unresolved":true,"context_lines":[{"line_number":125,"context_line":"        return any(check(row, old) for check in self._match_checks)"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"    def _is_localport_ext_ids_update(self, row, old):"},{"line_number":128,"context_line":"        if row.type \u003d\u003d ovn_const.LSP_TYPE_LOCALPORT:"},{"line_number":129,"context_line":"            if hasattr(old, \u0027external_ids\u0027):"},{"line_number":130,"context_line":"                device_id \u003d row.external_ids.get("},{"line_number":131,"context_line":"                    ovn_const.OVN_DEVID_EXT_ID_KEY, \"\")"}],"source_content_type":"text/x-python","patch_set":8,"id":"3741360d_f4c18f2e","line":128,"range":{"start_line":128,"start_character":8,"end_line":128,"end_character":52},"updated":"2023-09-25 14:06:52.000000000","message":"nit: not needed, already tested in \n```\n  if not super().match_fn(event, row, old):\n    return False\n```","commit_id":"5f4e3ff0667751072b2b618a28e6c2dedcc809be"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"686891bf1af6bb201864d6628463a3646d855f35","unresolved":false,"context_lines":[{"line_number":125,"context_line":"        return any(check(row, old) for check in self._match_checks)"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"    def _is_localport_ext_ids_update(self, row, old):"},{"line_number":128,"context_line":"        if row.type \u003d\u003d ovn_const.LSP_TYPE_LOCALPORT:"},{"line_number":129,"context_line":"            if hasattr(old, \u0027external_ids\u0027):"},{"line_number":130,"context_line":"                device_id \u003d row.external_ids.get("},{"line_number":131,"context_line":"                    ovn_const.OVN_DEVID_EXT_ID_KEY, \"\")"}],"source_content_type":"text/x-python","patch_set":8,"id":"c56f1025_78ed9e5b","line":128,"range":{"start_line":128,"start_character":8,"end_line":128,"end_character":52},"in_reply_to":"3741360d_f4c18f2e","updated":"2023-09-25 14:08:05.000000000","message":"Never mind, discard this comment.","commit_id":"5f4e3ff0667751072b2b618a28e6c2dedcc809be"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0d76606f63c06afcc5c4cb792075bbf00d4e2db7","unresolved":false,"context_lines":[{"line_number":125,"context_line":"        return any(check(row, old) for check in self._match_checks)"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"    def _is_localport_ext_ids_update(self, row, old):"},{"line_number":128,"context_line":"        if row.type \u003d\u003d ovn_const.LSP_TYPE_LOCALPORT:"},{"line_number":129,"context_line":"            if hasattr(old, \u0027external_ids\u0027):"},{"line_number":130,"context_line":"                device_id \u003d row.external_ids.get("},{"line_number":131,"context_line":"                    ovn_const.OVN_DEVID_EXT_ID_KEY, \"\")"}],"source_content_type":"text/x-python","patch_set":8,"id":"d04c34bc_da3fc25f","line":128,"in_reply_to":"b0253360_38e494cd","updated":"2023-10-03 15:05:54.000000000","message":"Done","commit_id":"5f4e3ff0667751072b2b618a28e6c2dedcc809be"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"a79866bdad0f6299b691e621c78dca8e95711d0a","unresolved":true,"context_lines":[{"line_number":154,"context_line":"    def _is_chassis_removed(self, row, old):"},{"line_number":155,"context_line":"        self.log_msg \u003d \"Port %s in datapath %s unbound from our chassis\""},{"line_number":156,"context_line":"        try:"},{"line_number":157,"context_line":"            return (old.chassis[0].name \u003d\u003d self.agent.chassis and"},{"line_number":158,"context_line":"                    not row.chassis)"},{"line_number":159,"context_line":"        except (IndexError, AttributeError):"},{"line_number":160,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":8,"id":"501649fb_187283b8","line":157,"updated":"2023-10-03 14:27:51.000000000","message":"(No action required) this seems to rely on the fact that ovn-controller will not update .chassis field before it\u0027s unset? I don\u0027t think ovn-controller promises it. Here is the code in controller/binding.c that handles switch from one chassis to another:\n\n```\n            if (pb-\u003echassis) {\n                VLOG_INFO(\"Changing chassis for lport %s from %s to %s.\",\n                          pb-\u003elogical_port, pb-\u003echassis-\u003ename,\n                          chassis_rec-\u003ename);\n            } else {\n                VLOG_INFO(\"Claiming lport %s for this chassis.\",\n                          pb-\u003elogical_port);\n            }\n...\n            sbrec_port_binding_set_chassis(pb, chassis_rec);\n\n```\n\nNote it sets new chassis without an intermediate NULL state. Are we sure this code works as it should? (I understand it\u0027s probably not an issue with this patch, hence No action required)\n\n(I believe a similar scenario where our own chassis changed active chassis is not handled above either - in `is_new_chassis_set`)","commit_id":"5f4e3ff0667751072b2b618a28e6c2dedcc809be"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0d76606f63c06afcc5c4cb792075bbf00d4e2db7","unresolved":true,"context_lines":[{"line_number":154,"context_line":"    def _is_chassis_removed(self, row, old):"},{"line_number":155,"context_line":"        self.log_msg \u003d \"Port %s in datapath %s unbound from our chassis\""},{"line_number":156,"context_line":"        try:"},{"line_number":157,"context_line":"            return (old.chassis[0].name \u003d\u003d self.agent.chassis and"},{"line_number":158,"context_line":"                    not row.chassis)"},{"line_number":159,"context_line":"        except (IndexError, AttributeError):"},{"line_number":160,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":8,"id":"6542134a_2f9be582","line":157,"in_reply_to":"501649fb_187283b8","updated":"2023-10-03 15:05:54.000000000","message":"Thanks, I\u0027ll look into it but the change should have its own patch to not change any logic in a refactoring patch.","commit_id":"5f4e3ff0667751072b2b618a28e6c2dedcc809be"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"a79866bdad0f6299b691e621c78dca8e95711d0a","unresolved":true,"context_lines":[{"line_number":176,"context_line":"                self.log_msg \u003d ("},{"line_number":177,"context_line":"                    \"Port %s in datapath %s unbound from our chassis\")"},{"line_number":178,"context_line":"                return True"},{"line_number":179,"context_line":"        except (IndexError, AttributeError):"},{"line_number":180,"context_line":"            return False"},{"line_number":181,"context_line":"        return False"},{"line_number":182,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"f7ca4b7d_a22b2b58","line":179,"updated":"2023-10-03 14:27:51.000000000","message":"I think this handler should be narrower in scope (covering only line 170), we don\u0027t expect the same errors from other parts of the block.","commit_id":"5f4e3ff0667751072b2b618a28e6c2dedcc809be"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0d76606f63c06afcc5c4cb792075bbf00d4e2db7","unresolved":false,"context_lines":[{"line_number":176,"context_line":"                self.log_msg \u003d ("},{"line_number":177,"context_line":"                    \"Port %s in datapath %s unbound from our chassis\")"},{"line_number":178,"context_line":"                return True"},{"line_number":179,"context_line":"        except (IndexError, AttributeError):"},{"line_number":180,"context_line":"            return False"},{"line_number":181,"context_line":"        return False"},{"line_number":182,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"d1838d43_7deca0b5","line":179,"in_reply_to":"f7ca4b7d_a22b2b58","updated":"2023-10-03 15:05:54.000000000","message":"Done","commit_id":"5f4e3ff0667751072b2b618a28e6c2dedcc809be"}]}