)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"526a6dddff7574483349ec059fe9f68295cb3e2c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"115bfe18_6fd2cd6a","updated":"2024-07-31 09:36:12.000000000","message":"Can you open a LP bug describing the issue? What registers are affected, what is the expected output, etc.","commit_id":"2659a0354934d96e5ac250e5e12447a9c638c375"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"7fe43026f6aa241bf4e2c8b839a9420a0e274c57","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"6eff1a53_d551c07f","in_reply_to":"115bfe18_6fd2cd6a","updated":"2024-07-31 13:03:26.000000000","message":"That would be https://bugs.launchpad.net/neutron/+bug/2045415, will add it up in next patch update","commit_id":"2659a0354934d96e5ac250e5e12447a9c638c375"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"0468c9dfb632592d3869f1768ecd8b334006f8c6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"d6281712_f566f864","updated":"2024-08-06 09:12:34.000000000","message":"need add unit tests","commit_id":"e5c43663924c82facbbc3913dabb48e6de33ee09"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"ff81cb76d07e34bf37a147e5054f6b5c9e99290b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"1c24372c_f113c523","updated":"2024-08-07 17:02:17.000000000","message":"tobe add more unit test coverage","commit_id":"a8809d45d61928d13ca61e864956b65f2fafdbe5"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"ba0f1b52fd2763cac0ea1c16427936800e290375","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"b9bdf253_379df88a","updated":"2024-08-08 09:29:57.000000000","message":"recheck","commit_id":"496f31d474cfd22d222092384d673f575790bf61"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"79a14f0e93092726ba650a0eac271fb09e253cf6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"2cbe8243_e7af9f07","updated":"2024-08-28 16:27:09.000000000","message":"First of all, thank you very much, Rico, for working on this. It took me some time to understand and review the logic of the patch, some comments over the code.","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"c8327ecdd69c9301ee8f0dd6eda26ff9f52cb89c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"3933451a_33ed98ef","updated":"2024-08-29 05:37:13.000000000","message":"Thanks for the nice review!!! Some comment and refector I made :)","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"09db1e01434e403e5dc741b398330909644a1491","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"d9ee8c6c_3329fe8b","updated":"2024-08-29 09:53:50.000000000","message":"Thx Rico for applying so fast the changes, I give for the moment +1 while waiting for feedback from some other reviewer and meanwhile I\u0027m going to runt current version code and test it manually ‘destroying’ the OVN DBs ... I will come back with some feedback from the manual testing","commit_id":"ec836ec040a8b7be22f0f5e72d24ba916360166a"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"6fc9c2196511748479fb5e4a878108cf619c8a69","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"fa31c9bc_f4b70b18","updated":"2024-08-29 15:51:22.000000000","message":"just updated accordingly, also found a missing pieces in previous patch version, please test against the new one","commit_id":"ec836ec040a8b7be22f0f5e72d24ba916360166a"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"bd6991355c31168876c0382f07b02f5b9f4ee5af","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"f862865b_88694e43","updated":"2024-09-03 11:43:18.000000000","message":"I\u0027m not an expert in ovn-octavia, so my review is not valuable. In any case, the size of this patch is huge. There isn\u0027t a way to push this functionality in several patches? That would help the review process.","commit_id":"172177809b8c9022aebad46a4464427b9093035f"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"1aa97f5e8c9eb76141824eda2b9dbd54195d4ee0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"ab7aaf14_46623fd5","updated":"2024-09-06 11:45:05.000000000","message":"I did some tests removing some small elements from a OVN LB register and looks pretty fine, also the hard one destroying the OVN LB entries and they were recovery correctly. The only point that I concern is the comment in driver.py#L638, I need to add a time.sleep there to be able to test it. -1 just to cover this last point.\n\nAlso need to change the `member.subnet_id \u003d loadbalancer.vip_network_id` already commented in previous one.","commit_id":"03b94643464c5eb9215111ebd33150f0dc0f30f8"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"8c51d1d000b8200481bdf6ec6535f4b87fa8b8e8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"30918f41_343cf812","updated":"2024-09-09 16:48:19.000000000","message":"After second round of tests I saw the FIP are not restored in case you destroy the row in OVN DB, basically because this is an info from Neutron not managed by Octavia DB, but if you just clean the vips field, FIP is still in external_ids and correctly recreate in the final object on OVNB. -1 to discuss this point.","commit_id":"2a15461e219a1e508776c7208335e0b0ee39dbf0"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"cd4f8cfaaf3c17e3f6d025ff02b7c5902070c183","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"346a46e8_b7c5b5b6","updated":"2024-09-10 12:17:52.000000000","message":"I guess FIP should goes into current flow as neutron ovn db sync command doesn\u0027t care about FIP in loadbalancer external_ids.","commit_id":"2a15461e219a1e508776c7208335e0b0ee39dbf0"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"9bde3bde8f9bf9006ab698a116a0aca18078f66b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"834aab9e_abfb5345","in_reply_to":"19f319c9_0cdbac0d","updated":"2024-09-12 13:39:36.000000000","message":"Implemented in https://review.opendev.org/c/openstack/ovn-octavia-provider/+/929039","commit_id":"2a15461e219a1e508776c7208335e0b0ee39dbf0"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"a1c160fb276e19c167c9b811e7209f2f2c4c7b11","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"19f319c9_0cdbac0d","in_reply_to":"346a46e8_b7c5b5b6","updated":"2024-09-10 13:23:46.000000000","message":"yeah, neutron ovn db sync tool doesn\u0027t take care of any FIP [1]\n\n[1] https://opendev.org/openstack/neutron/src/branch/master/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py#L1103","commit_id":"2a15461e219a1e508776c7208335e0b0ee39dbf0"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"4cc748892fe1e05c76a803fca82f0b2d5773c6ca","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"3deb0f0c_49eb8f05","updated":"2024-09-24 15:10:14.000000000","message":"recheck","commit_id":"c71a81fdd60ffd3d14ec7cac6b03aa903e770e17"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"680e72627788b067f8c8d0feb28aed9c3aa4ece4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"f17f54ab_6f1e85ac","updated":"2024-09-24 15:30:47.000000000","message":"I will during today and tomorrow last round of patchsets. \n\nThe pep8 errors should be fix here https://review.opendev.org/c/openstack/ovn-octavia-provider/+/930347","commit_id":"d4b20b2b6e8b6663d123e0eb3db04b6d4c354d6e"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"a7f0387cb70bf1d87524fdf588b22f46f1284d5e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":29,"id":"246ee087_723f0958","updated":"2024-10-17 11:21:59.000000000","message":"did not complete the review, just added some nits here and there","commit_id":"74abe5c765262cbd9387474ba1c291c5e88cbc07"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"164186ea9c15879a912881ca717cd316e40ab8cb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":34,"id":"4e5ac9bf_3cf63255","updated":"2025-01-14 20:48:03.000000000","message":"recheck ovn-octavia-provider-tempest-release unrelated","commit_id":"4f52590bc5989dce7236eecc136200c8ba52c541"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"941dc64d37e15d199ed34e816e44d7ac9697ea33","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"8730111b_c72dcdd8","updated":"2025-01-28 09:14:29.000000000","message":"recheck ovn-octavia-provider-functional-release unrelated","commit_id":"25095aa73de7b2334006e3276174e2fd94144143"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"6ebc5c9254a42bc1af0951192e9d255b871a3cdc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":43,"id":"56f24133_4e481c8d","updated":"2025-02-07 21:44:21.000000000","message":"recheck","commit_id":"07b4c4b919406d200d0aeda86e0b0621663d8e28"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"6a09e9f68ec81a371a355ac17d7e88ec7d5ac408","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":43,"id":"025e8297_5320d873","updated":"2025-02-10 09:59:55.000000000","message":"recheck ovn-octavia-provider-functional-master unrelated","commit_id":"07b4c4b919406d200d0aeda86e0b0621663d8e28"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"08ca5e173fa50f8ef20f6630e5855947027d205e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":43,"id":"47d245fc_88cd5318","updated":"2025-02-10 12:37:15.000000000","message":"recheck ovn-octavia-provider-functional-master unrelated","commit_id":"07b4c4b919406d200d0aeda86e0b0621663d8e28"}],"ovn_octavia_provider/cmd/octavia_ovn_db_sync_util.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"a7f0387cb70bf1d87524fdf588b22f46f1284d5e","unresolved":true,"context_lines":[{"line_number":41,"context_line":"    with the Octavia database."},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    \"\"\""},{"line_number":44,"context_line":"    setup_conf()"},{"line_number":45,"context_line":"    logging.setup(CONF, \u0027octavia_ovn_db_sync_util\u0027)"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    LOG.info(\"OVN Octavia DB sync start.\")"},{"line_number":48,"context_line":"    # Method can be call like `octavia-ovn-db-sync-util --debug`"}],"source_content_type":"text/x-python","patch_set":29,"id":"32f8347a_66a8d1b4","line":45,"range":{"start_line":44,"start_character":16,"end_line":45,"end_character":51},"updated":"2024-10-17 11:21:59.000000000","message":"the changes in this file perhaps belong to the other patch: https://review.opendev.org/c/openstack/ovn-octavia-provider/+/925747","commit_id":"74abe5c765262cbd9387474ba1c291c5e88cbc07"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"411f419657ddd0fd1e4a3d6718a6b0f83eb8b921","unresolved":false,"context_lines":[{"line_number":41,"context_line":"    with the Octavia database."},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    \"\"\""},{"line_number":44,"context_line":"    setup_conf()"},{"line_number":45,"context_line":"    logging.setup(CONF, \u0027octavia_ovn_db_sync_util\u0027)"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    LOG.info(\"OVN Octavia DB sync start.\")"},{"line_number":48,"context_line":"    # Method can be call like `octavia-ovn-db-sync-util --debug`"}],"source_content_type":"text/x-python","patch_set":29,"id":"964402cc_4c3988ae","line":45,"range":{"start_line":44,"start_character":16,"end_line":45,"end_character":51},"in_reply_to":"32f8347a_66a8d1b4","updated":"2024-10-29 17:31:58.000000000","message":"Done","commit_id":"74abe5c765262cbd9387474ba1c291c5e88cbc07"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"cf87e708c6cf6898731136cd310e5a2568818e23","unresolved":true,"context_lines":[{"line_number":44,"context_line":"    setup_conf()"},{"line_number":45,"context_line":"    logging.setup(CONF, \u0027octavia_ovn_db_sync_util\u0027)"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    LOG.info(\"OVN Octavia DB sync start.\")"},{"line_number":48,"context_line":"    # Method can be call like `octavia-ovn-db-sync-util --debug`"},{"line_number":49,"context_line":"    LOG.info(\"OVN Octavia DB sync start.\")"},{"line_number":50,"context_line":"    args \u003d sys.argv[1:]"}],"source_content_type":"text/x-python","patch_set":30,"id":"0f6940a4_49855591","line":47,"updated":"2024-11-13 09:37:41.000000000","message":"why duplicating this log? It is the same as in L49","commit_id":"f01257b0cdc52fb0b6a95c7ea69e59d6b5b30d74"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"081fd3fbb60d4b1612527c77b962ef30974b2947","unresolved":false,"context_lines":[{"line_number":44,"context_line":"    setup_conf()"},{"line_number":45,"context_line":"    logging.setup(CONF, \u0027octavia_ovn_db_sync_util\u0027)"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"    LOG.info(\"OVN Octavia DB sync start.\")"},{"line_number":48,"context_line":"    # Method can be call like `octavia-ovn-db-sync-util --debug`"},{"line_number":49,"context_line":"    LOG.info(\"OVN Octavia DB sync start.\")"},{"line_number":50,"context_line":"    args \u003d sys.argv[1:]"}],"source_content_type":"text/x-python","patch_set":30,"id":"680cedb7_b800c228","line":47,"in_reply_to":"0f6940a4_49855591","updated":"2024-11-19 11:53:15.000000000","message":"Done","commit_id":"f01257b0cdc52fb0b6a95c7ea69e59d6b5b30d74"}],"ovn_octavia_provider/driver.py":[{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"79a14f0e93092726ba650a0eac271fb09e253cf6","unresolved":true,"context_lines":[{"line_number":167,"context_line":"            request[\u0027info\u0027][\u0027session_persistence\u0027] \u003d pool.session_persistence"},{"line_number":168,"context_line":"        self._ovn_helper.add_request(request)"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    def pool_sync(self, pool):"},{"line_number":171,"context_line":"        self._pool_create_or_sync(pool, req_type\u003dovn_const.REQ_TYPE_POOL_SYNC)"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"    def pool_create(self, pool):"}],"source_content_type":"text/x-python","patch_set":12,"id":"e6dfb351_f8d61108","line":170,"updated":"2024-08-28 16:27:09.000000000","message":"To try to be as closed as possible to octavia_lib.api.drivers.provider_base [1] maybe these XXX_sync methods should be private, wdyt?\n\n[1] https://github.com/openstack/octavia-lib/blob/master/octavia_lib/api/drivers/provider_base.py","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"c8327ecdd69c9301ee8f0dd6eda26ff9f52cb89c","unresolved":false,"context_lines":[{"line_number":167,"context_line":"            request[\u0027info\u0027][\u0027session_persistence\u0027] \u003d pool.session_persistence"},{"line_number":168,"context_line":"        self._ovn_helper.add_request(request)"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    def pool_sync(self, pool):"},{"line_number":171,"context_line":"        self._pool_create_or_sync(pool, req_type\u003dovn_const.REQ_TYPE_POOL_SYNC)"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"    def pool_create(self, pool):"}],"source_content_type":"text/x-python","patch_set":12,"id":"170d60d4_f8eb9a97","line":170,"in_reply_to":"e6dfb351_f8d61108","updated":"2024-08-29 05:37:13.000000000","message":"sure","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"79a14f0e93092726ba650a0eac271fb09e253cf6","unresolved":true,"context_lines":[{"line_number":214,"context_line":"    def listener_create(self, listener):"},{"line_number":215,"context_line":"        self._listener_create_or_sync(listener)"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    def listener_sync(self, listener, is_sync\u003dTrue):"},{"line_number":218,"context_line":"        self._listener_create_or_sync("},{"line_number":219,"context_line":"            listener,"},{"line_number":220,"context_line":"            req_type\u003dovn_const.REQ_TYPE_LISTENER_SYNC)"}],"source_content_type":"text/x-python","patch_set":12,"id":"ac7a31c8_36bef861","line":217,"updated":"2024-08-28 16:27:09.000000000","message":"^idem","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"c8327ecdd69c9301ee8f0dd6eda26ff9f52cb89c","unresolved":false,"context_lines":[{"line_number":214,"context_line":"    def listener_create(self, listener):"},{"line_number":215,"context_line":"        self._listener_create_or_sync(listener)"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    def listener_sync(self, listener, is_sync\u003dTrue):"},{"line_number":218,"context_line":"        self._listener_create_or_sync("},{"line_number":219,"context_line":"            listener,"},{"line_number":220,"context_line":"            req_type\u003dovn_const.REQ_TYPE_LISTENER_SYNC)"}],"source_content_type":"text/x-python","patch_set":12,"id":"3077c525_6337110e","line":217,"in_reply_to":"ac7a31c8_36bef861","updated":"2024-08-29 05:37:13.000000000","message":"done","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"79a14f0e93092726ba650a0eac271fb09e253cf6","unresolved":true,"context_lines":[{"line_number":351,"context_line":"                   \u0027info\u0027: request_info}"},{"line_number":352,"context_line":"        self._ovn_helper.add_request(request)"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"    def member_sync(self, member):"},{"line_number":355,"context_line":"        return self._member_create_or_sync("},{"line_number":356,"context_line":"            member, req_type\u003dovn_const.REQ_TYPE_MEMBER_SYNC)"},{"line_number":357,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"d88d7696_15d703d0","line":354,"updated":"2024-08-28 16:27:09.000000000","message":"^idem","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"c8327ecdd69c9301ee8f0dd6eda26ff9f52cb89c","unresolved":false,"context_lines":[{"line_number":351,"context_line":"                   \u0027info\u0027: request_info}"},{"line_number":352,"context_line":"        self._ovn_helper.add_request(request)"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":"    def member_sync(self, member):"},{"line_number":355,"context_line":"        return self._member_create_or_sync("},{"line_number":356,"context_line":"            member, req_type\u003dovn_const.REQ_TYPE_MEMBER_SYNC)"},{"line_number":357,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"7f530418_12874728","line":354,"in_reply_to":"d88d7696_15d703d0","updated":"2024-08-29 05:37:13.000000000","message":"done","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"79a14f0e93092726ba650a0eac271fb09e253cf6","unresolved":true,"context_lines":[{"line_number":599,"context_line":"    def health_monitor_create(self, healthmonitor):"},{"line_number":600,"context_line":"        self._health_monitor_create_or_sync(healthmonitor)"},{"line_number":601,"context_line":""},{"line_number":602,"context_line":"    def health_monitor_sync(self, healthmonitor):"},{"line_number":603,"context_line":"        self._health_monitor_create_or_sync("},{"line_number":604,"context_line":"            healthmonitor, req_type\u003dovn_const.REQ_TYPE_HM_SYNC)"},{"line_number":605,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"1639dfae_b078649f","line":602,"updated":"2024-08-28 16:27:09.000000000","message":"^idem","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"c8327ecdd69c9301ee8f0dd6eda26ff9f52cb89c","unresolved":false,"context_lines":[{"line_number":599,"context_line":"    def health_monitor_create(self, healthmonitor):"},{"line_number":600,"context_line":"        self._health_monitor_create_or_sync(healthmonitor)"},{"line_number":601,"context_line":""},{"line_number":602,"context_line":"    def health_monitor_sync(self, healthmonitor):"},{"line_number":603,"context_line":"        self._health_monitor_create_or_sync("},{"line_number":604,"context_line":"            healthmonitor, req_type\u003dovn_const.REQ_TYPE_HM_SYNC)"},{"line_number":605,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"06aa41f0_9a64659f","line":602,"in_reply_to":"1639dfae_b078649f","updated":"2024-08-29 05:37:13.000000000","message":"done","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"79a14f0e93092726ba650a0eac271fb09e253cf6","unresolved":true,"context_lines":[{"line_number":629,"context_line":"    def loadbalancer_sync(self, loadbalancer):"},{"line_number":630,"context_line":"        ovn_lbs \u003d self._ovn_helper.find_ovn_lbs(loadbalancer.loadbalancer_id)"},{"line_number":631,"context_line":"        # Load Balancer"},{"line_number":632,"context_line":"        if not ovn_lbs:"},{"line_number":633,"context_line":"            LOG.debug("},{"line_number":634,"context_line":"                \"Start creating missing loadbalancer \""},{"line_number":635,"context_line":"                f\"{loadbalancer.loadbalancer_id} in OVN.\")"}],"source_content_type":"text/x-python","patch_set":12,"id":"62911e93_c4bbdc12","line":632,"updated":"2024-08-28 16:27:09.000000000","message":"This seems to be the starting point of an OVN LB sync, if we don\u0027t find the LB, it is clear that there will be no listener/pool/members/hm, so we could already make a decision to create it completely, not being necessary to go later if we find the OVN LB or not checking individually listener, pool, members... This way the sync would be:\n\n1) Do we find the OVN LB?\n   - No -\u003e Create it complete (LB+listener+pool+member+hm)\n   - Yes -\u003e Sync LB parameters and go down one hierarchy level (listener)\n   2) Do we find the listener in the OVN LB found in 1)?\n       - No -\u003e Create it complete (listener+pool+member+hm)\n       - Yes -\u003e sync listener parameters and down one hierarchy level (pool)   \n...............\n\nFeel free to comment, because maybe you already find an issue following that process that I don\u0027t","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"c8327ecdd69c9301ee8f0dd6eda26ff9f52cb89c","unresolved":true,"context_lines":[{"line_number":629,"context_line":"    def loadbalancer_sync(self, loadbalancer):"},{"line_number":630,"context_line":"        ovn_lbs \u003d self._ovn_helper.find_ovn_lbs(loadbalancer.loadbalancer_id)"},{"line_number":631,"context_line":"        # Load Balancer"},{"line_number":632,"context_line":"        if not ovn_lbs:"},{"line_number":633,"context_line":"            LOG.debug("},{"line_number":634,"context_line":"                \"Start creating missing loadbalancer \""},{"line_number":635,"context_line":"                f\"{loadbalancer.loadbalancer_id} in OVN.\")"}],"source_content_type":"text/x-python","patch_set":12,"id":"e506812c_825df5df","line":632,"in_reply_to":"62911e93_c4bbdc12","updated":"2024-08-29 05:37:13.000000000","message":"I think the current codebase already honor check (1). With  ovn_lbs not found, it pretty much go through LB+listener+pool+member+hm and do only creates.\nFor (2), I think with listener not found, there still be some data already exists in lb, like `pools`, so in this case, we should run through pool sync to make sure we didn\u0027t put redundant data inside lb.","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"6fc9c2196511748479fb5e4a878108cf619c8a69","unresolved":false,"context_lines":[{"line_number":629,"context_line":"    def loadbalancer_sync(self, loadbalancer):"},{"line_number":630,"context_line":"        ovn_lbs \u003d self._ovn_helper.find_ovn_lbs(loadbalancer.loadbalancer_id)"},{"line_number":631,"context_line":"        # Load Balancer"},{"line_number":632,"context_line":"        if not ovn_lbs:"},{"line_number":633,"context_line":"            LOG.debug("},{"line_number":634,"context_line":"                \"Start creating missing loadbalancer \""},{"line_number":635,"context_line":"                f\"{loadbalancer.loadbalancer_id} in OVN.\")"}],"source_content_type":"text/x-python","patch_set":12,"id":"8c72d55d_985ceeda","line":632,"in_reply_to":"d76fc5fb_3f966fcc","updated":"2024-08-29 15:51:22.000000000","message":"I added that logic to new patch now","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"09db1e01434e403e5dc741b398330909644a1491","unresolved":true,"context_lines":[{"line_number":629,"context_line":"    def loadbalancer_sync(self, loadbalancer):"},{"line_number":630,"context_line":"        ovn_lbs \u003d self._ovn_helper.find_ovn_lbs(loadbalancer.loadbalancer_id)"},{"line_number":631,"context_line":"        # Load Balancer"},{"line_number":632,"context_line":"        if not ovn_lbs:"},{"line_number":633,"context_line":"            LOG.debug("},{"line_number":634,"context_line":"                \"Start creating missing loadbalancer \""},{"line_number":635,"context_line":"                f\"{loadbalancer.loadbalancer_id} in OVN.\")"}],"source_content_type":"text/x-python","patch_set":12,"id":"d76fc5fb_3f966fcc","line":632,"in_reply_to":"e506812c_825df5df","updated":"2024-08-29 09:53:50.000000000","message":"Perhaps I am not explaining myself well. What I mean is that if in the L633 we don\u0027t find the OVN LB we could call self.loadbalancer_create(loadbalancer) instead of self._loadbalancer_create(loadbalancer) and that change would save us the blocks L644-L649 and L655-L660 and L680-L686. \n\nAnyway I will run current code on a devstack and make some manually testing to evaluate.","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"79a14f0e93092726ba650a0eac271fb09e253cf6","unresolved":true,"context_lines":[{"line_number":670,"context_line":""},{"line_number":671,"context_line":"                # Member"},{"line_number":672,"context_line":"                ovn_pool_key, ovn_pool_lb \u003d ("},{"line_number":673,"context_line":"                    self._ovn_helper.find_ovn_lb_by_pool_id_with_retry("},{"line_number":674,"context_line":"                        pool.pool_id, ovn_lbs"},{"line_number":675,"context_line":"                    )"},{"line_number":676,"context_line":"                )"}],"source_content_type":"text/x-python","patch_set":12,"id":"e71f59a2_f82b25ac","line":673,"updated":"2024-08-28 16:27:09.000000000","message":"Sometimes private methods are called, sometimes the public ones, dont clear to me what is the criterium here to use one of the refactored ones in the helper class.","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"c8327ecdd69c9301ee8f0dd6eda26ff9f52cb89c","unresolved":false,"context_lines":[{"line_number":670,"context_line":""},{"line_number":671,"context_line":"                # Member"},{"line_number":672,"context_line":"                ovn_pool_key, ovn_pool_lb \u003d ("},{"line_number":673,"context_line":"                    self._ovn_helper.find_ovn_lb_by_pool_id_with_retry("},{"line_number":674,"context_line":"                        pool.pool_id, ovn_lbs"},{"line_number":675,"context_line":"                    )"},{"line_number":676,"context_line":"                )"}],"source_content_type":"text/x-python","patch_set":12,"id":"3d390287_c01d04ab","line":673,"in_reply_to":"e71f59a2_f82b25ac","updated":"2024-08-29 05:37:13.000000000","message":"I think this is something we can do cleanup and just use _find_ovn_lb_by_pool_id_with_retry here","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"bd6991355c31168876c0382f07b02f5b9f4ee5af","unresolved":true,"context_lines":[{"line_number":667,"context_line":"                member_ids \u003d []"},{"line_number":668,"context_line":"                for member in pool.members:"},{"line_number":669,"context_line":"                    if not member.subnet_id:"},{"line_number":670,"context_line":"                        member.subnet_id \u003d loadbalancer.vip_network_id"},{"line_number":671,"context_line":"                    LOG.debug("},{"line_number":672,"context_line":"                        \"Start sync member \""},{"line_number":673,"context_line":"                        f\"{member.member_id} from pool \""}],"source_content_type":"text/x-python","patch_set":14,"id":"3266ab57_b0df0ea9","line":670,"range":{"start_line":670,"start_character":23,"end_line":670,"end_character":70},"updated":"2024-09-03 11:43:18.000000000","message":"I don\u0027t undestand why are you assigning a netID on a subnetID","commit_id":"172177809b8c9022aebad46a4464427b9093035f"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"0c8ab3176bbfd6d037c78d783566ece186dfaa73","unresolved":false,"context_lines":[{"line_number":667,"context_line":"                member_ids \u003d []"},{"line_number":668,"context_line":"                for member in pool.members:"},{"line_number":669,"context_line":"                    if not member.subnet_id:"},{"line_number":670,"context_line":"                        member.subnet_id \u003d loadbalancer.vip_network_id"},{"line_number":671,"context_line":"                    LOG.debug("},{"line_number":672,"context_line":"                        \"Start sync member \""},{"line_number":673,"context_line":"                        f\"{member.member_id} from pool \""}],"source_content_type":"text/x-python","patch_set":14,"id":"71f21345_136036af","line":670,"range":{"start_line":670,"start_character":23,"end_line":670,"end_character":70},"in_reply_to":"3266ab57_b0df0ea9","updated":"2024-09-06 04:51:30.000000000","message":"It\u0027s a direct copy from \nhttps://review.opendev.org/c/openstack/ovn-octavia-provider/+/925324/14/ovn_octavia_provider/driver.py#b118\n\nMy best guess is there are cases that you can assign member network and with no IP address? (no subnet forced)","commit_id":"172177809b8c9022aebad46a4464427b9093035f"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"055e5bbf441c13962c7d4a3db5a4741bdc00f321","unresolved":false,"context_lines":[{"line_number":667,"context_line":"                member_ids \u003d []"},{"line_number":668,"context_line":"                for member in pool.members:"},{"line_number":669,"context_line":"                    if not member.subnet_id:"},{"line_number":670,"context_line":"                        member.subnet_id \u003d loadbalancer.vip_network_id"},{"line_number":671,"context_line":"                    LOG.debug("},{"line_number":672,"context_line":"                        \"Start sync member \""},{"line_number":673,"context_line":"                        f\"{member.member_id} from pool \""}],"source_content_type":"text/x-python","patch_set":14,"id":"92702505_f46132bb","line":670,"range":{"start_line":670,"start_character":23,"end_line":670,"end_character":70},"in_reply_to":"714c4c65_e76b2dff","updated":"2024-09-09 09:00:49.000000000","message":"changed","commit_id":"172177809b8c9022aebad46a4464427b9093035f"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"424d38214c3cb39189fff4dfbd91c8ef26d8c974","unresolved":true,"context_lines":[{"line_number":667,"context_line":"                member_ids \u003d []"},{"line_number":668,"context_line":"                for member in pool.members:"},{"line_number":669,"context_line":"                    if not member.subnet_id:"},{"line_number":670,"context_line":"                        member.subnet_id \u003d loadbalancer.vip_network_id"},{"line_number":671,"context_line":"                    LOG.debug("},{"line_number":672,"context_line":"                        \"Start sync member \""},{"line_number":673,"context_line":"                        f\"{member.member_id} from pool \""}],"source_content_type":"text/x-python","patch_set":14,"id":"714c4c65_e76b2dff","line":670,"range":{"start_line":670,"start_character":23,"end_line":670,"end_character":70},"in_reply_to":"71f21345_136036af","updated":"2024-09-06 10:31:37.000000000","message":"Rodolfo is totally right! good catch!\nFix already sent here https://review.opendev.org/c/openstack/ovn-octavia-provider/+/928335","commit_id":"172177809b8c9022aebad46a4464427b9093035f"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"bd6991355c31168876c0382f07b02f5b9f4ee5af","unresolved":true,"context_lines":[{"line_number":706,"context_line":"                        self._ovn_helper._find_ovn_lb_from_hm_id("},{"line_number":707,"context_line":"                            pool.healthmonitor.healthmonitor_id)"},{"line_number":708,"context_line":"                    )"},{"line_number":709,"context_line":"                    if lbhcs is None and ovn_hm_lb is None:"},{"line_number":710,"context_line":"                        self.health_monitor_create(pool.healthmonitor)"},{"line_number":711,"context_line":"                    else:"},{"line_number":712,"context_line":"                        self._health_monitor_sync(pool.healthmonitor)"}],"source_content_type":"text/x-python","patch_set":14,"id":"59cfcbce_aa2d2fdf","line":709,"range":{"start_line":709,"start_character":20,"end_line":709,"end_character":59},"updated":"2024-09-03 11:43:18.000000000","message":"Method ``_find_ovn_lb_from_hm_id`` returns ([], ovn_lb). How can ``lbhcs`` be None?","commit_id":"172177809b8c9022aebad46a4464427b9093035f"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"0c8ab3176bbfd6d037c78d783566ece186dfaa73","unresolved":false,"context_lines":[{"line_number":706,"context_line":"                        self._ovn_helper._find_ovn_lb_from_hm_id("},{"line_number":707,"context_line":"                            pool.healthmonitor.healthmonitor_id)"},{"line_number":708,"context_line":"                    )"},{"line_number":709,"context_line":"                    if lbhcs is None and ovn_hm_lb is None:"},{"line_number":710,"context_line":"                        self.health_monitor_create(pool.healthmonitor)"},{"line_number":711,"context_line":"                    else:"},{"line_number":712,"context_line":"                        self._health_monitor_sync(pool.healthmonitor)"}],"source_content_type":"text/x-python","patch_set":14,"id":"a2e5c5f9_e363dcba","line":709,"range":{"start_line":709,"start_character":20,"end_line":709,"end_character":59},"in_reply_to":"59cfcbce_aa2d2fdf","updated":"2024-09-06 04:51:30.000000000","message":"good catch! just add new version to fix this","commit_id":"172177809b8c9022aebad46a4464427b9093035f"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"1aa97f5e8c9eb76141824eda2b9dbd54195d4ee0","unresolved":true,"context_lines":[{"line_number":635,"context_line":"            LOG.debug(f\"OVN loadbalancer {loadbalancer.loadbalancer_id} \""},{"line_number":636,"context_line":"                      \"not found. Start create process.\")"},{"line_number":637,"context_line":"            self.loadbalancer_create(loadbalancer)"},{"line_number":638,"context_line":"            return"},{"line_number":639,"context_line":"        # Load Balancer"},{"line_number":640,"context_line":"        LOG.debug(f\"Start sync loadbalancer {loadbalancer.loadbalancer_id}.\")"},{"line_number":641,"context_line":"        self._loadbalancer_create(loadbalancer,"}],"source_content_type":"text/x-python","patch_set":15,"id":"b0c10525_6d152e9a","line":638,"updated":"2024-09-06 11:45:05.000000000","message":"On my tests, return statement makes the process exits as soon the self.loadbalancer_create(...) sends the request lb_create to the helper. Assuming this is running on the same thread so finally the OVN LB is not created.","commit_id":"03b94643464c5eb9215111ebd33150f0dc0f30f8"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"6e5651bf693dca7e7f07c8b1af62446de8617589","unresolved":true,"context_lines":[{"line_number":635,"context_line":"            LOG.debug(f\"OVN loadbalancer {loadbalancer.loadbalancer_id} \""},{"line_number":636,"context_line":"                      \"not found. Start create process.\")"},{"line_number":637,"context_line":"            self.loadbalancer_create(loadbalancer)"},{"line_number":638,"context_line":"            return"},{"line_number":639,"context_line":"        # Load Balancer"},{"line_number":640,"context_line":"        LOG.debug(f\"Start sync loadbalancer {loadbalancer.loadbalancer_id}.\")"},{"line_number":641,"context_line":"        self._loadbalancer_create(loadbalancer,"}],"source_content_type":"text/x-python","patch_set":15,"id":"dc5a905e_7a3e4387","line":638,"in_reply_to":"706a4318_afcf09f9","updated":"2024-09-09 16:49:58.000000000","message":"Yeah, I saw it, basically it will recall twice (first one create the OVN LB, second one basically does nothing because first one keeps is completely recovery), not too much efficience solution but I like more than adding the time.sleep there.","commit_id":"03b94643464c5eb9215111ebd33150f0dc0f30f8"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"055e5bbf441c13962c7d4a3db5a4741bdc00f321","unresolved":true,"context_lines":[{"line_number":635,"context_line":"            LOG.debug(f\"OVN loadbalancer {loadbalancer.loadbalancer_id} \""},{"line_number":636,"context_line":"                      \"not found. Start create process.\")"},{"line_number":637,"context_line":"            self.loadbalancer_create(loadbalancer)"},{"line_number":638,"context_line":"            return"},{"line_number":639,"context_line":"        # Load Balancer"},{"line_number":640,"context_line":"        LOG.debug(f\"Start sync loadbalancer {loadbalancer.loadbalancer_id}.\")"},{"line_number":641,"context_line":"        self._loadbalancer_create(loadbalancer,"}],"source_content_type":"text/x-python","patch_set":15,"id":"706a4318_afcf09f9","line":638,"in_reply_to":"b0c10525_6d152e9a","updated":"2024-09-09 09:00:49.000000000","message":"I added a logic to rerun sync on those recreated LBs to make sure they\u0027re created correctly.\n\nAlso to add time.sleep helps in this case, but not sure those hardcode seconds should be.","commit_id":"03b94643464c5eb9215111ebd33150f0dc0f30f8"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"c9b268d9f3bf07deb8d6c7a30f54e711b9b32bc5","unresolved":false,"context_lines":[{"line_number":635,"context_line":"            LOG.debug(f\"OVN loadbalancer {loadbalancer.loadbalancer_id} \""},{"line_number":636,"context_line":"                      \"not found. Start create process.\")"},{"line_number":637,"context_line":"            self.loadbalancer_create(loadbalancer)"},{"line_number":638,"context_line":"            return"},{"line_number":639,"context_line":"        # Load Balancer"},{"line_number":640,"context_line":"        LOG.debug(f\"Start sync loadbalancer {loadbalancer.loadbalancer_id}.\")"},{"line_number":641,"context_line":"        self._loadbalancer_create(loadbalancer,"}],"source_content_type":"text/x-python","patch_set":15,"id":"ef41e544_4b42593a","line":638,"in_reply_to":"dc5a905e_7a3e4387","updated":"2024-09-25 14:33:56.000000000","message":"Done","commit_id":"03b94643464c5eb9215111ebd33150f0dc0f30f8"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"a7f0387cb70bf1d87524fdf588b22f46f1284d5e","unresolved":true,"context_lines":[{"line_number":592,"context_line":"        self._ovn_helper.add_request(request)"},{"line_number":593,"context_line":""},{"line_number":594,"context_line":"    def _ensure_loadbalancer(self, loadbalancer):"},{"line_number":595,"context_line":"        try:"},{"line_number":596,"context_line":"            ovn_lbs \u003d self._ovn_helper._find_ovn_lbs_with_retry("},{"line_number":597,"context_line":"                loadbalancer.loadbalancer_id)"},{"line_number":598,"context_line":"        except idlutils.RowNotFound:"}],"source_content_type":"text/x-python","patch_set":29,"id":"a7f42fee_2c0414d7","line":595,"range":{"start_line":595,"start_character":0,"end_line":595,"end_character":12},"updated":"2024-10-17 11:21:59.000000000","message":"perhaps this could be a try/except/else","commit_id":"74abe5c765262cbd9387474ba1c291c5e88cbc07"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"411f419657ddd0fd1e4a3d6718a6b0f83eb8b921","unresolved":false,"context_lines":[{"line_number":592,"context_line":"        self._ovn_helper.add_request(request)"},{"line_number":593,"context_line":""},{"line_number":594,"context_line":"    def _ensure_loadbalancer(self, loadbalancer):"},{"line_number":595,"context_line":"        try:"},{"line_number":596,"context_line":"            ovn_lbs \u003d self._ovn_helper._find_ovn_lbs_with_retry("},{"line_number":597,"context_line":"                loadbalancer.loadbalancer_id)"},{"line_number":598,"context_line":"        except idlutils.RowNotFound:"}],"source_content_type":"text/x-python","patch_set":29,"id":"dd9a354f_a24dd384","line":595,"range":{"start_line":595,"start_character":0,"end_line":595,"end_character":12},"in_reply_to":"a7f42fee_2c0414d7","updated":"2024-10-29 17:31:58.000000000","message":"Done","commit_id":"74abe5c765262cbd9387474ba1c291c5e88cbc07"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cd1591ebd10ae04421588f62993d73e0cc77059c","unresolved":true,"context_lines":[{"line_number":90,"context_line":"                user_fault_string\u003dmsg,"},{"line_number":91,"context_line":"                operator_fault_string\u003dmsg)"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    def _get_loadbalancer_request_info(self, loadbalancer):"},{"line_number":94,"context_line":"        admin_state_up \u003d loadbalancer.admin_state_up"},{"line_number":95,"context_line":"        if isinstance(admin_state_up, o_datamodels.UnsetType):"},{"line_number":96,"context_line":"            admin_state_up \u003d True"}],"source_content_type":"text/x-python","patch_set":36,"id":"60c82832_ee061c40","line":93,"updated":"2025-01-22 16:50:43.000000000","message":"Would make sense to make this a helper function outside of the class scope as it seems it just builds up a map of data and does not operate over class instance data at all.","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"50b1170ec6491297a5565dcdb5c7197ea2576fcb","unresolved":true,"context_lines":[{"line_number":90,"context_line":"                user_fault_string\u003dmsg,"},{"line_number":91,"context_line":"                operator_fault_string\u003dmsg)"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"    def _get_loadbalancer_request_info(self, loadbalancer):"},{"line_number":94,"context_line":"        admin_state_up \u003d loadbalancer.admin_state_up"},{"line_number":95,"context_line":"        if isinstance(admin_state_up, o_datamodels.UnsetType):"},{"line_number":96,"context_line":"            admin_state_up \u003d True"}],"source_content_type":"text/x-python","patch_set":36,"id":"07577cb5_26e73a4d","line":93,"in_reply_to":"60c82832_ee061c40","updated":"2025-01-23 06:29:32.000000000","message":"something to do as followup I guess, not really looking for putting more complexity on current patch ;)","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cd1591ebd10ae04421588f62993d73e0cc77059c","unresolved":true,"context_lines":[{"line_number":618,"context_line":"        lbs \u003d self._ovn_helper.get_octavia_lbs(octavia_client, **lb_filters)"},{"line_number":619,"context_line":"        for lb in lbs:"},{"line_number":620,"context_line":"            LOG.info(f\"Starting sync OVN DB with Loadbalancer {lb.name}\")"},{"line_number":621,"context_line":"            provider_lb \u003d \\"},{"line_number":622,"context_line":"                self._ovn_helper._octavia_driver_lib.get_loadbalancer(lb.id)"},{"line_number":623,"context_line":"            self._ensure_loadbalancer(provider_lb)"}],"source_content_type":"text/x-python","patch_set":36,"id":"c0bca92d_c2ac51bd","line":621,"updated":"2025-01-22 16:50:43.000000000","message":"```\nIt is preferred to wrap long lines in parentheses and not a backslash for line continuation.\n```\nhttps://docs.openstack.org/hacking/latest/user/hacking.html#general","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"50b1170ec6491297a5565dcdb5c7197ea2576fcb","unresolved":false,"context_lines":[{"line_number":618,"context_line":"        lbs \u003d self._ovn_helper.get_octavia_lbs(octavia_client, **lb_filters)"},{"line_number":619,"context_line":"        for lb in lbs:"},{"line_number":620,"context_line":"            LOG.info(f\"Starting sync OVN DB with Loadbalancer {lb.name}\")"},{"line_number":621,"context_line":"            provider_lb \u003d \\"},{"line_number":622,"context_line":"                self._ovn_helper._octavia_driver_lib.get_loadbalancer(lb.id)"},{"line_number":623,"context_line":"            self._ensure_loadbalancer(provider_lb)"}],"source_content_type":"text/x-python","patch_set":36,"id":"7e7525e8_1e2f897a","line":621,"in_reply_to":"c0bca92d_c2ac51bd","updated":"2025-01-23 06:29:32.000000000","message":"Done","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"}],"ovn_octavia_provider/helper.py":[{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"79a14f0e93092726ba650a0eac271fb09e253cf6","unresolved":true,"context_lines":[{"line_number":454,"context_line":"    def _find_ovn_lbs_with_retry(self, lb_id, protocol\u003dNone):"},{"line_number":455,"context_line":"        return self._find_ovn_lbs(lb_id, protocol\u003dprotocol)"},{"line_number":456,"context_line":""},{"line_number":457,"context_line":"    def find_ovn_lbs(self, lb_id, protocol\u003dNone):"},{"line_number":458,"context_line":"        try:"},{"line_number":459,"context_line":"            return self._find_ovn_lbs(lb_id, protocol\u003dprotocol)"},{"line_number":460,"context_line":"        except idlutils.RowNotFound:"}],"source_content_type":"text/x-python","patch_set":12,"id":"496f6727_a297eb47","line":457,"updated":"2024-08-28 16:27:09.000000000","message":"why don\u0027t use the existing one some lines above? the tenacity.retry will return the same exception in case it achieves the max timeout and would retry in case of any issue when requesting objects to ovsdbapp.","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"c8327ecdd69c9301ee8f0dd6eda26ff9f52cb89c","unresolved":false,"context_lines":[{"line_number":454,"context_line":"    def _find_ovn_lbs_with_retry(self, lb_id, protocol\u003dNone):"},{"line_number":455,"context_line":"        return self._find_ovn_lbs(lb_id, protocol\u003dprotocol)"},{"line_number":456,"context_line":""},{"line_number":457,"context_line":"    def find_ovn_lbs(self, lb_id, protocol\u003dNone):"},{"line_number":458,"context_line":"        try:"},{"line_number":459,"context_line":"            return self._find_ovn_lbs(lb_id, protocol\u003dprotocol)"},{"line_number":460,"context_line":"        except idlutils.RowNotFound:"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a98482d_36645369","line":457,"in_reply_to":"496f6727_a297eb47","updated":"2024-08-29 05:37:13.000000000","message":"done","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"79a14f0e93092726ba650a0eac271fb09e253cf6","unresolved":true,"context_lines":[{"line_number":580,"context_line":"        # or updated exising, empty one."},{"line_number":581,"context_line":"        return self._find_ovn_lbs(lb_id, protocol\u003dprotocol)"},{"line_number":582,"context_line":""},{"line_number":583,"context_line":"    def _find_ovn_lb_with_pool_key(self, pool_key, loadbalancers\u003d[]):"},{"line_number":584,"context_line":"        if not loadbalancers:"},{"line_number":585,"context_line":"            loadbalancers \u003d self.ovn_nbdb_api.db_list_rows("},{"line_number":586,"context_line":"                \u0027Load_Balancer\u0027).execute(check_error\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":12,"id":"4732e609_57f7fef2","line":583,"updated":"2024-08-28 16:27:09.000000000","message":"Why are you adding this parameter with a default value \u003d [] if no other point in the code are calling it with a value?","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"c8327ecdd69c9301ee8f0dd6eda26ff9f52cb89c","unresolved":true,"context_lines":[{"line_number":580,"context_line":"        # or updated exising, empty one."},{"line_number":581,"context_line":"        return self._find_ovn_lbs(lb_id, protocol\u003dprotocol)"},{"line_number":582,"context_line":""},{"line_number":583,"context_line":"    def _find_ovn_lb_with_pool_key(self, pool_key, loadbalancers\u003d[]):"},{"line_number":584,"context_line":"        if not loadbalancers:"},{"line_number":585,"context_line":"            loadbalancers \u003d self.ovn_nbdb_api.db_list_rows("},{"line_number":586,"context_line":"                \u0027Load_Balancer\u0027).execute(check_error\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa4f752e_3bc8b60e","line":583,"in_reply_to":"4732e609_57f7fef2","updated":"2024-08-29 05:37:13.000000000","message":"the calls with `loadbalancers` are all in driver.py \nlike https://review.opendev.org/c/openstack/ovn-octavia-provider/+/925324/12/ovn_octavia_provider/driver.py#660","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"09db1e01434e403e5dc741b398330909644a1491","unresolved":false,"context_lines":[{"line_number":580,"context_line":"        # or updated exising, empty one."},{"line_number":581,"context_line":"        return self._find_ovn_lbs(lb_id, protocol\u003dprotocol)"},{"line_number":582,"context_line":""},{"line_number":583,"context_line":"    def _find_ovn_lb_with_pool_key(self, pool_key, loadbalancers\u003d[]):"},{"line_number":584,"context_line":"        if not loadbalancers:"},{"line_number":585,"context_line":"            loadbalancers \u003d self.ovn_nbdb_api.db_list_rows("},{"line_number":586,"context_line":"                \u0027Load_Balancer\u0027).execute(check_error\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9f07dd4e_20908229","line":583,"in_reply_to":"fa4f752e_3bc8b60e","updated":"2024-08-29 09:53:50.000000000","message":"I saw after writting the comment, thx!","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"79a14f0e93092726ba650a0eac271fb09e253cf6","unresolved":true,"context_lines":[{"line_number":806,"context_line":""},{"line_number":807,"context_line":"    def _add_lb_to_lr_association(self, ovn_lb, ovn_lr, lr_rf, is_sync\u003dFalse):"},{"line_number":808,"context_line":"        commands \u003d []"},{"line_number":809,"context_line":"        if not is_sync or not (str(ovn_lb.uuid) in ["},{"line_number":810,"context_line":"                str(lr_lb.uuid) for lr_lb in ovn_lr.load_balancer]):"},{"line_number":811,"context_line":"            commands.append("},{"line_number":812,"context_line":"                self.ovn_nbdb_api.lr_lb_add(ovn_lr.uuid, ovn_lb.uuid,"}],"source_content_type":"text/x-python","patch_set":12,"id":"11847672_f84f0fa8","line":809,"updated":"2024-08-28 16:27:09.000000000","message":"\u0027or not (str(ovn_lb.uuid) in [str(lr_lb.uuid) for lr_lb in ovn_lr.load_balancer])\u0027: This is a improvement/refactoring that make difficult follow up the code relative to the ovn-db-sync-tool. Those improvements/refactoring will be bettter to see in a separate patch to dont mix both things, just to have a better trazability in case of a hypotetic revert.","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"c8327ecdd69c9301ee8f0dd6eda26ff9f52cb89c","unresolved":true,"context_lines":[{"line_number":806,"context_line":""},{"line_number":807,"context_line":"    def _add_lb_to_lr_association(self, ovn_lb, ovn_lr, lr_rf, is_sync\u003dFalse):"},{"line_number":808,"context_line":"        commands \u003d []"},{"line_number":809,"context_line":"        if not is_sync or not (str(ovn_lb.uuid) in ["},{"line_number":810,"context_line":"                str(lr_lb.uuid) for lr_lb in ovn_lr.load_balancer]):"},{"line_number":811,"context_line":"            commands.append("},{"line_number":812,"context_line":"                self.ovn_nbdb_api.lr_lb_add(ovn_lr.uuid, ovn_lb.uuid,"}],"source_content_type":"text/x-python","patch_set":12,"id":"36ee3910_71becbf7","line":809,"in_reply_to":"11847672_f84f0fa8","updated":"2024-08-29 05:37:13.000000000","message":"actually these conditions are all under `is_sync` status.\nLet me clean this if condition to make it clear","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"09db1e01434e403e5dc741b398330909644a1491","unresolved":false,"context_lines":[{"line_number":806,"context_line":""},{"line_number":807,"context_line":"    def _add_lb_to_lr_association(self, ovn_lb, ovn_lr, lr_rf, is_sync\u003dFalse):"},{"line_number":808,"context_line":"        commands \u003d []"},{"line_number":809,"context_line":"        if not is_sync or not (str(ovn_lb.uuid) in ["},{"line_number":810,"context_line":"                str(lr_lb.uuid) for lr_lb in ovn_lr.load_balancer]):"},{"line_number":811,"context_line":"            commands.append("},{"line_number":812,"context_line":"                self.ovn_nbdb_api.lr_lb_add(ovn_lr.uuid, ovn_lb.uuid,"}],"source_content_type":"text/x-python","patch_set":12,"id":"e30b73ab_3130d25f","line":809,"in_reply_to":"36ee3910_71becbf7","updated":"2024-08-29 09:53:50.000000000","message":"Acknowledged","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"79a14f0e93092726ba650a0eac271fb09e253cf6","unresolved":true,"context_lines":[{"line_number":1102,"context_line":"        else:"},{"line_number":1103,"context_line":"            return str(listener_protocol).lower() in ovn_lb.protocol"},{"line_number":1104,"context_line":""},{"line_number":1105,"context_line":"    def check_lb_protocol(self, lb_id, listener_protocol):"},{"line_number":1106,"context_line":"        ovn_lb \u003d self._find_ovn_lbs(lb_id, protocol\u003dlistener_protocol)"},{"line_number":1107,"context_line":"        return self._check_lb_protocol(ovn_lb, listener_protocol)"},{"line_number":1108,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"db1a03c7_8b7eba8c","line":1105,"updated":"2024-08-28 16:27:09.000000000","message":"This refactor looks not related to the patch. This will be bettter to see in a separate patch to dont mix both things, just to have a better trazability in case of a hypotetic revert.","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"c8327ecdd69c9301ee8f0dd6eda26ff9f52cb89c","unresolved":false,"context_lines":[{"line_number":1102,"context_line":"        else:"},{"line_number":1103,"context_line":"            return str(listener_protocol).lower() in ovn_lb.protocol"},{"line_number":1104,"context_line":""},{"line_number":1105,"context_line":"    def check_lb_protocol(self, lb_id, listener_protocol):"},{"line_number":1106,"context_line":"        ovn_lb \u003d self._find_ovn_lbs(lb_id, protocol\u003dlistener_protocol)"},{"line_number":1107,"context_line":"        return self._check_lb_protocol(ovn_lb, listener_protocol)"},{"line_number":1108,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"88b9c552_313bd987","line":1105,"in_reply_to":"db1a03c7_8b7eba8c","updated":"2024-08-29 05:37:13.000000000","message":"yeah, I think we can restore this part.","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"79a14f0e93092726ba650a0eac271fb09e253cf6","unresolved":true,"context_lines":[{"line_number":1766,"context_line":"                                             (\u0027external_ids\u0027, listener_info)))"},{"line_number":1767,"context_line":"            if not self._is_listener_in_lb(ovn_lb):"},{"line_number":1768,"context_line":"                if not is_sync or ("},{"line_number":1769,"context_line":"                    ovn_lb.protocol[0].lower() !\u003d str("},{"line_number":1770,"context_line":"                        listener[constants.PROTOCOL]).lower()"},{"line_number":1771,"context_line":"                ):"},{"line_number":1772,"context_line":"                    commands.append("}],"source_content_type":"text/x-python","patch_set":12,"id":"b2ce3f13_14aa5a60","line":1769,"updated":"2024-08-28 16:27:09.000000000","message":"\u0027or (ovn_lb.protocol[0].lower() !\u003d str(listener[constants.PROTOCOL]).lower()): This is a improvement/refactoring that make difficult follow up the code relative to the ovn-db-sync-tool. Those improvements/refactoring will be bettter to see in a separate patch to dont mix both things, just to have a better trazability in case of a hypotetic revert.","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"c8327ecdd69c9301ee8f0dd6eda26ff9f52cb89c","unresolved":true,"context_lines":[{"line_number":1766,"context_line":"                                             (\u0027external_ids\u0027, listener_info)))"},{"line_number":1767,"context_line":"            if not self._is_listener_in_lb(ovn_lb):"},{"line_number":1768,"context_line":"                if not is_sync or ("},{"line_number":1769,"context_line":"                    ovn_lb.protocol[0].lower() !\u003d str("},{"line_number":1770,"context_line":"                        listener[constants.PROTOCOL]).lower()"},{"line_number":1771,"context_line":"                ):"},{"line_number":1772,"context_line":"                    commands.append("}],"source_content_type":"text/x-python","patch_set":12,"id":"e27bc1d8_1d4162b9","line":1769,"in_reply_to":"b2ce3f13_14aa5a60","updated":"2024-08-29 05:37:13.000000000","message":"These are related to the patch, I refactored a bit to make it more clear","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"09db1e01434e403e5dc741b398330909644a1491","unresolved":false,"context_lines":[{"line_number":1766,"context_line":"                                             (\u0027external_ids\u0027, listener_info)))"},{"line_number":1767,"context_line":"            if not self._is_listener_in_lb(ovn_lb):"},{"line_number":1768,"context_line":"                if not is_sync or ("},{"line_number":1769,"context_line":"                    ovn_lb.protocol[0].lower() !\u003d str("},{"line_number":1770,"context_line":"                        listener[constants.PROTOCOL]).lower()"},{"line_number":1771,"context_line":"                ):"},{"line_number":1772,"context_line":"                    commands.append("}],"source_content_type":"text/x-python","patch_set":12,"id":"dbd7d673_0337e67c","line":1769,"in_reply_to":"e27bc1d8_1d4162b9","updated":"2024-08-29 09:53:50.000000000","message":"Acknowledged","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"79a14f0e93092726ba650a0eac271fb09e253cf6","unresolved":true,"context_lines":[{"line_number":3134,"context_line":"                    vip \u003d f\u0027[{vip}]\u0027"},{"line_number":3135,"context_line":"                vip \u003d vip + \u0027:\u0027 + str(vip_port)"},{"line_number":3136,"context_line":"        commands \u003d []"},{"line_number":3137,"context_line":"        if not is_sync or (vip !\u003d lbhc.vip):"},{"line_number":3138,"context_line":"            commands.append("},{"line_number":3139,"context_line":"                self.ovn_nbdb_api.db_set("},{"line_number":3140,"context_line":"                    \u0027Load_Balancer_Health_Check\u0027, lbhc.uuid,"}],"source_content_type":"text/x-python","patch_set":12,"id":"f9fa9ea1_e5010986","line":3137,"updated":"2024-08-28 16:27:09.000000000","message":"\u0027or (vip !\u003d lbhc.vip)\u0027: This is a improvement/refactoring that make difficult follow up the code relative to the ovn-db-sync-tool. Those improvements/refactoring will be bettter to see in a separate patch to dont mix both things, just to have a better trazability in case of a hypotetic revert.","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"09db1e01434e403e5dc741b398330909644a1491","unresolved":false,"context_lines":[{"line_number":3134,"context_line":"                    vip \u003d f\u0027[{vip}]\u0027"},{"line_number":3135,"context_line":"                vip \u003d vip + \u0027:\u0027 + str(vip_port)"},{"line_number":3136,"context_line":"        commands \u003d []"},{"line_number":3137,"context_line":"        if not is_sync or (vip !\u003d lbhc.vip):"},{"line_number":3138,"context_line":"            commands.append("},{"line_number":3139,"context_line":"                self.ovn_nbdb_api.db_set("},{"line_number":3140,"context_line":"                    \u0027Load_Balancer_Health_Check\u0027, lbhc.uuid,"}],"source_content_type":"text/x-python","patch_set":12,"id":"c5a16e26_cdfdd1f2","line":3137,"in_reply_to":"c2f65570_d4970f0f","updated":"2024-08-29 09:53:50.000000000","message":"Acknowledged","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"c8327ecdd69c9301ee8f0dd6eda26ff9f52cb89c","unresolved":true,"context_lines":[{"line_number":3134,"context_line":"                    vip \u003d f\u0027[{vip}]\u0027"},{"line_number":3135,"context_line":"                vip \u003d vip + \u0027:\u0027 + str(vip_port)"},{"line_number":3136,"context_line":"        commands \u003d []"},{"line_number":3137,"context_line":"        if not is_sync or (vip !\u003d lbhc.vip):"},{"line_number":3138,"context_line":"            commands.append("},{"line_number":3139,"context_line":"                self.ovn_nbdb_api.db_set("},{"line_number":3140,"context_line":"                    \u0027Load_Balancer_Health_Check\u0027, lbhc.uuid,"}],"source_content_type":"text/x-python","patch_set":12,"id":"c2f65570_d4970f0f","line":3137,"in_reply_to":"f9fa9ea1_e5010986","updated":"2024-08-29 05:37:13.000000000","message":"This is related to the patch, I added is_sync with comments hope that help to clear the case","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"a7f0387cb70bf1d87524fdf588b22f46f1284d5e","unresolved":true,"context_lines":[{"line_number":665,"context_line":"                                \u0027not found in OVN NBDB. Exiting.\u0027,"},{"line_number":666,"context_line":"                                {\u0027ls\u0027: ls_name, \u0027lb\u0027: ovn_lb.name})"},{"line_number":667,"context_line":"                    return commands"},{"line_number":668,"context_line":"            if is_sync:"},{"line_number":669,"context_line":"                for ls_lb in ovn_ls.load_balancer:"},{"line_number":670,"context_line":"                    if str(ls_lb.uuid) \u003d\u003d str(ovn_lb.uuid):"},{"line_number":671,"context_line":"                        # lb already in ls, skip assocate for sync steps"}],"source_content_type":"text/x-python","patch_set":29,"id":"7933ef56_4031dbff","line":668,"range":{"start_line":668,"start_character":0,"end_line":668,"end_character":23},"updated":"2024-10-17 11:21:59.000000000","message":"perhaps add a comment about why skipping it for the sync","commit_id":"74abe5c765262cbd9387474ba1c291c5e88cbc07"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"411f419657ddd0fd1e4a3d6718a6b0f83eb8b921","unresolved":false,"context_lines":[{"line_number":665,"context_line":"                                \u0027not found in OVN NBDB. Exiting.\u0027,"},{"line_number":666,"context_line":"                                {\u0027ls\u0027: ls_name, \u0027lb\u0027: ovn_lb.name})"},{"line_number":667,"context_line":"                    return commands"},{"line_number":668,"context_line":"            if is_sync:"},{"line_number":669,"context_line":"                for ls_lb in ovn_ls.load_balancer:"},{"line_number":670,"context_line":"                    if str(ls_lb.uuid) \u003d\u003d str(ovn_lb.uuid):"},{"line_number":671,"context_line":"                        # lb already in ls, skip assocate for sync steps"}],"source_content_type":"text/x-python","patch_set":29,"id":"35ca6b23_b6dcb1a1","line":668,"range":{"start_line":668,"start_character":0,"end_line":668,"end_character":23},"in_reply_to":"7933ef56_4031dbff","updated":"2024-10-29 17:31:58.000000000","message":"Done","commit_id":"74abe5c765262cbd9387474ba1c291c5e88cbc07"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"a7f0387cb70bf1d87524fdf588b22f46f1284d5e","unresolved":true,"context_lines":[{"line_number":681,"context_line":"        else:"},{"line_number":682,"context_line":"            ls_refs \u003d {}"},{"line_number":683,"context_line":""},{"line_number":684,"context_line":"        if not skip_ls_lb_actions:"},{"line_number":685,"context_line":"            if associate and ls_name:"},{"line_number":686,"context_line":"                if ls_name in ls_refs:"},{"line_number":687,"context_line":"                    ref_ct \u003d ls_refs[ls_name]"}],"source_content_type":"text/x-python","patch_set":29,"id":"cb3fc500_f6097ed4","line":684,"range":{"start_line":684,"start_character":0,"end_line":684,"end_character":34},"updated":"2024-10-17 11:21:59.000000000","message":"perhaps simple in the other case, \"if skip_ls_lb_actions\", and this block in the else","commit_id":"74abe5c765262cbd9387474ba1c291c5e88cbc07"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"411f419657ddd0fd1e4a3d6718a6b0f83eb8b921","unresolved":false,"context_lines":[{"line_number":681,"context_line":"        else:"},{"line_number":682,"context_line":"            ls_refs \u003d {}"},{"line_number":683,"context_line":""},{"line_number":684,"context_line":"        if not skip_ls_lb_actions:"},{"line_number":685,"context_line":"            if associate and ls_name:"},{"line_number":686,"context_line":"                if ls_name in ls_refs:"},{"line_number":687,"context_line":"                    ref_ct \u003d ls_refs[ls_name]"}],"source_content_type":"text/x-python","patch_set":29,"id":"ae8ccae3_4b6da69d","line":684,"range":{"start_line":684,"start_character":0,"end_line":684,"end_character":34},"in_reply_to":"cb3fc500_f6097ed4","updated":"2024-10-29 17:31:58.000000000","message":"Done","commit_id":"74abe5c765262cbd9387474ba1c291c5e88cbc07"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"a7f0387cb70bf1d87524fdf588b22f46f1284d5e","unresolved":true,"context_lines":[{"line_number":1104,"context_line":"                        break"},{"line_number":1105,"context_line":"        return port, subnet"},{"line_number":1106,"context_line":""},{"line_number":1107,"context_line":"    def lb_sync(self, loadbalancer, ovn_lb):"},{"line_number":1108,"context_line":"        commands \u003d []"},{"line_number":1109,"context_line":"        port \u003d None"},{"line_number":1110,"context_line":"        subnet \u003d None"}],"source_content_type":"text/x-python","patch_set":29,"id":"3b6987aa_958e433f","line":1107,"range":{"start_line":1107,"start_character":2,"end_line":1107,"end_character":44},"updated":"2024-10-17 11:21:59.000000000","message":"I know that this is also missing in the other functions, but perhaps worth to add a docstring explaining the steps here","commit_id":"74abe5c765262cbd9387474ba1c291c5e88cbc07"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"5bca935c3f746fd5dc9b9cd76a3f75ada7874062","unresolved":false,"context_lines":[{"line_number":1104,"context_line":"                        break"},{"line_number":1105,"context_line":"        return port, subnet"},{"line_number":1106,"context_line":""},{"line_number":1107,"context_line":"    def lb_sync(self, loadbalancer, ovn_lb):"},{"line_number":1108,"context_line":"        commands \u003d []"},{"line_number":1109,"context_line":"        port \u003d None"},{"line_number":1110,"context_line":"        subnet \u003d None"}],"source_content_type":"text/x-python","patch_set":29,"id":"900b8ee0_56d9eff6","line":1107,"range":{"start_line":1107,"start_character":2,"end_line":1107,"end_character":44},"in_reply_to":"3b6987aa_958e433f","updated":"2024-10-29 17:32:52.000000000","message":"done! also to the xxx_sync added in further patch on the chain.","commit_id":"74abe5c765262cbd9387474ba1c291c5e88cbc07"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"cf87e708c6cf6898731136cd310e5a2568818e23","unresolved":true,"context_lines":[{"line_number":1141,"context_line":"                loadbalancer.get(constants.VIP_NETWORK_ID, None),"},{"line_number":1142,"context_line":"                loadbalancer.get(constants.VIP_ADDRESS, None))"},{"line_number":1143,"context_line":"        except Exception:"},{"line_number":1144,"context_line":"            LOG.warn(\u0027Cannot get info from neutron client for loadbalancer \u0027"},{"line_number":1145,"context_line":"                     \u0027sync.\u0027)"},{"line_number":1146,"context_line":"            return False"},{"line_number":1147,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"8628f5f8_46fb0ea3","line":1144,"range":{"start_line":1144,"start_character":51,"end_line":1144,"end_character":57},"updated":"2024-11-13 09:37:41.000000000","message":"nitty nit: I think that \"client\" is not needed here. You don\u0027t try to get info from client actually but from neutron using client 😊","commit_id":"f01257b0cdc52fb0b6a95c7ea69e59d6b5b30d74"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"081fd3fbb60d4b1612527c77b962ef30974b2947","unresolved":false,"context_lines":[{"line_number":1141,"context_line":"                loadbalancer.get(constants.VIP_NETWORK_ID, None),"},{"line_number":1142,"context_line":"                loadbalancer.get(constants.VIP_ADDRESS, None))"},{"line_number":1143,"context_line":"        except Exception:"},{"line_number":1144,"context_line":"            LOG.warn(\u0027Cannot get info from neutron client for loadbalancer \u0027"},{"line_number":1145,"context_line":"                     \u0027sync.\u0027)"},{"line_number":1146,"context_line":"            return False"},{"line_number":1147,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"c37657d2_cadded3f","line":1144,"range":{"start_line":1144,"start_character":51,"end_line":1144,"end_character":57},"in_reply_to":"8628f5f8_46fb0ea3","updated":"2024-11-19 11:53:15.000000000","message":"Done","commit_id":"f01257b0cdc52fb0b6a95c7ea69e59d6b5b30d74"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"cf87e708c6cf6898731136cd310e5a2568818e23","unresolved":true,"context_lines":[{"line_number":1206,"context_line":"                    (\u0027selection_fields\u0027, selection_fields))"},{"line_number":1207,"context_line":"            )"},{"line_number":1208,"context_line":"        try:"},{"line_number":1209,"context_line":"            self._execute_commands(commands)"},{"line_number":1210,"context_line":"            ovn_lb \u003d self._find_ovn_lbs_with_retry("},{"line_number":1211,"context_line":"                loadbalancer[constants.ID],"},{"line_number":1212,"context_line":"                protocol\u003dprotocol)"}],"source_content_type":"text/x-python","patch_set":30,"id":"054be549_6243415f","line":1209,"updated":"2024-11-13 09:37:41.000000000","message":"shouldn\u0027t you first check if \"commands\" actually is not empty list here?","commit_id":"f01257b0cdc52fb0b6a95c7ea69e59d6b5b30d74"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"081fd3fbb60d4b1612527c77b962ef30974b2947","unresolved":false,"context_lines":[{"line_number":1206,"context_line":"                    (\u0027selection_fields\u0027, selection_fields))"},{"line_number":1207,"context_line":"            )"},{"line_number":1208,"context_line":"        try:"},{"line_number":1209,"context_line":"            self._execute_commands(commands)"},{"line_number":1210,"context_line":"            ovn_lb \u003d self._find_ovn_lbs_with_retry("},{"line_number":1211,"context_line":"                loadbalancer[constants.ID],"},{"line_number":1212,"context_line":"                protocol\u003dprotocol)"}],"source_content_type":"text/x-python","patch_set":30,"id":"35e6cd49_1eb9132b","line":1209,"in_reply_to":"054be549_6243415f","updated":"2024-11-19 11:53:15.000000000","message":"Done in _execute_commands to ensure for any other further call not checking empty list","commit_id":"f01257b0cdc52fb0b6a95c7ea69e59d6b5b30d74"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"cf87e708c6cf6898731136cd310e5a2568818e23","unresolved":true,"context_lines":[{"line_number":1262,"context_line":"                    self._update_lb_to_ls_association("},{"line_number":1263,"context_line":"                        ovn_lb, network_id\u003dutils.ovn_uuid(ls),"},{"line_number":1264,"context_line":"                        associate\u003dTrue, update_ls_ref\u003dTrue, is_sync\u003dTrue)"},{"line_number":1265,"context_line":"        except Exception:"},{"line_number":1266,"context_line":"            LOG.exception(\"Cannot finish sync loadbalancer \""},{"line_number":1267,"context_line":"                          f\"{loadbalancer[constants.ID]} to OVN.\")"},{"line_number":1268,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"f42e0eed_f476390c","line":1265,"updated":"2024-11-13 09:37:41.000000000","message":"this is just a question: isn\u0027t that try..except.. block to big here? If anything will happen we may not know really in which step it failed and what was the error actually.","commit_id":"f01257b0cdc52fb0b6a95c7ea69e59d6b5b30d74"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"f13535c07ea201341f807e89bb77ba88c06bcabc","unresolved":false,"context_lines":[{"line_number":1262,"context_line":"                    self._update_lb_to_ls_association("},{"line_number":1263,"context_line":"                        ovn_lb, network_id\u003dutils.ovn_uuid(ls),"},{"line_number":1264,"context_line":"                        associate\u003dTrue, update_ls_ref\u003dTrue, is_sync\u003dTrue)"},{"line_number":1265,"context_line":"        except Exception:"},{"line_number":1266,"context_line":"            LOG.exception(\"Cannot finish sync loadbalancer \""},{"line_number":1267,"context_line":"                          f\"{loadbalancer[constants.ID]} to OVN.\")"},{"line_number":1268,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"93434ec9_7886b6a3","line":1265,"in_reply_to":"2c623a11_445fc953","updated":"2025-02-07 10:19:38.000000000","message":"Done","commit_id":"f01257b0cdc52fb0b6a95c7ea69e59d6b5b30d74"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cd1591ebd10ae04421588f62993d73e0cc77059c","unresolved":true,"context_lines":[{"line_number":1262,"context_line":"                    self._update_lb_to_ls_association("},{"line_number":1263,"context_line":"                        ovn_lb, network_id\u003dutils.ovn_uuid(ls),"},{"line_number":1264,"context_line":"                        associate\u003dTrue, update_ls_ref\u003dTrue, is_sync\u003dTrue)"},{"line_number":1265,"context_line":"        except Exception:"},{"line_number":1266,"context_line":"            LOG.exception(\"Cannot finish sync loadbalancer \""},{"line_number":1267,"context_line":"                          f\"{loadbalancer[constants.ID]} to OVN.\")"},{"line_number":1268,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"7482465c_47cdeba1","line":1265,"in_reply_to":"6e466377_f39e8220","updated":"2025-01-22 16:50:43.000000000","message":"hehe, I complained about using broad Exception on L1144 already and now I got here :)","commit_id":"f01257b0cdc52fb0b6a95c7ea69e59d6b5b30d74"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"0702d2f76e28876777697435b344ab8d1ec638be","unresolved":true,"context_lines":[{"line_number":1262,"context_line":"                    self._update_lb_to_ls_association("},{"line_number":1263,"context_line":"                        ovn_lb, network_id\u003dutils.ovn_uuid(ls),"},{"line_number":1264,"context_line":"                        associate\u003dTrue, update_ls_ref\u003dTrue, is_sync\u003dTrue)"},{"line_number":1265,"context_line":"        except Exception:"},{"line_number":1266,"context_line":"            LOG.exception(\"Cannot finish sync loadbalancer \""},{"line_number":1267,"context_line":"                          f\"{loadbalancer[constants.ID]} to OVN.\")"},{"line_number":1268,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"2c623a11_445fc953","line":1265,"in_reply_to":"7482465c_47cdeba1","updated":"2025-01-27 18:52:50.000000000","message":"I tried to cover comment on L1144 and this from Slawek, let me know your feedback","commit_id":"f01257b0cdc52fb0b6a95c7ea69e59d6b5b30d74"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"081fd3fbb60d4b1612527c77b962ef30974b2947","unresolved":true,"context_lines":[{"line_number":1262,"context_line":"                    self._update_lb_to_ls_association("},{"line_number":1263,"context_line":"                        ovn_lb, network_id\u003dutils.ovn_uuid(ls),"},{"line_number":1264,"context_line":"                        associate\u003dTrue, update_ls_ref\u003dTrue, is_sync\u003dTrue)"},{"line_number":1265,"context_line":"        except Exception:"},{"line_number":1266,"context_line":"            LOG.exception(\"Cannot finish sync loadbalancer \""},{"line_number":1267,"context_line":"                          f\"{loadbalancer[constants.ID]} to OVN.\")"},{"line_number":1268,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"6e466377_f39e8220","line":1265,"in_reply_to":"f42e0eed_f476390c","updated":"2024-11-19 11:53:15.000000000","message":"Yeah, totally agree. This is coming from the lb_create method where a similar block exists. Maybe we can split in smaller try...except ones in a future patch where both method (lb_create/lb_sync) could be fixed at same time/way. wdyt?","commit_id":"f01257b0cdc52fb0b6a95c7ea69e59d6b5b30d74"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cd1591ebd10ae04421588f62993d73e0cc77059c","unresolved":true,"context_lines":[{"line_number":669,"context_line":"            # if is_sync and LB already in LS_LB, we don\u0027t need to call to"},{"line_number":670,"context_line":"            # ls_lb_add"},{"line_number":671,"context_line":"            if is_sync:"},{"line_number":672,"context_line":"                for ls_lb in ovn_ls.load_balancer:"},{"line_number":673,"context_line":"                    if str(ls_lb.uuid) \u003d\u003d str(ovn_lb.uuid):"},{"line_number":674,"context_line":"                        # lb already in ls, skip assocate for sync steps"},{"line_number":675,"context_line":"                        skip_ls_lb_actions \u003d True"}],"source_content_type":"text/x-python","patch_set":36,"id":"e651c9f2_b94d138c","line":672,"range":{"start_line":672,"start_character":29,"end_line":672,"end_character":49},"updated":"2025-01-22 16:50:43.000000000","message":"This raises an `AttributeError` if the LS is not found in the OVN DB and `associate` is set to `False` and `is_sync` is set to `True`","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"0702d2f76e28876777697435b344ab8d1ec638be","unresolved":false,"context_lines":[{"line_number":669,"context_line":"            # if is_sync and LB already in LS_LB, we don\u0027t need to call to"},{"line_number":670,"context_line":"            # ls_lb_add"},{"line_number":671,"context_line":"            if is_sync:"},{"line_number":672,"context_line":"                for ls_lb in ovn_ls.load_balancer:"},{"line_number":673,"context_line":"                    if str(ls_lb.uuid) \u003d\u003d str(ovn_lb.uuid):"},{"line_number":674,"context_line":"                        # lb already in ls, skip assocate for sync steps"},{"line_number":675,"context_line":"                        skip_ls_lb_actions \u003d True"}],"source_content_type":"text/x-python","patch_set":36,"id":"18cb0f01_7f107579","line":672,"range":{"start_line":672,"start_character":29,"end_line":672,"end_character":49},"in_reply_to":"71db69e7_1d08feb7","updated":"2025-01-27 18:52:50.000000000","message":"Done","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"206343420e61da12291d91bd242fc5fb7e85a9b2","unresolved":true,"context_lines":[{"line_number":669,"context_line":"            # if is_sync and LB already in LS_LB, we don\u0027t need to call to"},{"line_number":670,"context_line":"            # ls_lb_add"},{"line_number":671,"context_line":"            if is_sync:"},{"line_number":672,"context_line":"                for ls_lb in ovn_ls.load_balancer:"},{"line_number":673,"context_line":"                    if str(ls_lb.uuid) \u003d\u003d str(ovn_lb.uuid):"},{"line_number":674,"context_line":"                        # lb already in ls, skip assocate for sync steps"},{"line_number":675,"context_line":"                        skip_ls_lb_actions \u003d True"}],"source_content_type":"text/x-python","patch_set":36,"id":"71db69e7_1d08feb7","line":672,"range":{"start_line":672,"start_character":29,"end_line":672,"end_character":49},"in_reply_to":"c0e7251a_6c0098b1","updated":"2025-01-23 14:16:56.000000000","message":"Well, we can always cover the case indicated by Jakub by validating that ovn_ls is not None and save ourselves for a future change that does not fall into this specific case. Yeah, the associate parameter in this function has connotations of add or remove yes. Regarding the change of line 678, as that line has not been modified in the patch better not to put more changes hehe indeed .get(ovn_const.LB_EXT_EXT_IDS_LS_REFS_KEY) will return None in case the attribute does not exist, here we are safe.","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"50b1170ec6491297a5565dcdb5c7197ea2576fcb","unresolved":true,"context_lines":[{"line_number":669,"context_line":"            # if is_sync and LB already in LS_LB, we don\u0027t need to call to"},{"line_number":670,"context_line":"            # ls_lb_add"},{"line_number":671,"context_line":"            if is_sync:"},{"line_number":672,"context_line":"                for ls_lb in ovn_ls.load_balancer:"},{"line_number":673,"context_line":"                    if str(ls_lb.uuid) \u003d\u003d str(ovn_lb.uuid):"},{"line_number":674,"context_line":"                        # lb already in ls, skip assocate for sync steps"},{"line_number":675,"context_line":"                        skip_ls_lb_actions \u003d True"}],"source_content_type":"text/x-python","patch_set":36,"id":"c0e7251a_6c0098b1","line":672,"range":{"start_line":672,"start_character":29,"end_line":672,"end_character":49},"in_reply_to":"e651c9f2_b94d138c","updated":"2025-01-23 06:29:32.000000000","message":"is_sync currently using ONLY with cases associate set to True, so I think we\u0027re safe?\nIIUC, associate to false only use in delete case (lb_delete_lrp_assoc) which we don\u0027t need to do sync around there\nif we gonna fix this here, we might as well as to fix `ls_refs \u003d ovn_lb.external_ids.get` in line 678 and also","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cd1591ebd10ae04421588f62993d73e0cc77059c","unresolved":true,"context_lines":[{"line_number":690,"context_line":"        else:"},{"line_number":691,"context_line":"            if associate and ls_name:"},{"line_number":692,"context_line":"                if ls_name in ls_refs:"},{"line_number":693,"context_line":"                    ref_ct \u003d ls_refs[ls_name]"},{"line_number":694,"context_line":"                    ls_refs[ls_name] \u003d ref_ct + 1"},{"line_number":695,"context_line":"                else:"},{"line_number":696,"context_line":"                    ls_refs[ls_name] \u003d 1"},{"line_number":697,"context_line":"                    # NOTE(froyo): To cover the initial lb to ls association,"}],"source_content_type":"text/x-python","patch_set":36,"id":"0b3e37a7_99be84ad","line":694,"range":{"start_line":693,"start_character":0,"end_line":694,"end_character":49},"updated":"2025-01-22 16:50:43.000000000","message":"nit: I know the code was there before the patch, just `ls_refs[ls_name] +\u003d 1`","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"50b1170ec6491297a5565dcdb5c7197ea2576fcb","unresolved":false,"context_lines":[{"line_number":690,"context_line":"        else:"},{"line_number":691,"context_line":"            if associate and ls_name:"},{"line_number":692,"context_line":"                if ls_name in ls_refs:"},{"line_number":693,"context_line":"                    ref_ct \u003d ls_refs[ls_name]"},{"line_number":694,"context_line":"                    ls_refs[ls_name] \u003d ref_ct + 1"},{"line_number":695,"context_line":"                else:"},{"line_number":696,"context_line":"                    ls_refs[ls_name] \u003d 1"},{"line_number":697,"context_line":"                    # NOTE(froyo): To cover the initial lb to ls association,"}],"source_content_type":"text/x-python","patch_set":36,"id":"6f47fb37_c260868e","line":694,"range":{"start_line":693,"start_character":0,"end_line":694,"end_character":49},"in_reply_to":"0b3e37a7_99be84ad","updated":"2025-01-23 06:29:32.000000000","message":"Done","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cd1591ebd10ae04421588f62993d73e0cc77059c","unresolved":true,"context_lines":[{"line_number":733,"context_line":"                        ovn_ls_refs \u003d jsonutils.loads(ovn_ls_refs)"},{"line_number":734,"context_line":"                    except ValueError:"},{"line_number":735,"context_line":"                        ovn_ls_refs \u003d {}"},{"line_number":736,"context_line":"                check_ls_refs \u003d all("},{"line_number":737,"context_line":"                    k in ovn_ls_refs for k in ls_refs"},{"line_number":738,"context_line":"                ) and (len(ovn_ls_refs) \u003d\u003d len(ls_refs))"},{"line_number":739,"context_line":"            if not is_sync or ("},{"line_number":740,"context_line":"                is_sync and not check_ls_refs"},{"line_number":741,"context_line":"            ):"}],"source_content_type":"text/x-python","patch_set":36,"id":"87b5952b_38c093d3","line":738,"range":{"start_line":736,"start_character":32,"end_line":738,"end_character":55},"updated":"2025-01-22 16:50:43.000000000","message":"`ovn_ls_refs.keys() \u003d\u003d ls_refs.keys()`","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"50b1170ec6491297a5565dcdb5c7197ea2576fcb","unresolved":false,"context_lines":[{"line_number":733,"context_line":"                        ovn_ls_refs \u003d jsonutils.loads(ovn_ls_refs)"},{"line_number":734,"context_line":"                    except ValueError:"},{"line_number":735,"context_line":"                        ovn_ls_refs \u003d {}"},{"line_number":736,"context_line":"                check_ls_refs \u003d all("},{"line_number":737,"context_line":"                    k in ovn_ls_refs for k in ls_refs"},{"line_number":738,"context_line":"                ) and (len(ovn_ls_refs) \u003d\u003d len(ls_refs))"},{"line_number":739,"context_line":"            if not is_sync or ("},{"line_number":740,"context_line":"                is_sync and not check_ls_refs"},{"line_number":741,"context_line":"            ):"}],"source_content_type":"text/x-python","patch_set":36,"id":"2a8f1fdb_9f9c0a39","line":738,"range":{"start_line":736,"start_character":32,"end_line":738,"end_character":55},"in_reply_to":"87b5952b_38c093d3","updated":"2025-01-23 06:29:32.000000000","message":"Done","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cd1591ebd10ae04421588f62993d73e0cc77059c","unresolved":true,"context_lines":[{"line_number":736,"context_line":"                check_ls_refs \u003d all("},{"line_number":737,"context_line":"                    k in ovn_ls_refs for k in ls_refs"},{"line_number":738,"context_line":"                ) and (len(ovn_ls_refs) \u003d\u003d len(ls_refs))"},{"line_number":739,"context_line":"            if not is_sync or ("},{"line_number":740,"context_line":"                is_sync and not check_ls_refs"},{"line_number":741,"context_line":"            ):"},{"line_number":742,"context_line":"                ls_refs_dict \u003d {"},{"line_number":743,"context_line":"                    ovn_const.LB_EXT_IDS_LS_REFS_KEY: jsonutils.dumps("}],"source_content_type":"text/x-python","patch_set":36,"id":"a24c31d1_5dc21b51","line":740,"range":{"start_line":739,"start_character":0,"end_line":740,"end_character":45},"updated":"2025-01-22 16:50:43.000000000","message":"the `is_sync` variable has absolutely no influence on the result of this logical expression :) If it is `False` then the whole expression is `True`. If it\u0027s `True` then you check again if it\u0027s `True` and then depend on `check_ls_refs`. If it\u0027s `False` then `check_ls_refs` is `False` too.","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"50b1170ec6491297a5565dcdb5c7197ea2576fcb","unresolved":false,"context_lines":[{"line_number":736,"context_line":"                check_ls_refs \u003d all("},{"line_number":737,"context_line":"                    k in ovn_ls_refs for k in ls_refs"},{"line_number":738,"context_line":"                ) and (len(ovn_ls_refs) \u003d\u003d len(ls_refs))"},{"line_number":739,"context_line":"            if not is_sync or ("},{"line_number":740,"context_line":"                is_sync and not check_ls_refs"},{"line_number":741,"context_line":"            ):"},{"line_number":742,"context_line":"                ls_refs_dict \u003d {"},{"line_number":743,"context_line":"                    ovn_const.LB_EXT_IDS_LS_REFS_KEY: jsonutils.dumps("}],"source_content_type":"text/x-python","patch_set":36,"id":"21a4f6f6_7cd80765","line":740,"range":{"start_line":739,"start_character":0,"end_line":740,"end_character":45},"in_reply_to":"a24c31d1_5dc21b51","updated":"2025-01-23 06:29:32.000000000","message":"Done","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cd1591ebd10ae04421588f62993d73e0cc77059c","unresolved":true,"context_lines":[{"line_number":1052,"context_line":""},{"line_number":1053,"context_line":"    def _refresh_lb_vips(self, ovn_lb, lb_external_ids, is_sync\u003dFalse):"},{"line_number":1054,"context_line":"        vip_ips \u003d self._frame_vip_ips(ovn_lb, lb_external_ids)"},{"line_number":1055,"context_line":"        if is_sync and all("},{"line_number":1056,"context_line":"            ovn_lb.vips.get(k) \u003d\u003d v for k, v in vip_ips.items()"},{"line_number":1057,"context_line":"        ) and len(vip_ips) \u003d\u003d len(ovn_lb.vips):"},{"line_number":1058,"context_line":"            return []"},{"line_number":1059,"context_line":"        return [self.ovn_nbdb_api.db_clear(\u0027Load_Balancer\u0027, ovn_lb.uuid,"},{"line_number":1060,"context_line":"                                           \u0027vips\u0027),"}],"source_content_type":"text/x-python","patch_set":36,"id":"2931040d_72089aa7","line":1057,"range":{"start_line":1055,"start_character":23,"end_line":1057,"end_character":46},"updated":"2025-01-22 16:50:43.000000000","message":"Can\u0027t you just do `ovn_lb.vips \u003d\u003d vip_ips` ?","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"50b1170ec6491297a5565dcdb5c7197ea2576fcb","unresolved":false,"context_lines":[{"line_number":1052,"context_line":""},{"line_number":1053,"context_line":"    def _refresh_lb_vips(self, ovn_lb, lb_external_ids, is_sync\u003dFalse):"},{"line_number":1054,"context_line":"        vip_ips \u003d self._frame_vip_ips(ovn_lb, lb_external_ids)"},{"line_number":1055,"context_line":"        if is_sync and all("},{"line_number":1056,"context_line":"            ovn_lb.vips.get(k) \u003d\u003d v for k, v in vip_ips.items()"},{"line_number":1057,"context_line":"        ) and len(vip_ips) \u003d\u003d len(ovn_lb.vips):"},{"line_number":1058,"context_line":"            return []"},{"line_number":1059,"context_line":"        return [self.ovn_nbdb_api.db_clear(\u0027Load_Balancer\u0027, ovn_lb.uuid,"},{"line_number":1060,"context_line":"                                           \u0027vips\u0027),"}],"source_content_type":"text/x-python","patch_set":36,"id":"381a37d0_27c03208","line":1057,"range":{"start_line":1055,"start_character":23,"end_line":1057,"end_character":46},"in_reply_to":"2931040d_72089aa7","updated":"2025-01-23 06:29:32.000000000","message":"I think the issue is to prevent any nested structure in vips, or you guys think I worry to much here?","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"206343420e61da12291d91bd242fc5fb7e85a9b2","unresolved":false,"context_lines":[{"line_number":1052,"context_line":""},{"line_number":1053,"context_line":"    def _refresh_lb_vips(self, ovn_lb, lb_external_ids, is_sync\u003dFalse):"},{"line_number":1054,"context_line":"        vip_ips \u003d self._frame_vip_ips(ovn_lb, lb_external_ids)"},{"line_number":1055,"context_line":"        if is_sync and all("},{"line_number":1056,"context_line":"            ovn_lb.vips.get(k) \u003d\u003d v for k, v in vip_ips.items()"},{"line_number":1057,"context_line":"        ) and len(vip_ips) \u003d\u003d len(ovn_lb.vips):"},{"line_number":1058,"context_line":"            return []"},{"line_number":1059,"context_line":"        return [self.ovn_nbdb_api.db_clear(\u0027Load_Balancer\u0027, ovn_lb.uuid,"},{"line_number":1060,"context_line":"                                           \u0027vips\u0027),"}],"source_content_type":"text/x-python","patch_set":36,"id":"5e70690d_5450dac7","line":1057,"range":{"start_line":1055,"start_character":23,"end_line":1057,"end_character":46},"in_reply_to":"381a37d0_27c03208","updated":"2025-01-23 14:16:56.000000000","message":"Not possible to have nested structures in vips, so +1 to Jakub\u0027s comment","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cd1591ebd10ae04421588f62993d73e0cc77059c","unresolved":true,"context_lines":[{"line_number":1111,"context_line":"    def lb_sync(self, loadbalancer, ovn_lb):"},{"line_number":1112,"context_line":"        \"\"\"Sync LoadBalancer object with an OVN LoadBalancer"},{"line_number":1113,"context_line":""},{"line_number":1114,"context_line":"        The method performs the following steps:"},{"line_number":1115,"context_line":"        1. Retrieves the port and subnet of the VIP"},{"line_number":1116,"context_line":"        2. Builds `external_ids` based on the information from the LoadBalancer"},{"line_number":1117,"context_line":"        3. Compares the constructed `external_ids` with the OVN LoadBalancer\u0027s"}],"source_content_type":"text/x-python","patch_set":36,"id":"334d82e4_e575aa17","line":1114,"updated":"2025-01-22 16:50:43.000000000","message":"Would be good to consider implementing each of the described steps below as its own manageable method/function to break down the \u003e100 LOC method and increase its maintainability and readability.","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"8d01e374a864872be362227f59c2964e95828304","unresolved":false,"context_lines":[{"line_number":1111,"context_line":"    def lb_sync(self, loadbalancer, ovn_lb):"},{"line_number":1112,"context_line":"        \"\"\"Sync LoadBalancer object with an OVN LoadBalancer"},{"line_number":1113,"context_line":""},{"line_number":1114,"context_line":"        The method performs the following steps:"},{"line_number":1115,"context_line":"        1. Retrieves the port and subnet of the VIP"},{"line_number":1116,"context_line":"        2. Builds `external_ids` based on the information from the LoadBalancer"},{"line_number":1117,"context_line":"        3. Compares the constructed `external_ids` with the OVN LoadBalancer\u0027s"}],"source_content_type":"text/x-python","patch_set":36,"id":"88ea68ab_c3806c78","line":1114,"in_reply_to":"334d82e4_e575aa17","updated":"2025-02-14 11:46:36.000000000","message":"Done","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cd1591ebd10ae04421588f62993d73e0cc77059c","unresolved":true,"context_lines":[{"line_number":1135,"context_line":"        port \u003d None"},{"line_number":1136,"context_line":"        subnet \u003d None"},{"line_number":1137,"context_line":"        try:"},{"line_number":1138,"context_line":"            neutron_client \u003d clients.get_neutron_client()"},{"line_number":1139,"context_line":"            port, subnet \u003d self._get_port_from_info("},{"line_number":1140,"context_line":"                neutron_client,"},{"line_number":1141,"context_line":"                loadbalancer.get(constants.VIP_PORT_ID, None),"},{"line_number":1142,"context_line":"                loadbalancer.get(constants.VIP_NETWORK_ID, None),"}],"source_content_type":"text/x-python","patch_set":36,"id":"a2e641fe_6e808c01","line":1139,"range":{"start_line":1138,"start_character":0,"end_line":1139,"end_character":52},"updated":"2025-01-22 16:50:43.000000000","message":"These two should have their own try-blocks as get_neutron_client seems to raise DriverError only. With it you can provide better information than just \"something has failed\".","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"206343420e61da12291d91bd242fc5fb7e85a9b2","unresolved":false,"context_lines":[{"line_number":1135,"context_line":"        port \u003d None"},{"line_number":1136,"context_line":"        subnet \u003d None"},{"line_number":1137,"context_line":"        try:"},{"line_number":1138,"context_line":"            neutron_client \u003d clients.get_neutron_client()"},{"line_number":1139,"context_line":"            port, subnet \u003d self._get_port_from_info("},{"line_number":1140,"context_line":"                neutron_client,"},{"line_number":1141,"context_line":"                loadbalancer.get(constants.VIP_PORT_ID, None),"},{"line_number":1142,"context_line":"                loadbalancer.get(constants.VIP_NETWORK_ID, None),"}],"source_content_type":"text/x-python","patch_set":36,"id":"48b6bdd2_ebbf5500","line":1139,"range":{"start_line":1138,"start_character":0,"end_line":1139,"end_character":52},"in_reply_to":"a2e641fe_6e808c01","updated":"2025-01-23 14:16:56.000000000","message":"Done","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cd1591ebd10ae04421588f62993d73e0cc77059c","unresolved":true,"context_lines":[{"line_number":1141,"context_line":"                loadbalancer.get(constants.VIP_PORT_ID, None),"},{"line_number":1142,"context_line":"                loadbalancer.get(constants.VIP_NETWORK_ID, None),"},{"line_number":1143,"context_line":"                loadbalancer.get(constants.VIP_ADDRESS, None))"},{"line_number":1144,"context_line":"        except Exception:"},{"line_number":1145,"context_line":"            LOG.warn(\u0027Cannot get info from neutron for loadbalancer \u0027"},{"line_number":1146,"context_line":"                     \u0027sync.\u0027)"},{"line_number":1147,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":36,"id":"23336cf4_30058aff","line":1144,"range":{"start_line":1144,"start_character":15,"end_line":1144,"end_character":24},"updated":"2025-01-22 16:50:43.000000000","message":"Catching overly broad exceptions is a Python anti-pattern. There should be specific exception from the used neutron client `get()` methods and the message should explain what has failed.","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"50b1170ec6491297a5565dcdb5c7197ea2576fcb","unresolved":true,"context_lines":[{"line_number":1141,"context_line":"                loadbalancer.get(constants.VIP_PORT_ID, None),"},{"line_number":1142,"context_line":"                loadbalancer.get(constants.VIP_NETWORK_ID, None),"},{"line_number":1143,"context_line":"                loadbalancer.get(constants.VIP_ADDRESS, None))"},{"line_number":1144,"context_line":"        except Exception:"},{"line_number":1145,"context_line":"            LOG.warn(\u0027Cannot get info from neutron for loadbalancer \u0027"},{"line_number":1146,"context_line":"                     \u0027sync.\u0027)"},{"line_number":1147,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":36,"id":"89d0864d_90a61eeb","line":1144,"range":{"start_line":1144,"start_character":15,"end_line":1144,"end_character":24},"in_reply_to":"23336cf4_30058aff","updated":"2025-01-23 06:29:32.000000000","message":"This is just another adopt from current design like in lb_create. maybe someone can help with doing categories exception as followup?","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"206343420e61da12291d91bd242fc5fb7e85a9b2","unresolved":true,"context_lines":[{"line_number":1141,"context_line":"                loadbalancer.get(constants.VIP_PORT_ID, None),"},{"line_number":1142,"context_line":"                loadbalancer.get(constants.VIP_NETWORK_ID, None),"},{"line_number":1143,"context_line":"                loadbalancer.get(constants.VIP_ADDRESS, None))"},{"line_number":1144,"context_line":"        except Exception:"},{"line_number":1145,"context_line":"            LOG.warn(\u0027Cannot get info from neutron for loadbalancer \u0027"},{"line_number":1146,"context_line":"                     \u0027sync.\u0027)"},{"line_number":1147,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":36,"id":"a186c82d_f7071b52","line":1144,"range":{"start_line":1144,"start_character":15,"end_line":1144,"end_character":24},"in_reply_to":"89d0864d_90a61eeb","updated":"2025-01-23 14:16:56.000000000","message":"IIUC Jakub\u0027s comments indicates to cover here any possible exception triggered by \u0027_get_port_from_info()\u0027 method, right?","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"0702d2f76e28876777697435b344ab8d1ec638be","unresolved":false,"context_lines":[{"line_number":1141,"context_line":"                loadbalancer.get(constants.VIP_PORT_ID, None),"},{"line_number":1142,"context_line":"                loadbalancer.get(constants.VIP_NETWORK_ID, None),"},{"line_number":1143,"context_line":"                loadbalancer.get(constants.VIP_ADDRESS, None))"},{"line_number":1144,"context_line":"        except Exception:"},{"line_number":1145,"context_line":"            LOG.warn(\u0027Cannot get info from neutron for loadbalancer \u0027"},{"line_number":1146,"context_line":"                     \u0027sync.\u0027)"},{"line_number":1147,"context_line":"            return False"}],"source_content_type":"text/x-python","patch_set":36,"id":"f2cc88ec_8b556778","line":1144,"range":{"start_line":1144,"start_character":15,"end_line":1144,"end_character":24},"in_reply_to":"a186c82d_f7071b52","updated":"2025-01-27 18:52:50.000000000","message":"Done","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cd1591ebd10ae04421588f62993d73e0cc77059c","unresolved":true,"context_lines":[{"line_number":1144,"context_line":"        except Exception:"},{"line_number":1145,"context_line":"            LOG.warn(\u0027Cannot get info from neutron for loadbalancer \u0027"},{"line_number":1146,"context_line":"                     \u0027sync.\u0027)"},{"line_number":1147,"context_line":"            return False"},{"line_number":1148,"context_line":""},{"line_number":1149,"context_line":"        # If protocol set make sure its lowercase"},{"line_number":1150,"context_line":"        protocol \u003d ovn_lb.protocol[0].lower() if ovn_lb.protocol else []"}],"source_content_type":"text/x-python","patch_set":36,"id":"56f8bdd2_efb17be8","line":1147,"range":{"start_line":1147,"start_character":19,"end_line":1147,"end_character":24},"updated":"2025-01-22 16:50:43.000000000","message":"Where is this used?","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"50b1170ec6491297a5565dcdb5c7197ea2576fcb","unresolved":false,"context_lines":[{"line_number":1144,"context_line":"        except Exception:"},{"line_number":1145,"context_line":"            LOG.warn(\u0027Cannot get info from neutron for loadbalancer \u0027"},{"line_number":1146,"context_line":"                     \u0027sync.\u0027)"},{"line_number":1147,"context_line":"            return False"},{"line_number":1148,"context_line":""},{"line_number":1149,"context_line":"        # If protocol set make sure its lowercase"},{"line_number":1150,"context_line":"        protocol \u003d ovn_lb.protocol[0].lower() if ovn_lb.protocol else []"}],"source_content_type":"text/x-python","patch_set":36,"id":"1ed0bdf9_e954b51f","line":1147,"range":{"start_line":1147,"start_character":19,"end_line":1147,"end_character":24},"in_reply_to":"56f8bdd2_efb17be8","updated":"2025-01-23 06:29:32.000000000","message":"Done","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cd1591ebd10ae04421588f62993d73e0cc77059c","unresolved":true,"context_lines":[{"line_number":1147,"context_line":"            return False"},{"line_number":1148,"context_line":""},{"line_number":1149,"context_line":"        # If protocol set make sure its lowercase"},{"line_number":1150,"context_line":"        protocol \u003d ovn_lb.protocol[0].lower() if ovn_lb.protocol else []"},{"line_number":1151,"context_line":"        # In case port is not found for the vip_address we will see an"},{"line_number":1152,"context_line":"        # exception when port[\u0027id\u0027] is accessed."},{"line_number":1153,"context_line":"        external_ids \u003d {"}],"source_content_type":"text/x-python","patch_set":36,"id":"24e4e48e_19029d59","line":1150,"range":{"start_line":1150,"start_character":70,"end_line":1150,"end_character":72},"updated":"2025-01-22 16:50:43.000000000","message":"Would be good to have just one type in a variable. As this is only used on L1213 and the default is `None`, then it would make sense to be consistent and use None here too if the protocol is not set.","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"50b1170ec6491297a5565dcdb5c7197ea2576fcb","unresolved":false,"context_lines":[{"line_number":1147,"context_line":"            return False"},{"line_number":1148,"context_line":""},{"line_number":1149,"context_line":"        # If protocol set make sure its lowercase"},{"line_number":1150,"context_line":"        protocol \u003d ovn_lb.protocol[0].lower() if ovn_lb.protocol else []"},{"line_number":1151,"context_line":"        # In case port is not found for the vip_address we will see an"},{"line_number":1152,"context_line":"        # exception when port[\u0027id\u0027] is accessed."},{"line_number":1153,"context_line":"        external_ids \u003d {"}],"source_content_type":"text/x-python","patch_set":36,"id":"6cd307ad_81ee395a","line":1150,"range":{"start_line":1150,"start_character":70,"end_line":1150,"end_character":72},"in_reply_to":"24e4e48e_19029d59","updated":"2025-01-23 06:29:32.000000000","message":"Done","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cd1591ebd10ae04421588f62993d73e0cc77059c","unresolved":true,"context_lines":[{"line_number":1153,"context_line":"        external_ids \u003d {"},{"line_number":1154,"context_line":"            ovn_const.LB_EXT_IDS_VIP_KEY: loadbalancer[constants.VIP_ADDRESS],"},{"line_number":1155,"context_line":"            ovn_const.LB_EXT_IDS_VIP_PORT_ID_KEY:"},{"line_number":1156,"context_line":"                loadbalancer.get(constants.VIP_PORT_ID) or port.id,"},{"line_number":1157,"context_line":"            \u0027enabled\u0027: str(loadbalancer[constants.ADMIN_STATE_UP])}"},{"line_number":1158,"context_line":""},{"line_number":1159,"context_line":"        # In case additional_vips was passed"}],"source_content_type":"text/x-python","patch_set":36,"id":"6c372435_cd677ee3","line":1156,"range":{"start_line":1156,"start_character":55,"end_line":1156,"end_character":66},"updated":"2025-01-22 16:50:43.000000000","message":"Q: Is this because VIP_PORT_ID can be set to an empty string?","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"206343420e61da12291d91bd242fc5fb7e85a9b2","unresolved":true,"context_lines":[{"line_number":1153,"context_line":"        external_ids \u003d {"},{"line_number":1154,"context_line":"            ovn_const.LB_EXT_IDS_VIP_KEY: loadbalancer[constants.VIP_ADDRESS],"},{"line_number":1155,"context_line":"            ovn_const.LB_EXT_IDS_VIP_PORT_ID_KEY:"},{"line_number":1156,"context_line":"                loadbalancer.get(constants.VIP_PORT_ID) or port.id,"},{"line_number":1157,"context_line":"            \u0027enabled\u0027: str(loadbalancer[constants.ADMIN_STATE_UP])}"},{"line_number":1158,"context_line":""},{"line_number":1159,"context_line":"        # In case additional_vips was passed"}],"source_content_type":"text/x-python","patch_set":36,"id":"b2bc5f40_4b98dd34","line":1156,"range":{"start_line":1156,"start_character":55,"end_line":1156,"end_character":66},"in_reply_to":"1cb2ead7_8cc89ab9","updated":"2025-01-23 14:16:56.000000000","message":"yeah, in a normal LB creation, the VIP port is created before any further action over the OVN NB LB table, so in this lb_sync if loadbalancer object doesn\u0027t give us  the info (for whatever reason) we can infer from port object.","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"50b1170ec6491297a5565dcdb5c7197ea2576fcb","unresolved":true,"context_lines":[{"line_number":1153,"context_line":"        external_ids \u003d {"},{"line_number":1154,"context_line":"            ovn_const.LB_EXT_IDS_VIP_KEY: loadbalancer[constants.VIP_ADDRESS],"},{"line_number":1155,"context_line":"            ovn_const.LB_EXT_IDS_VIP_PORT_ID_KEY:"},{"line_number":1156,"context_line":"                loadbalancer.get(constants.VIP_PORT_ID) or port.id,"},{"line_number":1157,"context_line":"            \u0027enabled\u0027: str(loadbalancer[constants.ADMIN_STATE_UP])}"},{"line_number":1158,"context_line":""},{"line_number":1159,"context_line":"        # In case additional_vips was passed"}],"source_content_type":"text/x-python","patch_set":36,"id":"1cb2ead7_8cc89ab9","line":1156,"range":{"start_line":1156,"start_character":55,"end_line":1156,"end_character":66},"in_reply_to":"6c372435_cd677ee3","updated":"2025-01-23 06:29:32.000000000","message":"I think so, getting this from `lb_create`","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"0702d2f76e28876777697435b344ab8d1ec638be","unresolved":false,"context_lines":[{"line_number":1153,"context_line":"        external_ids \u003d {"},{"line_number":1154,"context_line":"            ovn_const.LB_EXT_IDS_VIP_KEY: loadbalancer[constants.VIP_ADDRESS],"},{"line_number":1155,"context_line":"            ovn_const.LB_EXT_IDS_VIP_PORT_ID_KEY:"},{"line_number":1156,"context_line":"                loadbalancer.get(constants.VIP_PORT_ID) or port.id,"},{"line_number":1157,"context_line":"            \u0027enabled\u0027: str(loadbalancer[constants.ADMIN_STATE_UP])}"},{"line_number":1158,"context_line":""},{"line_number":1159,"context_line":"        # In case additional_vips was passed"}],"source_content_type":"text/x-python","patch_set":36,"id":"10998735_56a9b48f","line":1156,"range":{"start_line":1156,"start_character":55,"end_line":1156,"end_character":66},"in_reply_to":"b2bc5f40_4b98dd34","updated":"2025-01-27 18:52:50.000000000","message":"Done","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cd1591ebd10ae04421588f62993d73e0cc77059c","unresolved":true,"context_lines":[{"line_number":1159,"context_line":"        # In case additional_vips was passed"},{"line_number":1160,"context_line":"        if loadbalancer.get(constants.ADDITIONAL_VIPS):"},{"line_number":1161,"context_line":"            addi_vip \u003d [x[\u0027ip_address\u0027]"},{"line_number":1162,"context_line":"                        for x in loadbalancer.get(constants.ADDITIONAL_VIPS)]"},{"line_number":1163,"context_line":"            addi_vip_port_id \u003d [x[\u0027port_id\u0027]"},{"line_number":1164,"context_line":"                                for x in loadbalancer.get("},{"line_number":1165,"context_line":"                                    constants.ADDITIONAL_VIPS)]"}],"source_content_type":"text/x-python","patch_set":36,"id":"7c88b4e8_b96e4237","line":1162,"range":{"start_line":1162,"start_character":33,"end_line":1162,"end_character":76},"updated":"2025-01-22 16:50:43.000000000","message":"`loadbalancer[constants.ADDITIONAL_VIPS]` as you know there are some already because of L1160","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"50b1170ec6491297a5565dcdb5c7197ea2576fcb","unresolved":true,"context_lines":[{"line_number":1159,"context_line":"        # In case additional_vips was passed"},{"line_number":1160,"context_line":"        if loadbalancer.get(constants.ADDITIONAL_VIPS):"},{"line_number":1161,"context_line":"            addi_vip \u003d [x[\u0027ip_address\u0027]"},{"line_number":1162,"context_line":"                        for x in loadbalancer.get(constants.ADDITIONAL_VIPS)]"},{"line_number":1163,"context_line":"            addi_vip_port_id \u003d [x[\u0027port_id\u0027]"},{"line_number":1164,"context_line":"                                for x in loadbalancer.get("},{"line_number":1165,"context_line":"                                    constants.ADDITIONAL_VIPS)]"}],"source_content_type":"text/x-python","patch_set":36,"id":"f421e2f9_6704551a","line":1162,"range":{"start_line":1162,"start_character":33,"end_line":1162,"end_character":76},"in_reply_to":"7c88b4e8_b96e4237","updated":"2025-01-23 06:29:32.000000000","message":"just to get ip_address map","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"0702d2f76e28876777697435b344ab8d1ec638be","unresolved":false,"context_lines":[{"line_number":1159,"context_line":"        # In case additional_vips was passed"},{"line_number":1160,"context_line":"        if loadbalancer.get(constants.ADDITIONAL_VIPS):"},{"line_number":1161,"context_line":"            addi_vip \u003d [x[\u0027ip_address\u0027]"},{"line_number":1162,"context_line":"                        for x in loadbalancer.get(constants.ADDITIONAL_VIPS)]"},{"line_number":1163,"context_line":"            addi_vip_port_id \u003d [x[\u0027port_id\u0027]"},{"line_number":1164,"context_line":"                                for x in loadbalancer.get("},{"line_number":1165,"context_line":"                                    constants.ADDITIONAL_VIPS)]"}],"source_content_type":"text/x-python","patch_set":36,"id":"7be4c502_5cb79820","line":1162,"range":{"start_line":1162,"start_character":33,"end_line":1162,"end_character":76},"in_reply_to":"b4bd5154_a24b6c9e","updated":"2025-01-27 18:52:50.000000000","message":"Done","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"206343420e61da12291d91bd242fc5fb7e85a9b2","unresolved":true,"context_lines":[{"line_number":1159,"context_line":"        # In case additional_vips was passed"},{"line_number":1160,"context_line":"        if loadbalancer.get(constants.ADDITIONAL_VIPS):"},{"line_number":1161,"context_line":"            addi_vip \u003d [x[\u0027ip_address\u0027]"},{"line_number":1162,"context_line":"                        for x in loadbalancer.get(constants.ADDITIONAL_VIPS)]"},{"line_number":1163,"context_line":"            addi_vip_port_id \u003d [x[\u0027port_id\u0027]"},{"line_number":1164,"context_line":"                                for x in loadbalancer.get("},{"line_number":1165,"context_line":"                                    constants.ADDITIONAL_VIPS)]"}],"source_content_type":"text/x-python","patch_set":36,"id":"b4bd5154_a24b6c9e","line":1162,"range":{"start_line":1162,"start_character":33,"end_line":1162,"end_character":76},"in_reply_to":"f421e2f9_6704551a","updated":"2025-01-23 14:16:56.000000000","message":"I think Jakub\u0027s comment propose \n```suggestion\n                        for x in loadbalancer[constants.ADDITIONAL_VIPS]\n```\n\nas we are already check in L1160 that we have something there.","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cd1591ebd10ae04421588f62993d73e0cc77059c","unresolved":true,"context_lines":[{"line_number":1162,"context_line":"                        for x in loadbalancer.get(constants.ADDITIONAL_VIPS)]"},{"line_number":1163,"context_line":"            addi_vip_port_id \u003d [x[\u0027port_id\u0027]"},{"line_number":1164,"context_line":"                                for x in loadbalancer.get("},{"line_number":1165,"context_line":"                                    constants.ADDITIONAL_VIPS)]"},{"line_number":1166,"context_line":"            addi_vip \u003d \u0027,\u0027.join(addi_vip)"},{"line_number":1167,"context_line":"            addi_vip_port_id \u003d \u0027,\u0027.join(addi_vip_port_id)"},{"line_number":1168,"context_line":"            external_ids[ovn_const.LB_EXT_IDS_ADDIT_VIP_KEY] \u003d addi_vip"}],"source_content_type":"text/x-python","patch_set":36,"id":"dda5ff95_a2309400","line":1165,"updated":"2025-01-22 16:50:43.000000000","message":"ditto","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"206343420e61da12291d91bd242fc5fb7e85a9b2","unresolved":true,"context_lines":[{"line_number":1162,"context_line":"                        for x in loadbalancer.get(constants.ADDITIONAL_VIPS)]"},{"line_number":1163,"context_line":"            addi_vip_port_id \u003d [x[\u0027port_id\u0027]"},{"line_number":1164,"context_line":"                                for x in loadbalancer.get("},{"line_number":1165,"context_line":"                                    constants.ADDITIONAL_VIPS)]"},{"line_number":1166,"context_line":"            addi_vip \u003d \u0027,\u0027.join(addi_vip)"},{"line_number":1167,"context_line":"            addi_vip_port_id \u003d \u0027,\u0027.join(addi_vip_port_id)"},{"line_number":1168,"context_line":"            external_ids[ovn_const.LB_EXT_IDS_ADDIT_VIP_KEY] \u003d addi_vip"}],"source_content_type":"text/x-python","patch_set":36,"id":"96cc6444_876690e5","line":1165,"in_reply_to":"55acefbd_784e678b","updated":"2025-01-23 14:16:56.000000000","message":"same as previous one","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"0702d2f76e28876777697435b344ab8d1ec638be","unresolved":false,"context_lines":[{"line_number":1162,"context_line":"                        for x in loadbalancer.get(constants.ADDITIONAL_VIPS)]"},{"line_number":1163,"context_line":"            addi_vip_port_id \u003d [x[\u0027port_id\u0027]"},{"line_number":1164,"context_line":"                                for x in loadbalancer.get("},{"line_number":1165,"context_line":"                                    constants.ADDITIONAL_VIPS)]"},{"line_number":1166,"context_line":"            addi_vip \u003d \u0027,\u0027.join(addi_vip)"},{"line_number":1167,"context_line":"            addi_vip_port_id \u003d \u0027,\u0027.join(addi_vip_port_id)"},{"line_number":1168,"context_line":"            external_ids[ovn_const.LB_EXT_IDS_ADDIT_VIP_KEY] \u003d addi_vip"}],"source_content_type":"text/x-python","patch_set":36,"id":"79dd595e_68125517","line":1165,"in_reply_to":"96cc6444_876690e5","updated":"2025-01-27 18:52:50.000000000","message":"Done","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"50b1170ec6491297a5565dcdb5c7197ea2576fcb","unresolved":true,"context_lines":[{"line_number":1162,"context_line":"                        for x in loadbalancer.get(constants.ADDITIONAL_VIPS)]"},{"line_number":1163,"context_line":"            addi_vip_port_id \u003d [x[\u0027port_id\u0027]"},{"line_number":1164,"context_line":"                                for x in loadbalancer.get("},{"line_number":1165,"context_line":"                                    constants.ADDITIONAL_VIPS)]"},{"line_number":1166,"context_line":"            addi_vip \u003d \u0027,\u0027.join(addi_vip)"},{"line_number":1167,"context_line":"            addi_vip_port_id \u003d \u0027,\u0027.join(addi_vip_port_id)"},{"line_number":1168,"context_line":"            external_ids[ovn_const.LB_EXT_IDS_ADDIT_VIP_KEY] \u003d addi_vip"}],"source_content_type":"text/x-python","patch_set":36,"id":"55acefbd_784e678b","line":1165,"in_reply_to":"dda5ff95_a2309400","updated":"2025-01-23 06:29:32.000000000","message":"just to get port_id map","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cd1591ebd10ae04421588f62993d73e0cc77059c","unresolved":true,"context_lines":[{"line_number":1166,"context_line":"            addi_vip \u003d \u0027,\u0027.join(addi_vip)"},{"line_number":1167,"context_line":"            addi_vip_port_id \u003d \u0027,\u0027.join(addi_vip_port_id)"},{"line_number":1168,"context_line":"            external_ids[ovn_const.LB_EXT_IDS_ADDIT_VIP_KEY] \u003d addi_vip"},{"line_number":1169,"context_line":"            external_ids[ovn_const.LB_EXT_IDS_ADDIT_VIP_PORT_ID_KEY] \u003d \\"},{"line_number":1170,"context_line":"                addi_vip_port_id"},{"line_number":1171,"context_line":""},{"line_number":1172,"context_line":"        # In case vip_fip was passed - use it."}],"source_content_type":"text/x-python","patch_set":36,"id":"1eb4e235_c1ace664","line":1169,"range":{"start_line":1169,"start_character":71,"end_line":1169,"end_character":72},"updated":"2025-01-22 16:50:43.000000000","message":"parenthesis are preferable","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"50b1170ec6491297a5565dcdb5c7197ea2576fcb","unresolved":false,"context_lines":[{"line_number":1166,"context_line":"            addi_vip \u003d \u0027,\u0027.join(addi_vip)"},{"line_number":1167,"context_line":"            addi_vip_port_id \u003d \u0027,\u0027.join(addi_vip_port_id)"},{"line_number":1168,"context_line":"            external_ids[ovn_const.LB_EXT_IDS_ADDIT_VIP_KEY] \u003d addi_vip"},{"line_number":1169,"context_line":"            external_ids[ovn_const.LB_EXT_IDS_ADDIT_VIP_PORT_ID_KEY] \u003d \\"},{"line_number":1170,"context_line":"                addi_vip_port_id"},{"line_number":1171,"context_line":""},{"line_number":1172,"context_line":"        # In case vip_fip was passed - use it."}],"source_content_type":"text/x-python","patch_set":36,"id":"5b85f6b8_b0bb1ee2","line":1169,"range":{"start_line":1169,"start_character":71,"end_line":1169,"end_character":72},"in_reply_to":"1eb4e235_c1ace664","updated":"2025-01-23 06:29:32.000000000","message":"Done","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cd1591ebd10ae04421588f62993d73e0cc77059c","unresolved":true,"context_lines":[{"line_number":1177,"context_line":"        additional_vip_fip \u003d loadbalancer.get("},{"line_number":1178,"context_line":"            ovn_const.LB_EXT_IDS_ADDIT_VIP_FIP_KEY)"},{"line_number":1179,"context_line":"        if additional_vip_fip:"},{"line_number":1180,"context_line":"            external_ids[ovn_const.LB_EXT_IDS_ADDIT_VIP_FIP_KEY] \u003d \\"},{"line_number":1181,"context_line":"                additional_vip_fip"},{"line_number":1182,"context_line":"        # In case of lr_ref passed - use it."},{"line_number":1183,"context_line":"        lr_ref \u003d loadbalancer.get(ovn_const.LB_EXT_IDS_LR_REF_KEY)"}],"source_content_type":"text/x-python","patch_set":36,"id":"122b8a0a_18dc0038","line":1180,"range":{"start_line":1180,"start_character":67,"end_line":1180,"end_character":68},"updated":"2025-01-22 16:50:43.000000000","message":"ditto","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"50b1170ec6491297a5565dcdb5c7197ea2576fcb","unresolved":false,"context_lines":[{"line_number":1177,"context_line":"        additional_vip_fip \u003d loadbalancer.get("},{"line_number":1178,"context_line":"            ovn_const.LB_EXT_IDS_ADDIT_VIP_FIP_KEY)"},{"line_number":1179,"context_line":"        if additional_vip_fip:"},{"line_number":1180,"context_line":"            external_ids[ovn_const.LB_EXT_IDS_ADDIT_VIP_FIP_KEY] \u003d \\"},{"line_number":1181,"context_line":"                additional_vip_fip"},{"line_number":1182,"context_line":"        # In case of lr_ref passed - use it."},{"line_number":1183,"context_line":"        lr_ref \u003d loadbalancer.get(ovn_const.LB_EXT_IDS_LR_REF_KEY)"}],"source_content_type":"text/x-python","patch_set":36,"id":"c07c363d_cb7f7a74","line":1180,"range":{"start_line":1180,"start_character":67,"end_line":1180,"end_character":68},"in_reply_to":"122b8a0a_18dc0038","updated":"2025-01-23 06:29:32.000000000","message":"Done","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cd1591ebd10ae04421588f62993d73e0cc77059c","unresolved":true,"context_lines":[{"line_number":1199,"context_line":"                    \u0027Load_Balancer\u0027, ovn_lb.uuid,"},{"line_number":1200,"context_line":"                    (\u0027external_ids\u0027, external_ids))"},{"line_number":1201,"context_line":"            )"},{"line_number":1202,"context_line":"        if selection_fields is not None and ("},{"line_number":1203,"context_line":"                selection_fields !\u003d ovn_lb.selection_fields):"},{"line_number":1204,"context_line":"            commands.append("},{"line_number":1205,"context_line":"                self.ovn_nbdb_api.db_set("}],"source_content_type":"text/x-python","patch_set":36,"id":"8e1b642a_478ceb97","line":1202,"range":{"start_line":1202,"start_character":11,"end_line":1202,"end_character":39},"updated":"2025-01-22 16:50:43.000000000","message":"this is essentially double-checking as this variable is directly set by the value of `self._are_selection_fields_supported()`","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"50b1170ec6491297a5565dcdb5c7197ea2576fcb","unresolved":false,"context_lines":[{"line_number":1199,"context_line":"                    \u0027Load_Balancer\u0027, ovn_lb.uuid,"},{"line_number":1200,"context_line":"                    (\u0027external_ids\u0027, external_ids))"},{"line_number":1201,"context_line":"            )"},{"line_number":1202,"context_line":"        if selection_fields is not None and ("},{"line_number":1203,"context_line":"                selection_fields !\u003d ovn_lb.selection_fields):"},{"line_number":1204,"context_line":"            commands.append("},{"line_number":1205,"context_line":"                self.ovn_nbdb_api.db_set("}],"source_content_type":"text/x-python","patch_set":36,"id":"9f50fe27_b9b4f492","line":1202,"range":{"start_line":1202,"start_character":11,"end_line":1202,"end_character":39},"in_reply_to":"8e1b642a_478ceb97","updated":"2025-01-23 06:29:32.000000000","message":"selection_fields already checked in L1191","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"cd1591ebd10ae04421588f62993d73e0cc77059c","unresolved":true,"context_lines":[{"line_number":1205,"context_line":"                self.ovn_nbdb_api.db_set("},{"line_number":1206,"context_line":"                    \u0027Load_Balancer\u0027, ovn_lb.uuid,"},{"line_number":1207,"context_line":"                    (\u0027selection_fields\u0027, selection_fields))"},{"line_number":1208,"context_line":"            )"},{"line_number":1209,"context_line":"        try:"},{"line_number":1210,"context_line":"            self._execute_commands(commands)"},{"line_number":1211,"context_line":"            ovn_lb \u003d self._find_ovn_lbs_with_retry("}],"source_content_type":"text/x-python","patch_set":36,"id":"7e82534c_1179760e","line":1208,"updated":"2025-01-22 16:50:43.000000000","message":"In general I wouldn\u0027t bother much by avoiding calling db_set on something that has the same value. If the db_set command set same values then IDL notices it and considers it a noop operation - not sending anything to the remote ovsdb-server. The checks here just increases code complexity for no value. Consider changing lines L1190-L1208 to just:\n```\ndb_set_args \u003d {\u0027external_ids\u0027: external_ids}\nif self._are_selection_fields_supported():\n    db_set_args[\u0027selection_fields\u0027] \u003d selection_fields\ncommands.append(\n    self.ovn_nbdb_api.db_set(\n        \u0027Load_Balancer\u0027, ovn_lb.uuid, **db_set_args))\n```","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"50b1170ec6491297a5565dcdb5c7197ea2576fcb","unresolved":false,"context_lines":[{"line_number":1205,"context_line":"                self.ovn_nbdb_api.db_set("},{"line_number":1206,"context_line":"                    \u0027Load_Balancer\u0027, ovn_lb.uuid,"},{"line_number":1207,"context_line":"                    (\u0027selection_fields\u0027, selection_fields))"},{"line_number":1208,"context_line":"            )"},{"line_number":1209,"context_line":"        try:"},{"line_number":1210,"context_line":"            self._execute_commands(commands)"},{"line_number":1211,"context_line":"            ovn_lb \u003d self._find_ovn_lbs_with_retry("}],"source_content_type":"text/x-python","patch_set":36,"id":"5837a4c4_db24e6da","line":1208,"in_reply_to":"7e82534c_1179760e","updated":"2025-01-23 06:29:32.000000000","message":"The check with `selection_fields !\u003d ovn_lb.selection_fields` is to avoid unnecessary db set to OVN when they\u0027re identical. I think that\u0027s the value in this part.","commit_id":"745498ad8e308658bf010274ed67af3d1cad3890"}],"ovn_octavia_provider/ovsdb/impl_idl_ovn.py":[{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"79a14f0e93092726ba650a0eac271fb09e253cf6","unresolved":true,"context_lines":[{"line_number":147,"context_line":"    def run_idl(self, txn):"},{"line_number":148,"context_line":"        try:"},{"line_number":149,"context_line":"            lb \u003d self.api.lookup(self.table, self.lb)"},{"line_number":150,"context_line":"            if not self.is_sync or not ("},{"line_number":151,"context_line":"                (self.backend_ip in lb.ip_port_mappings) and ("},{"line_number":152,"context_line":"                    str(lb.ip_port_mappings.get("},{"line_number":153,"context_line":"                        self.backend_ip)) \u003d\u003d f\"{self.port_name}:{self.src_ip}\""}],"source_content_type":"text/x-python","patch_set":12,"id":"97b8970f_4d68df49","line":150,"updated":"2024-08-28 16:27:09.000000000","message":"The right branch of \u0027or\u0027 in the if statement is an improvement that doesn\u0027t seem directly related to the patch. I don\u0027t doubt that it improves the code, but it might be better to include it in a separate patch so it doesn\u0027t get lost in case of a hypothetical revert.","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":34451,"name":"Fernando Royo","email":"froyo@redhat.com","username":"froyo"},"change_message_id":"09db1e01434e403e5dc741b398330909644a1491","unresolved":false,"context_lines":[{"line_number":147,"context_line":"    def run_idl(self, txn):"},{"line_number":148,"context_line":"        try:"},{"line_number":149,"context_line":"            lb \u003d self.api.lookup(self.table, self.lb)"},{"line_number":150,"context_line":"            if not self.is_sync or not ("},{"line_number":151,"context_line":"                (self.backend_ip in lb.ip_port_mappings) and ("},{"line_number":152,"context_line":"                    str(lb.ip_port_mappings.get("},{"line_number":153,"context_line":"                        self.backend_ip)) \u003d\u003d f\"{self.port_name}:{self.src_ip}\""}],"source_content_type":"text/x-python","patch_set":12,"id":"4c46c012_03fc86b6","line":150,"in_reply_to":"4c5e462a_a8574732","updated":"2024-08-29 09:53:50.000000000","message":"Acknowledged","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"c8327ecdd69c9301ee8f0dd6eda26ff9f52cb89c","unresolved":true,"context_lines":[{"line_number":147,"context_line":"    def run_idl(self, txn):"},{"line_number":148,"context_line":"        try:"},{"line_number":149,"context_line":"            lb \u003d self.api.lookup(self.table, self.lb)"},{"line_number":150,"context_line":"            if not self.is_sync or not ("},{"line_number":151,"context_line":"                (self.backend_ip in lb.ip_port_mappings) and ("},{"line_number":152,"context_line":"                    str(lb.ip_port_mappings.get("},{"line_number":153,"context_line":"                        self.backend_ip)) \u003d\u003d f\"{self.port_name}:{self.src_ip}\""}],"source_content_type":"text/x-python","patch_set":12,"id":"4c5e462a_a8574732","line":150,"in_reply_to":"97b8970f_4d68df49","updated":"2024-08-29 05:37:13.000000000","message":"These are sync related, will make it clear with comment","commit_id":"ec7c8387dd8a2e7454a48cb79dbb880e32ac72b7"}]}