)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":38024,"name":"Alexander Gnatyuk","display_name":"Alexander Gnatyuk","email":"agnatyuk@k2.cloud","username":"agnatyuk"},"change_message_id":"19295d1908f6df0408d3f7d8ea7fdc02e434b8e2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"55a2673f_91202e04","updated":"2025-08-04 10:41:16.000000000","message":"Hello, gentle ping on this PR 😊","commit_id":"bf52b1d7f9ab32660fdf66fed660afae8efcc6f2"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"00b9c337e61bf02193c49208fbc92b7c210ef033","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"07773e17_60c01a85","updated":"2025-08-04 15:36:10.000000000","message":"Looks great, thanks! Just a tiny nit and a question about inheriting from `AddCommand`.","commit_id":"bf52b1d7f9ab32660fdf66fed660afae8efcc6f2"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"d45f9f47a7846e381283d8075fc063a68ec64542","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"27f0b8eb_96295b10","updated":"2025-08-04 14:54:45.000000000","message":"reading now. sorry about that.","commit_id":"bf52b1d7f9ab32660fdf66fed660afae8efcc6f2"},{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"3aa8619cf34f4811074c748d3f7261bb5fa7f920","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"cf28fea1_fc23cd33","updated":"2025-05-26 09:43:02.000000000","message":"thanks looks really useful","commit_id":"bf52b1d7f9ab32660fdf66fed660afae8efcc6f2"},{"author":{"_account_id":38024,"name":"Alexander Gnatyuk","display_name":"Alexander Gnatyuk","email":"agnatyuk@k2.cloud","username":"agnatyuk"},"change_message_id":"963c05d499e46554d2f8e3fad73343516db5485b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"bdebb13b_714a238c","updated":"2025-08-07 10:29:40.000000000","message":"Thanks for the review! I\u0027ve rebased this patch on master.\n\nI have a question about the `may_exist\u003dTrue` behavior. In the MirrorAdd command, if we specify `may_exist\u003dTrue` and such mirror already exists, the command will update row via `set_columns(mirror, **self.columns)`. \nThis patch provides `mirror_rules: []` in Mirror, and if we:\n1. create mirror\n2. create rules\n3. create mirror again with `may_exist\u003dTrue` -- all mirror rules will be deleted (because of `columns[\"mirror_rules\"] \u003d [] + set_columns(row, **columns)`.\n\nPersonally, I wouldn\u0027t use `set_columns` if the mirror exists, but since this code is 2 years old, removing it now is a bad idea, I guess. If you have any thoughts on this, please share.","commit_id":"dfa5343162ab3446e8471e72f751b9ce71711b3c"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"17982c3995f45851acdd8138d0682a2b59ab02b0","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3ec83e83_316a8813","in_reply_to":"0e7705c6_e26eb32f","updated":"2025-12-23 20:32:27.000000000","message":"I would generally default to not doing set_columns if may_exist\u003dTrue, but with that said I have reviewed some BGP patches where the author wanted certain Commands to be idempotent and operate kind of as a CREATE OR UPDATE and that is also a valid choice. In that case, I would then only set the columns that were specifically passed and default to the existing values maybe?\n\nWith all of that said, I generally like to think of the ovsdbapp Commands as reference implementations. In general, a lot of the core commands try to do exactly what the ovn-nbctl and ovn-sbctl commands do. So in that case, there would be separate MirrorAdd and MirrorRuleAdd commands--and in that case, what to do with may_exist\u003dTrue becomes more obvious.","commit_id":"dfa5343162ab3446e8471e72f751b9ce71711b3c"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"736fb009deeac1d27e66f8aa9ce7b2c2b5e6c7c8","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"0e7705c6_e26eb32f","in_reply_to":"bdebb13b_714a238c","updated":"2025-08-07 12:13:11.000000000","message":"That\u0027s a fair concern. If you call the ``MirrorAdd`` with ``may_exist\u003dTrue``, that will delete all the existing rules. In that case, the command is unexpectedly changing the exiting register.\n\nI would wait for Terry\u0027s feedback on this. IMO, if we don\u0027t explicitly pass any value and the register exists, we should avoid changing this list. This will require a functional test.","commit_id":"dfa5343162ab3446e8471e72f751b9ce71711b3c"}],"ovsdbapp/schema/ovn_northbound/commands.py":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"00b9c337e61bf02193c49208fbc92b7c210ef033","unresolved":true,"context_lines":[{"line_number":1246,"context_line":"        mirror.addvalue(\u0027mirror_rules\u0027, rule)"},{"line_number":1247,"context_line":"        self.result \u003d rule.uuid"},{"line_number":1248,"context_line":""},{"line_number":1249,"context_line":"    def post_commit(self, txn):"},{"line_number":1250,"context_line":"        real_uuid \u003d txn.get_insert_uuid(self.result)"},{"line_number":1251,"context_line":"        if real_uuid:"},{"line_number":1252,"context_line":"            table \u003d self.api.tables[\u0027Mirror_Rule\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"70ad3034_e55064a5","line":1249,"updated":"2025-08-04 15:36:10.000000000","message":"Is there a reason that this command cannot inherit from `ovsdbapp/backend/ovs_idl/commands/AddCommand` and define `table_name \u003d \"Mirror_Rule\"` to pick this up?","commit_id":"bf52b1d7f9ab32660fdf66fed660afae8efcc6f2"},{"author":{"_account_id":38024,"name":"Alexander Gnatyuk","display_name":"Alexander Gnatyuk","email":"agnatyuk@k2.cloud","username":"agnatyuk"},"change_message_id":"963c05d499e46554d2f8e3fad73343516db5485b","unresolved":false,"context_lines":[{"line_number":1246,"context_line":"        mirror.addvalue(\u0027mirror_rules\u0027, rule)"},{"line_number":1247,"context_line":"        self.result \u003d rule.uuid"},{"line_number":1248,"context_line":""},{"line_number":1249,"context_line":"    def post_commit(self, txn):"},{"line_number":1250,"context_line":"        real_uuid \u003d txn.get_insert_uuid(self.result)"},{"line_number":1251,"context_line":"        if real_uuid:"},{"line_number":1252,"context_line":"            table \u003d self.api.tables[\u0027Mirror_Rule\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aaf2188_d470db2b","line":1249,"in_reply_to":"70ad3034_e55064a5","updated":"2025-08-07 10:29:40.000000000","message":"Thanks! Made it inherit from AddCommand.","commit_id":"bf52b1d7f9ab32660fdf66fed660afae8efcc6f2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"736fb009deeac1d27e66f8aa9ce7b2c2b5e6c7c8","unresolved":true,"context_lines":[{"line_number":1169,"context_line":"            \u0027sink\u0027: dest,"},{"line_number":1170,"context_line":"            \u0027type\u0027: mirror_type,"},{"line_number":1171,"context_line":"            \u0027index\u0027: index,"},{"line_number":1172,"context_line":"            \u0027mirror_rules\u0027: [],"},{"line_number":1173,"context_line":"            \u0027external_ids\u0027: external_ids or {},"},{"line_number":1174,"context_line":"        }"},{"line_number":1175,"context_line":"        self.may_exist \u003d may_exist"}],"source_content_type":"text/x-python","patch_set":2,"id":"7892a8e3_eb57bdee","line":1172,"range":{"start_line":1172,"start_character":12,"end_line":1172,"end_character":31},"updated":"2025-08-07 12:13:11.000000000","message":"As you mentioned, that was merged recently in OVN. This is not even released. The functional tests work because we, by default, use master.\n\nI tested with a previous version (v24.09.2, because I\u0027m working with it now). The commands fail with this version (snippet: https://paste.opendev.org/show/bWq2Qf9gKwyVHCegqJHb/). You can manually do this locally setting OVN_BRANCH in tox or in the env variables.\n\nWe can\u0027t deliver a command that works only with OVN master.","commit_id":"dfa5343162ab3446e8471e72f751b9ce71711b3c"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"17982c3995f45851acdd8138d0682a2b59ab02b0","unresolved":true,"context_lines":[{"line_number":1169,"context_line":"            \u0027sink\u0027: dest,"},{"line_number":1170,"context_line":"            \u0027type\u0027: mirror_type,"},{"line_number":1171,"context_line":"            \u0027index\u0027: index,"},{"line_number":1172,"context_line":"            \u0027mirror_rules\u0027: [],"},{"line_number":1173,"context_line":"            \u0027external_ids\u0027: external_ids or {},"},{"line_number":1174,"context_line":"        }"},{"line_number":1175,"context_line":"        self.may_exist \u003d may_exist"}],"source_content_type":"text/x-python","patch_set":2,"id":"5783608c_5f2a5c30","line":1172,"range":{"start_line":1172,"start_character":12,"end_line":1172,"end_character":31},"in_reply_to":"7892a8e3_eb57bdee","updated":"2025-12-23 20:32:27.000000000","message":"Agreed. We don\u0027t have a specific version of OVN ovsdbapp requires, but we try to be as backward compatible as possible. There is a `table_has_column()` method that can be used to check if this column is supported at runtime.","commit_id":"dfa5343162ab3446e8471e72f751b9ce71711b3c"}],"ovsdbapp/tests/functional/schema/ovn_northbound/test_impl_idl.py":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"00b9c337e61bf02193c49208fbc92b7c210ef033","unresolved":true,"context_lines":[{"line_number":2968,"context_line":"        mirror \u003d self.api.mirror_add(utils.get_rand_name(),"},{"line_number":2969,"context_line":"                                     const.MIRROR_TYPE_LPORT, 0, \u0027both\u0027,"},{"line_number":2970,"context_line":"                                     self.port_uuid).execute(check_error\u003dTrue)"},{"line_number":2971,"context_line":"        rule1 \u003d self._mirror_rule_add(mirror,)"},{"line_number":2972,"context_line":"        rule2 \u003d self._mirror_rule_add(mirror, match\u003d\u0027ip4\u0027)"},{"line_number":2973,"context_line":"        rule3 \u003d self._mirror_rule_add(mirror, priority\u003d2)"},{"line_number":2974,"context_line":"        for rule in [rule1, rule2, rule3]:"}],"source_content_type":"text/x-python","patch_set":1,"id":"cd91d145_032b53a1","line":2971,"range":{"start_line":2971,"start_character":44,"end_line":2971,"end_character":45},"updated":"2025-08-04 15:36:10.000000000","message":"nit: trailing comma","commit_id":"bf52b1d7f9ab32660fdf66fed660afae8efcc6f2"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"00b9c337e61bf02193c49208fbc92b7c210ef033","unresolved":true,"context_lines":[{"line_number":2968,"context_line":"        mirror \u003d self.api.mirror_add(utils.get_rand_name(),"},{"line_number":2969,"context_line":"                                     const.MIRROR_TYPE_LPORT, 0, \u0027both\u0027,"},{"line_number":2970,"context_line":"                                     self.port_uuid).execute(check_error\u003dTrue)"},{"line_number":2971,"context_line":"        rule1 \u003d self._mirror_rule_add(mirror,)"},{"line_number":2972,"context_line":"        rule2 \u003d self._mirror_rule_add(mirror, match\u003d\u0027ip4\u0027)"},{"line_number":2973,"context_line":"        rule3 \u003d self._mirror_rule_add(mirror, priority\u003d2)"},{"line_number":2974,"context_line":"        for rule in [rule1, rule2, rule3]:"}],"source_content_type":"text/x-python","patch_set":1,"id":"1ca54735_e8984c3b","line":2971,"range":{"start_line":2971,"start_character":44,"end_line":2971,"end_character":45},"updated":"2025-08-04 15:36:10.000000000","message":"trailing comma","commit_id":"bf52b1d7f9ab32660fdf66fed660afae8efcc6f2"},{"author":{"_account_id":38024,"name":"Alexander Gnatyuk","display_name":"Alexander Gnatyuk","email":"agnatyuk@k2.cloud","username":"agnatyuk"},"change_message_id":"963c05d499e46554d2f8e3fad73343516db5485b","unresolved":false,"context_lines":[{"line_number":2968,"context_line":"        mirror \u003d self.api.mirror_add(utils.get_rand_name(),"},{"line_number":2969,"context_line":"                                     const.MIRROR_TYPE_LPORT, 0, \u0027both\u0027,"},{"line_number":2970,"context_line":"                                     self.port_uuid).execute(check_error\u003dTrue)"},{"line_number":2971,"context_line":"        rule1 \u003d self._mirror_rule_add(mirror,)"},{"line_number":2972,"context_line":"        rule2 \u003d self._mirror_rule_add(mirror, match\u003d\u0027ip4\u0027)"},{"line_number":2973,"context_line":"        rule3 \u003d self._mirror_rule_add(mirror, priority\u003d2)"},{"line_number":2974,"context_line":"        for rule in [rule1, rule2, rule3]:"}],"source_content_type":"text/x-python","patch_set":1,"id":"dc867753_4ca91f37","line":2971,"range":{"start_line":2971,"start_character":44,"end_line":2971,"end_character":45},"in_reply_to":"1ca54735_e8984c3b","updated":"2025-08-07 10:29:40.000000000","message":"Done","commit_id":"bf52b1d7f9ab32660fdf66fed660afae8efcc6f2"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"3c141189eaca9f46a1031242b9fc82b1094115c0","unresolved":false,"context_lines":[{"line_number":2968,"context_line":"        mirror \u003d self.api.mirror_add(utils.get_rand_name(),"},{"line_number":2969,"context_line":"                                     const.MIRROR_TYPE_LPORT, 0, \u0027both\u0027,"},{"line_number":2970,"context_line":"                                     self.port_uuid).execute(check_error\u003dTrue)"},{"line_number":2971,"context_line":"        rule1 \u003d self._mirror_rule_add(mirror,)"},{"line_number":2972,"context_line":"        rule2 \u003d self._mirror_rule_add(mirror, match\u003d\u0027ip4\u0027)"},{"line_number":2973,"context_line":"        rule3 \u003d self._mirror_rule_add(mirror, priority\u003d2)"},{"line_number":2974,"context_line":"        for rule in [rule1, rule2, rule3]:"}],"source_content_type":"text/x-python","patch_set":1,"id":"fe475a0c_6816e8e3","line":2971,"range":{"start_line":2971,"start_character":44,"end_line":2971,"end_character":45},"in_reply_to":"cd91d145_032b53a1","updated":"2025-08-04 15:36:50.000000000","message":"Done","commit_id":"bf52b1d7f9ab32660fdf66fed660afae8efcc6f2"}],"releasenotes/notes/ovn-mirror-rules-support-56a36a90c1fcaab3.yaml":[{"author":{"_account_id":8313,"name":"Lajos Katona","display_name":"lajoskatona","email":"katonalala@gmail.com","username":"elajkat","status":"Ericsson Software Technology"},"change_message_id":"3aa8619cf34f4811074c748d3f7261bb5fa7f920","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Support mirror rules and new mirror type \u0027lport\u0027. "},{"line_number":5,"context_line":"    It was added in https://github.com/ovn-org/ovn/commit/3dd8f36b85b3e648bc506839aa62e81b21c41e30"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"482afe8c_0f0d316d","line":4,"range":{"start_line":4,"start_character":53,"end_line":4,"end_character":54},"updated":"2025-05-26 09:43:02.000000000","message":"nit: please remove the useless whitespace","commit_id":"bf52b1d7f9ab32660fdf66fed660afae8efcc6f2"},{"author":{"_account_id":38024,"name":"Alexander Gnatyuk","display_name":"Alexander Gnatyuk","email":"agnatyuk@k2.cloud","username":"agnatyuk"},"change_message_id":"963c05d499e46554d2f8e3fad73343516db5485b","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Support mirror rules and new mirror type \u0027lport\u0027. "},{"line_number":5,"context_line":"    It was added in https://github.com/ovn-org/ovn/commit/3dd8f36b85b3e648bc506839aa62e81b21c41e30"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"a52735c1_8d7392da","line":4,"range":{"start_line":4,"start_character":53,"end_line":4,"end_character":54},"in_reply_to":"482afe8c_0f0d316d","updated":"2025-08-07 10:29:40.000000000","message":"Done","commit_id":"bf52b1d7f9ab32660fdf66fed660afae8efcc6f2"}]}