)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"ec3edacadeed6c18506e1692392187f672c7680a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c1ed2bbc_407dea3a","updated":"2023-09-27 14:43:24.000000000","message":"Adding Workflow -1 since you aren\u0027t ready for reviews. I\u0027ll try to keep an eye out for when you mark it ready for review. Thanks for the work!","commit_id":"281cc82124ef0ce00a1a1a7786c68d1e419a8fc4"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"a8da098ed138d00524e400de6a6864b8413ee85d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6d26726b_37e0047e","updated":"2023-09-27 12:56:12.000000000","message":"Please add functional tests","commit_id":"281cc82124ef0ce00a1a1a7786c68d1e419a8fc4"},{"author":{"_account_id":33871,"name":"Anton Vazhnetsov","display_name":"Anton Vazhnetsov","email":"dragen15051@gmail.com","username":"0x5b"},"change_message_id":"8fe9f0922903b505ad983069dfe609caca07afed","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"fcbd5094_1d4f7652","in_reply_to":"6d26726b_37e0047e","updated":"2023-09-27 14:26:09.000000000","message":"This PR is in progress, which is why I didn\u0027t add reviewers and Zuul.","commit_id":"281cc82124ef0ce00a1a1a7786c68d1e419a8fc4"},{"author":{"_account_id":33871,"name":"Anton Vazhnetsov","display_name":"Anton Vazhnetsov","email":"dragen15051@gmail.com","username":"0x5b"},"change_message_id":"5db31182c36c7bc1fac2a71427133d6d0e4173c1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"0c4e18a4_d728d464","in_reply_to":"c1ed2bbc_407dea3a","updated":"2023-09-27 21:19:26.000000000","message":"Thank you for the quick reply. Now this PR is ready for review.","commit_id":"281cc82124ef0ce00a1a1a7786c68d1e419a8fc4"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"d4856110e0f97d7f48afb9c656b834996c6e3a96","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"0eaab154_92ee6d58","updated":"2023-09-28 06:07:14.000000000","message":"nice addition, just one nit","commit_id":"a33db3562dec703a38ff8a9a70fc7b7eda6c152b"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"4460a22512dcc6359f77c55308feb5236c8ca9d2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"bbbd2ca9_d6418b48","updated":"2023-09-28 09:41:31.000000000","message":"thanks!","commit_id":"d30e80475048585273c460612a1655d828434c6f"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"5a68993232a6cd07ab0cc0be20b9e5167c194707","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"f7b1b6e3_6168e97f","updated":"2023-10-05 13:23:55.000000000","message":"I\u0027ll add the validation in a separate patch after PTO. Thanks!","commit_id":"033b6e3bd217c3e424e13f94893d1cb112abf87c"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"10879314af97d724605cb90b4371bb3bfef5fbfe","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"26cadf4b_49f42cc9","updated":"2023-10-04 18:13:57.000000000","message":"Looks good. Thanks! Just one comment re: validating data.","commit_id":"033b6e3bd217c3e424e13f94893d1cb112abf87c"}],"ovsdbapp/schema/ovn_northbound/api.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"d25c1569586ac7406de4462f34ac706b05abf176","unresolved":true,"context_lines":[{"line_number":762,"context_line":"        \"\"\""},{"line_number":763,"context_line":""},{"line_number":764,"context_line":"    @abc.abstractmethod"},{"line_number":765,"context_line":"    def lr_route_del(self, router, prefix\u003dNone, if_exists\u003dFalse, nexthop\u003dNone):"},{"line_number":766,"context_line":"        \"\"\"Remove routes from \u0027router\u0027"},{"line_number":767,"context_line":""},{"line_number":768,"context_line":"        :param router:    The name or uuid of the router"}],"source_content_type":"text/x-python","patch_set":6,"id":"06896e78_24c38ca8","line":765,"range":{"start_line":765,"start_character":48,"end_line":765,"end_character":77},"updated":"2023-10-04 15:12:58.000000000","message":"Why this change from PS5?\n\nBy default it is recommendable to add any new parameter at the end of the method. But it is also known that \"if_exists\" is usually at the end of the ovsdbapp methods.","commit_id":"033b6e3bd217c3e424e13f94893d1cb112abf87c"},{"author":{"_account_id":33871,"name":"Anton Vazhnetsov","display_name":"Anton Vazhnetsov","email":"dragen15051@gmail.com","username":"0x5b"},"change_message_id":"183fd8450da923b736ca681ab51cab01a191c5e2","unresolved":true,"context_lines":[{"line_number":762,"context_line":"        \"\"\""},{"line_number":763,"context_line":""},{"line_number":764,"context_line":"    @abc.abstractmethod"},{"line_number":765,"context_line":"    def lr_route_del(self, router, prefix\u003dNone, if_exists\u003dFalse, nexthop\u003dNone):"},{"line_number":766,"context_line":"        \"\"\"Remove routes from \u0027router\u0027"},{"line_number":767,"context_line":""},{"line_number":768,"context_line":"        :param router:    The name or uuid of the router"}],"source_content_type":"text/x-python","patch_set":6,"id":"4b0ab1d5_f025048c","line":765,"range":{"start_line":765,"start_character":48,"end_line":765,"end_character":77},"in_reply_to":"06896e78_24c38ca8","updated":"2023-10-04 15:38:47.000000000","message":"I would have loved to leave it as is, but in this discussion it was decided to change:\nhttps://review.opendev.org/c/openstack/ovsdbapp/+/896793/comment/133fdb8e_5857ce86/","commit_id":"033b6e3bd217c3e424e13f94893d1cb112abf87c"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"ef48299abc50db1bf9b52bd0e34cecdeb5954d01","unresolved":false,"context_lines":[{"line_number":762,"context_line":"        \"\"\""},{"line_number":763,"context_line":""},{"line_number":764,"context_line":"    @abc.abstractmethod"},{"line_number":765,"context_line":"    def lr_route_del(self, router, prefix\u003dNone, if_exists\u003dFalse, nexthop\u003dNone):"},{"line_number":766,"context_line":"        \"\"\"Remove routes from \u0027router\u0027"},{"line_number":767,"context_line":""},{"line_number":768,"context_line":"        :param router:    The name or uuid of the router"}],"source_content_type":"text/x-python","patch_set":6,"id":"e17ba383_e65045a4","line":765,"range":{"start_line":765,"start_character":48,"end_line":765,"end_character":77},"in_reply_to":"4b0ab1d5_f025048c","updated":"2023-10-04 17:52:16.000000000","message":"@rodolfo There\u0027s no reason for if_exists to be last. There\u0027s nothing that would ever know what a \"last argument\" was in python args. They\u0027re either positional, in which case \"last\" is just \"seventh\" or whatever, or they\u0027re passed as keyword arguments and order doesn\u0027t matter at all. It\u0027s safest to append arguments to avoid breaking user code, so that\u0027s what should be done when modifying existing API code.","commit_id":"033b6e3bd217c3e424e13f94893d1cb112abf87c"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"271554e43d02a3a2bfb718edfd192495290338d2","unresolved":false,"context_lines":[{"line_number":762,"context_line":"        \"\"\""},{"line_number":763,"context_line":""},{"line_number":764,"context_line":"    @abc.abstractmethod"},{"line_number":765,"context_line":"    def lr_route_del(self, router, prefix\u003dNone, if_exists\u003dFalse, nexthop\u003dNone):"},{"line_number":766,"context_line":"        \"\"\"Remove routes from \u0027router\u0027"},{"line_number":767,"context_line":""},{"line_number":768,"context_line":"        :param router:    The name or uuid of the router"}],"source_content_type":"text/x-python","patch_set":6,"id":"e1e4fcfb_435da0cc","line":765,"range":{"start_line":765,"start_character":48,"end_line":765,"end_character":77},"in_reply_to":"e17ba383_e65045a4","updated":"2023-10-05 10:40:26.000000000","message":"Ok, I really thought we had this convention in this project, at least for these API calls.","commit_id":"033b6e3bd217c3e424e13f94893d1cb112abf87c"}],"ovsdbapp/schema/ovn_northbound/commands.py":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"10879314af97d724605cb90b4371bb3bfef5fbfe","unresolved":true,"context_lines":[{"line_number":1193,"context_line":"        super().__init__(api)"},{"line_number":1194,"context_line":"        self.router \u003d router"},{"line_number":1195,"context_line":"        self.prefix \u003d prefix"},{"line_number":1196,"context_line":"        self.nexthop \u003d nexthop"},{"line_number":1197,"context_line":"        self.if_exists \u003d if_exists"},{"line_number":1198,"context_line":""},{"line_number":1199,"context_line":"    def run_idl(self, txn):"}],"source_content_type":"text/x-python","patch_set":6,"id":"0c05738e_b29acff3","line":1196,"updated":"2023-10-04 18:13:57.000000000","message":"If I\u0027m reading the ovn-nbctl code [1] correctly, valid values for `nexthop` are either an IP or \"discard\". Since the OVN schema makes this a bare string field and leaves data validation up to a client (Bad DB. Bad!), we should probably check input on these values before writing them. Thoughts?\n\n[1] https://github.com/ovn-org/ovn/blob/2b1d8e14c02dfd947a101a0dff8d723a5205f3a3/utilities/ovn-nbctl.c#L4688","commit_id":"033b6e3bd217c3e424e13f94893d1cb112abf87c"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"2e97c138d90ec15de3dab5cc4bc58f335b0df501","unresolved":true,"context_lines":[{"line_number":1193,"context_line":"        super().__init__(api)"},{"line_number":1194,"context_line":"        self.router \u003d router"},{"line_number":1195,"context_line":"        self.prefix \u003d prefix"},{"line_number":1196,"context_line":"        self.nexthop \u003d nexthop"},{"line_number":1197,"context_line":"        self.if_exists \u003d if_exists"},{"line_number":1198,"context_line":""},{"line_number":1199,"context_line":"    def run_idl(self, txn):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3ccf0636_4e3f2b1f","line":1196,"in_reply_to":"0c05738e_b29acff3","updated":"2023-10-04 19:58:19.000000000","message":"I should note that if you don\u0027t have time to add this, I\u0027m not opposed to just doing it sometime as a follow-up patch.","commit_id":"033b6e3bd217c3e424e13f94893d1cb112abf87c"},{"author":{"_account_id":33871,"name":"Anton Vazhnetsov","display_name":"Anton Vazhnetsov","email":"dragen15051@gmail.com","username":"0x5b"},"change_message_id":"6f505b259b8bf34aa560559135ec129fc2a3359e","unresolved":true,"context_lines":[{"line_number":1193,"context_line":"        super().__init__(api)"},{"line_number":1194,"context_line":"        self.router \u003d router"},{"line_number":1195,"context_line":"        self.prefix \u003d prefix"},{"line_number":1196,"context_line":"        self.nexthop \u003d nexthop"},{"line_number":1197,"context_line":"        self.if_exists \u003d if_exists"},{"line_number":1198,"context_line":""},{"line_number":1199,"context_line":"    def run_idl(self, txn):"}],"source_content_type":"text/x-python","patch_set":6,"id":"1abd1e8e_2b75ad2f","line":1196,"in_reply_to":"3ccf0636_4e3f2b1f","updated":"2023-10-04 21:20:51.000000000","message":"I thought about validation, but in the worst case it will return an error that the route was not found, so I did not complicate the code.","commit_id":"033b6e3bd217c3e424e13f94893d1cb112abf87c"}],"ovsdbapp/tests/functional/schema/ovn_northbound/test_impl_idl.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"d4856110e0f97d7f48afb9c656b834996c6e3a96","unresolved":true,"context_lines":[{"line_number":1040,"context_line":"        self._lr_add_route(lr.uuid, prefix\u003dprefix, nexthop\u003dnexthop1)"},{"line_number":1041,"context_line":"        ecmp_route \u003d self._lr_add_route(lr.uuid, prefix\u003dprefix,"},{"line_number":1042,"context_line":"                                        nexthop\u003dnexthop2, ecmp\u003dTrue)"},{"line_number":1043,"context_line":"        self.assertEqual("},{"line_number":1044,"context_line":"            len(self.api.tables[\u0027Logical_Router_Static_Route\u0027].rows), 2)"},{"line_number":1045,"context_line":""},{"line_number":1046,"context_line":"        self.api.lr_route_del(lr.uuid, prefix, nexthop\u003dnexthop2).execute("},{"line_number":1047,"context_line":"            check_error\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":2,"id":"17021936_24101601","line":1044,"range":{"start_line":1043,"start_character":0,"end_line":1044,"end_character":72},"updated":"2023-09-28 06:07:14.000000000","message":"as you are asserting here that there is 2 entries, perhaps worth asserting there is still one after the deletion","commit_id":"a33db3562dec703a38ff8a9a70fc7b7eda6c152b"},{"author":{"_account_id":33871,"name":"Anton Vazhnetsov","display_name":"Anton Vazhnetsov","email":"dragen15051@gmail.com","username":"0x5b"},"change_message_id":"23058fc6e975f87fd2b62560e406b9c4742848e7","unresolved":false,"context_lines":[{"line_number":1040,"context_line":"        self._lr_add_route(lr.uuid, prefix\u003dprefix, nexthop\u003dnexthop1)"},{"line_number":1041,"context_line":"        ecmp_route \u003d self._lr_add_route(lr.uuid, prefix\u003dprefix,"},{"line_number":1042,"context_line":"                                        nexthop\u003dnexthop2, ecmp\u003dTrue)"},{"line_number":1043,"context_line":"        self.assertEqual("},{"line_number":1044,"context_line":"            len(self.api.tables[\u0027Logical_Router_Static_Route\u0027].rows), 2)"},{"line_number":1045,"context_line":""},{"line_number":1046,"context_line":"        self.api.lr_route_del(lr.uuid, prefix, nexthop\u003dnexthop2).execute("},{"line_number":1047,"context_line":"            check_error\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ca4d21e2_e06a3047","line":1044,"range":{"start_line":1043,"start_character":0,"end_line":1044,"end_character":72},"in_reply_to":"17021936_24101601","updated":"2023-09-28 13:24:49.000000000","message":"Done","commit_id":"a33db3562dec703a38ff8a9a70fc7b7eda6c152b"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"4ca6259ff9b8d3d1ae44c58a0be839657be14e1d","unresolved":true,"context_lines":[{"line_number":1043,"context_line":"        self.assertEqual("},{"line_number":1044,"context_line":"            len(self.api.tables[\u0027Logical_Router_Static_Route\u0027].rows), 2)"},{"line_number":1045,"context_line":""},{"line_number":1046,"context_line":"        self.api.lr_route_del(lr.uuid, prefix, nexthop\u003dnexthop2).execute("},{"line_number":1047,"context_line":"            check_error\u003dTrue)"},{"line_number":1048,"context_line":""},{"line_number":1049,"context_line":"        self.assertNotIn("},{"line_number":1050,"context_line":"            ecmp_route.uuid,"}],"source_content_type":"text/x-python","patch_set":4,"id":"54e2e247_a322eadc","line":1047,"range":{"start_line":1046,"start_character":8,"end_line":1047,"end_character":29},"updated":"2023-10-03 14:29:00.000000000","message":"You are checking only the positive case. Please add a test failing to find the expected \"nexthop\"","commit_id":"61cf813855f3ab780158e8ef2d41848209641375"},{"author":{"_account_id":33871,"name":"Anton Vazhnetsov","display_name":"Anton Vazhnetsov","email":"dragen15051@gmail.com","username":"0x5b"},"change_message_id":"aa6dfeb70e4ad0d571200e098ed007227501427f","unresolved":false,"context_lines":[{"line_number":1043,"context_line":"        self.assertEqual("},{"line_number":1044,"context_line":"            len(self.api.tables[\u0027Logical_Router_Static_Route\u0027].rows), 2)"},{"line_number":1045,"context_line":""},{"line_number":1046,"context_line":"        self.api.lr_route_del(lr.uuid, prefix, nexthop\u003dnexthop2).execute("},{"line_number":1047,"context_line":"            check_error\u003dTrue)"},{"line_number":1048,"context_line":""},{"line_number":1049,"context_line":"        self.assertNotIn("},{"line_number":1050,"context_line":"            ecmp_route.uuid,"}],"source_content_type":"text/x-python","patch_set":4,"id":"52b762cd_c27f160a","line":1047,"range":{"start_line":1046,"start_character":8,"end_line":1047,"end_character":29},"in_reply_to":"54e2e247_a322eadc","updated":"2023-10-03 15:28:54.000000000","message":"Done","commit_id":"61cf813855f3ab780158e8ef2d41848209641375"}]}