)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"2c1f5b568dd6a72684ffbc460f23f7c6151e8e5a","unresolved":true,"context_lines":[{"line_number":26,"context_line":"underlying OVS Python IDL is kept up to date during the course of"},{"line_number":27,"context_line":"the transaction [0][1][2]."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Move implementation of schduling of unhosted gateways into an"},{"line_number":30,"context_line":"ovsdbapp command, using a plugin reference to the Neutron"},{"line_number":31,"context_line":"OVNClient class for any calls into the Neutron code, allowing"},{"line_number":32,"context_line":"scheduling decisions to be made on up to date data as the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":51,"id":"9b4cee51_87607d67","line":29,"range":{"start_line":29,"start_character":23,"end_line":29,"end_character":32},"updated":"2023-12-08 23:00:31.000000000","message":"nit: scheduling","commit_id":"6127824bdd5d2bfbdf5c22e992a8d5f3978578dd"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"4e9c72782a656466e8facd84281568d9960546f4","unresolved":false,"context_lines":[{"line_number":26,"context_line":"underlying OVS Python IDL is kept up to date during the course of"},{"line_number":27,"context_line":"the transaction [0][1][2]."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"Move implementation of schduling of unhosted gateways into an"},{"line_number":30,"context_line":"ovsdbapp command, using a plugin reference to the Neutron"},{"line_number":31,"context_line":"OVNClient class for any calls into the Neutron code, allowing"},{"line_number":32,"context_line":"scheduling decisions to be made on up to date data as the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":51,"id":"820c309f_bd5dbaa0","line":29,"range":{"start_line":29,"start_character":23,"end_line":29,"end_character":32},"in_reply_to":"9b4cee51_87607d67","updated":"2024-01-26 15:49:37.000000000","message":"Done","commit_id":"6127824bdd5d2bfbdf5c22e992a8d5f3978578dd"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":24824,"name":"Dmitrii Shcherbakov","username":"dmitriis"},"change_message_id":"eb894c5fa9b55de2f9e4fab4fb6c0d6896effa86","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"a297b33e_ac7fd040","updated":"2023-04-30 14:12:28.000000000","message":"recheck vexxhost nested virt hosts disabled","commit_id":"18e295eba976e6be60319c92a6b75c2133d505ea"},{"author":{"_account_id":24824,"name":"Dmitrii Shcherbakov","username":"dmitriis"},"change_message_id":"5de8009e7849c1c84530bca643c97e2d395980ab","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"a75b6a25_6b7c8ad7","updated":"2023-04-28 13:46:31.000000000","message":"recheck vexxhost nested virt hosts disabled","commit_id":"18e295eba976e6be60319c92a6b75c2133d505ea"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"c1f0256a3cd05c15318733b3f73209e781f85029","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":34,"id":"daa795b2_814964c5","updated":"2023-06-16 22:47:43.000000000","message":"As an example that it works:\n\nhttps://paste.openstack.org/show/820387/\n\nwould be the more ovsdbapp-api-friendly way to do things. It\u0027s still a little ugly due to passing in the plugin (which I really just do for the ovn_client call, otherwise I would have passed the scheduler) and I didn\u0027t do anything with the unit test. But it passes the functional test. Though there\u0027s nothing ostensibly wrong with making an API call that requires a plugin object.\n\nThere doesn\u0027t seem to really be any reason to even use update_lrouter_port() for this special case of scheduling gateways.","commit_id":"d95e2e41d2dbda1d4c102ed814cd94227b0dcd78"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"2533bd1b8577ce3962c5e22c0537bd004cf2a8d7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":34,"id":"14f51b14_48b151c7","updated":"2023-06-14 21:57:10.000000000","message":"I may need to review this for longer to really understand what is trying to be done, so I apologize if I\u0027m misreading the patch\u0027s intent.\n\nMy initial impression is that callback functions that are called in run_idl() are not the way to do this with ovsdbapp. For one, the run_idl() code is only creating the transaction that will be sent to the server. It is *not* updating what we really have in-memory. And as it runs *before* the transaction commits, running a callback seems like something we would not want to do at this point.\n\nAfter run_idl() is called and we commit() the txn, we get a notification from ovsdb-server that contains the committed data that will then be applied to our in-memory copy of the db and a response that indicates whether the transaction was successful.\n\nThe proper way to take action upon the successful commit of data to ovsdb-server is through ovsdbapp\u0027s Event handling. See ovsdb_monitor.py for examples.","commit_id":"d95e2e41d2dbda1d4c102ed814cd94227b0dcd78"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"80a53ccccab31745dff3e4d38b7c8d2934ba19e9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":34,"id":"5b0642e7_48a875cb","updated":"2023-06-14 21:35:40.000000000","message":"Nice!","commit_id":"d95e2e41d2dbda1d4c102ed814cd94227b0dcd78"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"02ec7a579a594815e7ebaf13f2d575ed4b3fa905","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":34,"id":"72fbda23_ab71ae2e","updated":"2023-06-17 03:07:24.000000000","message":"Yeah, the \"it should be upstreamed into ovsdbapp\" has kind of been the default behavior. But there are lots of parts of neutron that never used the upstream ovsdbapp implementations (like the pretty much all of the router stuff).\n\nAfter seeing the usage of ovsdbapp over the years, if I could go back in time I\u0027d just make the ovsdbapp in-tree stuff be the \"reference implementation\" that essentially just recreated the ovs-vsctl/ovn-{nb,sb}-ctl functionality (which isn\u0027t always super efficient) and encouraged applications to design things optimized for their use case.\n\nI figured people would more frequently use the generic db_set/db_get interface, but everyone kept submitting special functions that were basically just those but for specific tables. Nobody uses libraries the way you think they will. ;)\n\nThere are probably better/cleaner implementations than my PoC, but I didn\u0027t want to delay you any more (and I\u0027m not super familiar with the l3 gateway scheduling code).\n\nIf you look at that and decide that it doesn\u0027t really fit for some reason, there\u0027s tons of other \"not really the way ovsdbapp was designed to be used\" code in ml2/ovn, so it won\u0027t be the end of the world if there is more. Future backends are hypothetical at this point, and there will be plenty of other stuff to clean up if we want to use one.","commit_id":"d95e2e41d2dbda1d4c102ed814cd94227b0dcd78"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"5d60a8450400a51c99c2cf85875cb91b3aece85e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":34,"id":"dc47ee9d_46ba180c","in_reply_to":"14f51b14_48b151c7","updated":"2023-06-15 04:10:39.000000000","message":"Terry, thank you for taking the time to review, much appreciated.\n\nThe in-memory state is indeed updated as ovsdbapp interacts with the OVS Python IDL prior to the real database commit ref [0][1][2]. We of course also get the notification on the real database commit as you lay out, but for the intent of this change, that would be too late. Making use of the Event handling would also be too late.\n\nThe problem we are trying to solve here is: Let\u0027s say Neutron creates 4 ports subject to scheduling of gateway chassis on the back of a single API request.\n\nWithout this change the current code would most likely place all of these 4 ports with the highest priority on a single chassis. This is because the code will be called in an iterative fashion making lookup on stale data, only to have the OVSDB transaction commit when done at the end.\n\nThe change becomes pertinent in the implementation of anti affinity for multiple gateway ports [3] where making use of multiple transactions does not work, there is a functional test case that proves it working with the change (and it does of course not work without the change :) )\n\nThere is also a functional test added in this patch set which demonstrates how scheduling decisions are made based on updated data without using multiple ovsdbapp transactions.\n\n0: https://github.com/openvswitch/ovs/blob/e3ba0be48ca457ab3a1c9f1e3522e82218eca0f9/python/ovs/db/idl.py#L1316\n1: https://github.com/openvswitch/ovs/blob/e3ba0be48ca457ab3a1c9f1e3522e82218eca0f9/python/ovs/db/idl.py#L1400\n2: https://github.com/openvswitch/ovs/blob/1f47d73996b0c565f9ce035c899a042f2ea394a6/python/ovs/db/idl.py#L2083\n3: https://review.opendev.org/c/openstack/neutron/+/873699","commit_id":"d95e2e41d2dbda1d4c102ed814cd94227b0dcd78"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"4a32183ca365d08faf80c22f380d6f6ad8c6a38c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":34,"id":"1dad3333_c34969e0","in_reply_to":"320faf18_56b94fe9","updated":"2023-06-21 08:26:15.000000000","message":"Marking this as resolved, thank you again for the discussion! New patch set imminent.","commit_id":"d95e2e41d2dbda1d4c102ed814cd94227b0dcd78"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"ff0022038ef57a40bb25bb9b655bc221f3e0c01e","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":34,"id":"b4169fd9_ffcde7b3","in_reply_to":"58ef1c42_15fc0685","updated":"2023-06-16 04:35:41.000000000","message":"I accidentally left \"resolved\" checked for the above comment. I should also note that there is technically no rule that commands have to live in commands.py or that there can only be one \"API\" for NB/SB stuff. The idea behind ovsdbapp API classes is \"these are the methods that would have to be implemented in a new backend to support this particular API.\" This isn\u0027t necessarily a recommendation, just an observation if there was a certain amount of state inherent in some code that one didn\u0027t want to put in the main NB API. There\u0027s also nothing especially wrong with having an API method that takes things like a plugin as an argument.\n\nI apologize for the lack of good developer docs with ovsdbapp right now. It is something I\u0027m actively working on this month.","commit_id":"d95e2e41d2dbda1d4c102ed814cd94227b0dcd78"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"755f071abe892c7eaaa619d9e5fc7638c44806eb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":34,"id":"80aca04e_475768c3","in_reply_to":"98af1eca_3a92d521","updated":"2023-06-17 22:00:52.000000000","message":"\u003e Continuing re: https://paste.openstack.org/show/820387/ ... Even if you did need to pair it with something like an add_lrouter_port() in [3], putting both commands in the same transaction 100% would have them share the same data inside the transaction (things would be horribly broken if this didn\u0027t work).\n\nAs long as we can call back to the neutron code trough the appropriate methods passed in as a plugin class (as we agreed) I think it will work. We probably could even do with transaction context already set up by Neutron. As long as the decision is made on the basis of live data as it is applied in the same transaction it does not matter.\n\n\u003e If you need different scheduler behavior between add/update those can easily different commands as well--the only real difference from above is that they aren\u0027t nested functions/closures. And it has the benefit of not being \"some random method passed in\" that if another backend was implemented would mean grepping the code for every usage of add/update_lrouter_port and tracking down what methods were passed in, then coming up with a backend-appropriate implementation of those.\n\u003e \n\u003e Ultimately, I also get that it is an organizational/style issue. The function of the code will be pretty much identical. I\u0027m in the middle of trying to write \"How to Properly Use ovsdbapp\" documentation, so I\u0027m probably more prone to be picky right now. :-p I totally get that the patch as-is is the simplest way to fix the issue. It\u0027s clever, it\u0027s short, and leaves most of the code right where it was, and I love code golf. 😊\n\nWhen proposing this, I knew passing in a local function as a callback would be controversial, although I must admit I had fun discovering what hoops Python would jump through for me to have that work.\n\nProposing code to a project and having it reviewed is a conversation, and my strategy in those conversations is always to start out by changing as little as possible to accomplish the goal.\n\nSometimes that results in a wonderful small change that is accepted.\n\nOther times it results in a very good and pointed discussion on the very core of what needs to be changed. In that sense I\u0027m very pleased with the result of this proposal, because I think that is just the type of discussion we have had 😊\n\n\u003e It\u0027s really just that if there were ever another backend, those functions that are passed would probably need to be rewritten for that backend and in ovsdbapp that would mean writing a Command/api entry for them.\n\nIndeed, at the end of the day we write code for humans and future maintainability, I\u0027m 100% with you on that.","commit_id":"d95e2e41d2dbda1d4c102ed814cd94227b0dcd78"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"eb174b3445d57731c27b6c18d884648bb892a61e","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":34,"id":"320faf18_56b94fe9","in_reply_to":"b4169fd9_ffcde7b3","updated":"2023-06-16 15:36:09.000000000","message":"\u003e With that said, having a \"callback\" function argument that will only ever be one function feels a bit strange to me. Ideally, code that should be in run_idl would just be in run_idl(). Passing in a function just makes debugging a bit harder. But I also can see why you\u0027d do it this way.\n\nIn the current proposal it is already used by two different functions as the pre-existing code determines candidates for scheduling in a slightly different way for new and existing gw ports.\n\nMoreover the code in question calls into configurable/pluggable scheduler clases which brings even more facets to the problem.\n\nThe callback approach was chosen so that this whole complex in the Neutron codebase could remain as-is while solving the problem of exposing them to the live in-memory state while the transaction is being applied.\n\n\u003e The \"ovsdbapp way\" to write this would be to add a separate SetGatewayChassisCommand() and just do:\n\n    with self._nb_ovn.transaction(check_error\u003dTrue) as txn:\n        for g_name in unhosted_gateways:\n            txn.add(self._nb_ovn.update_lrouter_port(g_name)\n            txn.add(self._nb_ovn.set_gateway_chassis(g_name, values)\n\nThe above illustrates the problem, the code calling into `self._nb_ovn.set_gateway_chassis` in your example above does not control the ovsdbapp transaction context as it is already started prior to getting there. I have seen some examples of starting \"sub-transactions\" in the code base, but I have tried this and it does not work for [3].\n\nThe calling code also cannot provide \u0027values\u0027, because the values need to be determined while inside `run_idl` on the basis of what is in in-memory state.\n\n\u003e I accidentally left \"resolved\" checked for the above comment. I should also note that there is technically no rule that commands have to live in commands.py or that there can only be one \"API\" for NB/SB stuff. The idea behind ovsdbapp API classes is \"these are the methods that would have to be implemented in a new backend to support this particular API.\" This isn\u0027t necessarily a recommendation, just an observation if there was a certain amount of state inherent in some code that one didn\u0027t want to put in the main NB API. There\u0027s also nothing especially wrong with having an API method that takes things like a plugin as an argument.\n\nI guess an alternative could be to move the entire Neutron OVN gateway scheduling code complex into something that could fit in the ovsdbapp structure, but it would be a lot more work. And in the context of our change we\u0027re kind of walking into a pre-exisitng architectural issue in the code base. We should of course be good citizens and help improve it for everyone as we contribute, but I hope we can find some pragmatic compromise.\n\n\u003e I apologize for the lack of good developer docs with ovsdbapp right now. It is something I\u0027m actively working on this month.\n\nNo need to apologize, you\u0027re being very helpful by discussing the topic with me on this review.","commit_id":"d95e2e41d2dbda1d4c102ed814cd94227b0dcd78"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"02ec7a579a594815e7ebaf13f2d575ed4b3fa905","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":34,"id":"98af1eca_3a92d521","in_reply_to":"daa795b2_814964c5","updated":"2023-06-17 03:07:24.000000000","message":"Continuing re: https://paste.openstack.org/show/820387/ ... Even if you did need to pair it with something like an add_lrouter_port() in [3], putting both commands in the same transaction 100% would have them share the same data inside the transaction (things would be horribly broken if this didn\u0027t work).\n\nIf you need different scheduler behavior between add/update those can easily different commands as well--the only real difference from above is that they aren\u0027t nested functions/closures. And it has the benefit of not being \"some random method passed in\" that if another backend was implemented would mean grepping the code for every usage of add/update_lrouter_port and tracking down what methods were passed in, then coming up with a backend-appropriate implementation of those.\n\nUltimately, I also get that it is an organizational/style issue. The function of the code will be pretty much identical. I\u0027m in the middle of trying to write \"How to Properly Use ovsdbapp\" documentation, so I\u0027m probably more prone to be picky right now. :-p I totally get that the patch as-is is the simplest way to fix the issue. It\u0027s clever, it\u0027s short, and leaves most of the code right where it was, and I love code golf. 😊\n\nIt\u0027s really just that if there were ever another backend, those functions that are passed would probably need to be rewritten for that backend and in ovsdbapp that would mean writing a Command/api entry for them.","commit_id":"d95e2e41d2dbda1d4c102ed814cd94227b0dcd78"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"4bcaf608a23ec667250431fcfe264e92ea373aed","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":34,"id":"7d963ee1_f5251184","in_reply_to":"daa795b2_814964c5","updated":"2023-06-17 00:54:34.000000000","message":"Thanks, Terry. So essentially you are passing in the class to have access to all methods from `run_idl`. I guess that would work, technically it\u0027s still a callback though ;)\n\nI\u0027ll extrapolate your counter proposal to [3] and see if it works there.\n\nMy mental model of the relationship between the consumer (Neutron) and ovsdbapp was that ideally every command/API would be generic and upstreamed to ovsdbapp itself, while application specific code was to be kept in the consumer. Hence the desire to use a narrow surfaced generic callback function interface.","commit_id":"d95e2e41d2dbda1d4c102ed814cd94227b0dcd78"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"6baa8dfa7cae9cce2ef5c3009c6f80a3cf9d7ef2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":34,"id":"58ef1c42_15fc0685","in_reply_to":"dc47ee9d_46ba180c","updated":"2023-06-16 03:59:00.000000000","message":"\u003e The in-memory state is indeed updated as ovsdbapp interacts with the OVS Python IDL prior to the real database commit ref [0][1][2].\n\nWhen I say \"it is not really updating what we have in memory\" I am referring to the changes actually being stored in the in-memmory copy of the database (permanently). The changes are temporarily stored in the Row._changes field and in the table.rows while there is an active transaction, but go away after transaction commit() is called (Transaction.__disassmeble() is called). If the commit succeeds on the server, the changes are written to the actual in-memory copy of the db, but not until then.\n\nSo my initial concern was just seeing a callback method that I was afraid might rely on data that could disappear. I see \"callback\" and my brain goes to on_success/on_fail kinds of things. What this is doing is more just \"I want to run this code inside this other function\", so that particular concern wasn\u0027t really valid.\n\nWith that said, having a \"callback\" function argument that will only ever be one function feels a bit strange to me. Ideally, code that should be in run_idl would just be in run_idl(). Passing in a function just makes debugging a bit harder. But I also can see why you\u0027d do it this way.\n\nThe \"ovsdbapp way\" to write this would be to add a separate SetGatewayChassisCommand() and just do:\n\n```\nwith self._nb_ovn.transaction(check_error\u003dTrue) as txn:\n    for g_name in unhosted_gateways:\n        txn.add(self._nb_ovn.update_lrouter_port(g_name)\n        txn.add(self._nb_ovn.set_gateway_chassis(g_name, values)\n```\n\nin the one place that you need it. Both would run under the same transactional context, so changes from one would affect the other. This would also get rid of the weird _add_gateway_chassis method in commands.py and just promote it to a Command like it basically is. Ideally, *all* of the db reads that are done in schedule_unhosted_gateways() (at least from the NB DB) would be done under the transactional context.","commit_id":"d95e2e41d2dbda1d4c102ed814cd94227b0dcd78"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"5324e8ca5c3c503906caa706fc04ce383337bcec","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":36,"id":"8ea43463_50c539ff","updated":"2023-06-22 09:35:26.000000000","message":"Updated the name of the `ScheduleNewGatewayCommand` ovsdbapp command, as it became clear it is really specific to the process of scheduling unhosted gateways while working on updating https://review.opendev.org/c/openstack/neutron/+/873699.\n\nAlso added proper docstring in the API definition.","commit_id":"e71df44e8afb8074798246cdae8d484096d249d4"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"d1584319d8083ad8cb3fdaf1a7932c7264147185","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":37,"id":"9c3aaac5_f3261db2","updated":"2023-06-23 13:55:48.000000000","message":"recheck no bug spawn of instance in test timed out","commit_id":"9fa1eb8d258c3c38ed7479607ee9c47366278082"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"90be4aa4db7d6995f9a9445bc08c7422f6841a90","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":37,"id":"510be5f1_120b0c44","updated":"2023-06-23 10:09:45.000000000","message":"recheck no bug timed out","commit_id":"9fa1eb8d258c3c38ed7479607ee9c47366278082"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"61c40eb093e89f4cc0b42df37afc6199190f2995","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":37,"id":"e06b09cf_326142f6","updated":"2023-06-23 22:25:17.000000000","message":"recheck no bug timed out","commit_id":"9fa1eb8d258c3c38ed7479607ee9c47366278082"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"72c4bb6686478ac598f1a665fac2e9cc9ec7ee07","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"c5bea82f_f065fcd6","updated":"2023-06-30 15:05:02.000000000","message":"LGTM. Some nits, but nothing I\u0027d hold up merging over. Thanks for all of the hard work on this!","commit_id":"60da20bfd19ed3fbae530effcee341db67c91b5c"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"7a60e3e9167950a6f0ac87a11768814190e006aa","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"e7138e18_5320d8a2","updated":"2023-06-27 06:03:39.000000000","message":"recheck no bug failure in tox-cover job looks like a fluke?","commit_id":"60da20bfd19ed3fbae530effcee341db67c91b5c"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"d1a1b50e8571e080de47e36775877914c3eb3daf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":38,"id":"b221f851_602360ec","updated":"2023-06-27 14:43:57.000000000","message":"recheck timed out","commit_id":"60da20bfd19ed3fbae530effcee341db67c91b5c"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"6a62e4151cbfc1bafa1b2d7e57beccbe2ee32ab1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":39,"id":"618c4e3e_214887c0","updated":"2023-07-03 18:19:31.000000000","message":"Rebased to get out of merge conflict, would appreciate if previous code-review scores would be restored when convenient. Thanks!","commit_id":"ea56ed40ad920c9ade675892659bfcadbbc010f2"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"3c0fb54940ef5cc67c687da7b6897cfe8d2f52c4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":44,"id":"5a97a6c0_3f7a01a8","updated":"2023-08-23 06:27:44.000000000","message":"recheck","commit_id":"ee626039e308b00d93be2b7fdd6dcb8823ae8c8f"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"18531d99d3305a5a43f8f1bc4f24be9d2568c118","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":50,"id":"f25fac37_4b563e7f","updated":"2023-10-20 20:15:13.000000000","message":"recheck","commit_id":"da420991c4a3aac9d9e98ce52583b94421282746"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"4e9c72782a656466e8facd84281568d9960546f4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":51,"id":"38f84caf_7c31bce1","updated":"2024-01-26 15:49:37.000000000","message":"I will manually rebase this as Frode is out this week and want to move it along. A change did sneak-into this code path but shouldn\u0027t be too hard.","commit_id":"6127824bdd5d2bfbdf5c22e992a8d5f3978578dd"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"0f5b3e54d8fcaf99053f80d270d5b61c7f298a72","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":51,"id":"4450a1e0_6d49a745","updated":"2023-12-04 22:41:34.000000000","message":"Just one nit on a docstring, looks good otherwise","commit_id":"6127824bdd5d2bfbdf5c22e992a8d5f3978578dd"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"830853bfde0267838fff0570e4db58781e0c7c81","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":52,"id":"36a38c24_afa3ce54","updated":"2024-01-27 14:13:53.000000000","message":"Thanks a lot, Brian for the attempt while I was away, and thanks everyone for reviews and merges of parent changes. Much appreciated!\n\nI\u0027ll pick this up, as well as any outstanding work for the series and related patches up once I return Tuesday","commit_id":"0aa9bfd3b3919c203c4918380159a4d9aa5c13f1"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"a5cfc194d88a55c4c435fdd6eb16b3106a0ab646","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":53,"id":"f4148f1f_74350c79","updated":"2024-01-30 20:07:48.000000000","message":"I made another attempt Frode, not sure how palatable it is.","commit_id":"339a602114e0f79361f336b5a564d0172a5052d0"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"af5878361dbd524646ce963fe3969f9838a1edde","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":54,"id":"5662656b_03a835a6","updated":"2024-01-31 14:23:31.000000000","message":"Thanks, Brian! Looks good, I\u0027ll use this as a base to rebase the others as well. Currently working on a unit test for `ScheduleUnhostedGatewaysCommand` which incorporates Felixes test addition in I786ff6c0c4d3403b79819df95f9b1d6ac5e8675f as it kind of belongs elsewhere too now.","commit_id":"f932bd283596d370a6364ed3a4d30ed51485a1d8"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"0fede9e5d59febcb4dc72a21b0c9f6fce60ca267","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":55,"id":"6accc991_91c20a23","updated":"2024-02-07 22:14:13.000000000","message":"Other than my nit I\u0027m fine with this, will only +1 since I did some of the rebase to master.","commit_id":"f4fa8c2cf8372e92a2edbd05b60ccae0ebc31dd7"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"aa191f567e9369038c9ad8bb25e1526780f08542","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":59,"id":"9b1a83e3_8af1166b","updated":"2024-02-27 15:35:20.000000000","message":"Good patch, nothing new since the last PS, just a rebase.","commit_id":"0bae4b70b6fc53cf5502622677a95ee50cc319a1"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"0f5b3e54d8fcaf99053f80d270d5b61c7f298a72","unresolved":true,"context_lines":[{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    @abc.abstractmethod"},{"line_number":154,"context_line":"    def schedule_unhosted_gateways(self, g_name, plugin, port_physnets,"},{"line_number":155,"context_line":"                                   all_gw_chassis, chassis_with_physnets,"},{"line_number":156,"context_line":"                                   chassis_with_azs):"},{"line_number":157,"context_line":"        \"\"\"Schedule unhosted gateways"},{"line_number":158,"context_line":""}],"source_content_type":"text/x-python","patch_set":51,"id":"88653bb7_b0498f36","line":155,"range":{"start_line":155,"start_character":51,"end_line":155,"end_character":72},"updated":"2023-12-04 22:41:34.000000000","message":"This is missing from docstring below","commit_id":"6127824bdd5d2bfbdf5c22e992a8d5f3978578dd"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"2c1f5b568dd6a72684ffbc460f23f7c6151e8e5a","unresolved":true,"context_lines":[{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    @abc.abstractmethod"},{"line_number":154,"context_line":"    def schedule_unhosted_gateways(self, g_name, plugin, port_physnets,"},{"line_number":155,"context_line":"                                   all_gw_chassis, chassis_with_physnets,"},{"line_number":156,"context_line":"                                   chassis_with_azs):"},{"line_number":157,"context_line":"        \"\"\"Schedule unhosted gateways"},{"line_number":158,"context_line":""}],"source_content_type":"text/x-python","patch_set":51,"id":"aa4d10e3_23bca7dd","line":155,"range":{"start_line":155,"start_character":51,"end_line":155,"end_character":72},"in_reply_to":"88653bb7_b0498f36","updated":"2023-12-08 23:00:31.000000000","message":"It should be chassis_with_azs","commit_id":"6127824bdd5d2bfbdf5c22e992a8d5f3978578dd"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"4e9c72782a656466e8facd84281568d9960546f4","unresolved":false,"context_lines":[{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    @abc.abstractmethod"},{"line_number":154,"context_line":"    def schedule_unhosted_gateways(self, g_name, plugin, port_physnets,"},{"line_number":155,"context_line":"                                   all_gw_chassis, chassis_with_physnets,"},{"line_number":156,"context_line":"                                   chassis_with_azs):"},{"line_number":157,"context_line":"        \"\"\"Schedule unhosted gateways"},{"line_number":158,"context_line":""}],"source_content_type":"text/x-python","patch_set":51,"id":"cf1206d2_9525720f","line":155,"range":{"start_line":155,"start_character":51,"end_line":155,"end_character":72},"in_reply_to":"aa4d10e3_23bca7dd","updated":"2024-01-26 15:49:37.000000000","message":"Done","commit_id":"6127824bdd5d2bfbdf5c22e992a8d5f3978578dd"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"0fede9e5d59febcb4dc72a21b0c9f6fce60ca267","unresolved":true,"context_lines":[{"line_number":157,"context_line":"        \"\"\"Schedule unhosted gateways"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"        :param sb_api:                IDL for the OVN SB API"},{"line_number":160,"context_line":"        :type class:                  :class:`OvsdbSbOvnIdl`"},{"line_number":161,"context_line":"        :param g_name:                The unique name of the lrouter port"},{"line_number":162,"context_line":"        :type g_name:                 string"},{"line_number":163,"context_line":"        :param plugin:                The L3 plugin to call back to for lookups"}],"source_content_type":"text/x-python","patch_set":55,"id":"be8e33cf_11da69c0","line":160,"updated":"2024-02-07 22:14:13.000000000","message":"My only nit here (and I did this) is the order of the arguments here, most others have the name first.","commit_id":"f4fa8c2cf8372e92a2edbd05b60ccae0ebc31dd7"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"f24cea6650b930267358c90f7f9b5977f3bf158c","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        \"\"\"Schedule unhosted gateways"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"        :param sb_api:                IDL for the OVN SB API"},{"line_number":160,"context_line":"        :type class:                  :class:`OvsdbSbOvnIdl`"},{"line_number":161,"context_line":"        :param g_name:                The unique name of the lrouter port"},{"line_number":162,"context_line":"        :type g_name:                 string"},{"line_number":163,"context_line":"        :param plugin:                The L3 plugin to call back to for lookups"}],"source_content_type":"text/x-python","patch_set":55,"id":"0ab62c61_9fdf35d1","line":160,"in_reply_to":"29917cc8_61de6250","updated":"2024-02-16 16:33:36.000000000","message":"Done","commit_id":"f4fa8c2cf8372e92a2edbd05b60ccae0ebc31dd7"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"eaf92ba96390380b2258a9229bc19b3ca23b6ebb","unresolved":true,"context_lines":[{"line_number":157,"context_line":"        \"\"\"Schedule unhosted gateways"},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"        :param sb_api:                IDL for the OVN SB API"},{"line_number":160,"context_line":"        :type class:                  :class:`OvsdbSbOvnIdl`"},{"line_number":161,"context_line":"        :param g_name:                The unique name of the lrouter port"},{"line_number":162,"context_line":"        :type g_name:                 string"},{"line_number":163,"context_line":"        :param plugin:                The L3 plugin to call back to for lookups"}],"source_content_type":"text/x-python","patch_set":55,"id":"29917cc8_61de6250","line":160,"in_reply_to":"be8e33cf_11da69c0","updated":"2024-02-16 08:11:25.000000000","message":"Oh, missed this one, on it.","commit_id":"f4fa8c2cf8372e92a2edbd05b60ccae0ebc31dd7"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"9f14c566e6a99ea6f4bb767d9377cb8df4ff0abe","unresolved":true,"context_lines":[{"line_number":358,"context_line":"        self.columns \u003d columns"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"    def run_idl(self, txn):"},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"        try:"},{"line_number":363,"context_line":"            lrouter \u003d idlutils.row_by_value(self.api.idl, \u0027Logical_Router\u0027,"},{"line_number":364,"context_line":"                                            \u0027name\u0027, self.lrouter)"}],"source_content_type":"text/x-python","patch_set":34,"id":"f7f04c3a_0e0df515","side":"PARENT","line":361,"updated":"2023-06-12 19:26:57.000000000","message":"nit: unrelated whitespace change","commit_id":"2c708eed969d3ec14073dabb42d4fee881110a5d"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"4a32183ca365d08faf80c22f380d6f6ad8c6a38c","unresolved":false,"context_lines":[{"line_number":358,"context_line":"        self.columns \u003d columns"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"    def run_idl(self, txn):"},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"        try:"},{"line_number":363,"context_line":"            lrouter \u003d idlutils.row_by_value(self.api.idl, \u0027Logical_Router\u0027,"},{"line_number":364,"context_line":"                                            \u0027name\u0027, self.lrouter)"}],"source_content_type":"text/x-python","patch_set":34,"id":"2216718b_22e7a275","side":"PARENT","line":361,"in_reply_to":"f7f04c3a_0e0df515","updated":"2023-06-21 08:26:15.000000000","message":"Ack","commit_id":"2c708eed969d3ec14073dabb42d4fee881110a5d"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"346d361e799f564c39aeb91e2cccaf2e855be1ed","unresolved":true,"context_lines":[{"line_number":364,"context_line":"    def __init__(self, nb_api, sb_api, g_name, plugin, port_physnets,"},{"line_number":365,"context_line":"                 all_gw_chassis, chassis_with_physnets, chassis_with_azs):"},{"line_number":366,"context_line":"        super().__init__(api\u003dnb_api)"},{"line_number":367,"context_line":"        # Feels like a hack, but could not think of a better way"},{"line_number":368,"context_line":"        self.sb_api \u003d sb_api"},{"line_number":369,"context_line":"        self.g_name \u003d g_name"},{"line_number":370,"context_line":"        self.scheduler \u003d plugin.scheduler"}],"source_content_type":"text/x-python","patch_set":54,"id":"eff3e155_927e63e2","line":367,"updated":"2024-01-31 15:06:19.000000000","message":"Should we leave this comment? I guess we can wait for reviews.","commit_id":"f932bd283596d370a6364ed3a4d30ed51485a1d8"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"0e9248208a3a6392c88ca5f5b7c322bfc51f775a","unresolved":false,"context_lines":[{"line_number":364,"context_line":"    def __init__(self, nb_api, sb_api, g_name, plugin, port_physnets,"},{"line_number":365,"context_line":"                 all_gw_chassis, chassis_with_physnets, chassis_with_azs):"},{"line_number":366,"context_line":"        super().__init__(api\u003dnb_api)"},{"line_number":367,"context_line":"        # Feels like a hack, but could not think of a better way"},{"line_number":368,"context_line":"        self.sb_api \u003d sb_api"},{"line_number":369,"context_line":"        self.g_name \u003d g_name"},{"line_number":370,"context_line":"        self.scheduler \u003d plugin.scheduler"}],"source_content_type":"text/x-python","patch_set":54,"id":"b2b8d517_62a8eaf0","line":367,"in_reply_to":"eff3e155_927e63e2","updated":"2024-01-31 18:49:49.000000000","message":"Done","commit_id":"f932bd283596d370a6364ed3a4d30ed51485a1d8"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py":[{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"34a8d24cd951cbd46ecafaa6ad1ab48ce4a6399b","unresolved":true,"context_lines":[{"line_number":1613,"context_line":"        columns \u003d {}"},{"line_number":1614,"context_line":"        columns[\u0027options\u0027] \u003d self._gen_router_port_options(port)"},{"line_number":1615,"context_line":""},{"line_number":1616,"context_line":"        candidates \u003d []"},{"line_number":1617,"context_line":"        if is_gw_port:"},{"line_number":1618,"context_line":"            port_net \u003d self._plugin.get_network(n_context.get_admin_context(),"},{"line_number":1619,"context_line":"                                                port[\u0027network_id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"1c464c5d_b2e0e7ce","line":1616,"updated":"2023-02-22 10:58:50.000000000","message":"This is an unnecessary change for this patch set, will remove.","commit_id":"30d14a3b42d4a0b74994f560c2d333aed5eb685c"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"e90ac861d46c4f82b94222d254046e3c0fa55d07","unresolved":false,"context_lines":[{"line_number":1613,"context_line":"        columns \u003d {}"},{"line_number":1614,"context_line":"        columns[\u0027options\u0027] \u003d self._gen_router_port_options(port)"},{"line_number":1615,"context_line":""},{"line_number":1616,"context_line":"        candidates \u003d []"},{"line_number":1617,"context_line":"        if is_gw_port:"},{"line_number":1618,"context_line":"            port_net \u003d self._plugin.get_network(n_context.get_admin_context(),"},{"line_number":1619,"context_line":"                                                port[\u0027network_id\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"77708eb3_9873cb01","line":1616,"in_reply_to":"1c464c5d_b2e0e7ce","updated":"2023-02-22 15:01:50.000000000","message":"Done","commit_id":"30d14a3b42d4a0b74994f560c2d333aed5eb685c"}],"neutron/services/ovn_l3/plugin.py":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"72c4bb6686478ac598f1a665fac2e9cc9ec7ee07","unresolved":true,"context_lines":[{"line_number":422,"context_line":""},{"line_number":423,"context_line":"        with self._nb_ovn.transaction(check_error\u003dTrue) as txn:"},{"line_number":424,"context_line":"            for g_name in unhosted_gateways:"},{"line_number":425,"context_line":"                # NOTE(fnordahl): Use callback for scheduling decissions so"},{"line_number":426,"context_line":"                # that scheduling is done based on up to date information as"},{"line_number":427,"context_line":"                # the transaction is applied."},{"line_number":428,"context_line":"                txn.add(self._nb_ovn.schedule_unhosted_gateways("}],"source_content_type":"text/x-python","patch_set":38,"id":"d88f88c6_5b803770","line":425,"range":{"start_line":425,"start_character":38,"end_line":425,"end_character":46},"updated":"2023-06-30 15:05:02.000000000","message":"nit, technically we don\u0027t have a callback anymore.","commit_id":"60da20bfd19ed3fbae530effcee341db67c91b5c"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"a3c7ee061242563357c824be04d328af8dc728d4","unresolved":false,"context_lines":[{"line_number":422,"context_line":""},{"line_number":423,"context_line":"        with self._nb_ovn.transaction(check_error\u003dTrue) as txn:"},{"line_number":424,"context_line":"            for g_name in unhosted_gateways:"},{"line_number":425,"context_line":"                # NOTE(fnordahl): Use callback for scheduling decissions so"},{"line_number":426,"context_line":"                # that scheduling is done based on up to date information as"},{"line_number":427,"context_line":"                # the transaction is applied."},{"line_number":428,"context_line":"                txn.add(self._nb_ovn.schedule_unhosted_gateways("}],"source_content_type":"text/x-python","patch_set":38,"id":"cb0b8907_7c9f02b6","line":425,"range":{"start_line":425,"start_character":38,"end_line":425,"end_character":46},"in_reply_to":"d88f88c6_5b803770","updated":"2023-07-03 14:11:45.000000000","message":"Ack, need to rebase out of merge conflict so might as well address this.","commit_id":"60da20bfd19ed3fbae530effcee341db67c91b5c"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"4e9c72782a656466e8facd84281568d9960546f4","unresolved":true,"context_lines":[{"line_number":454,"context_line":"            # as we will rely on those for scheduling subsequent gateways."},{"line_number":455,"context_line":"            with self._nb_ovn.transaction(check_error\u003dTrue) as txn:"},{"line_number":456,"context_line":"                txn.add(self._nb_ovn.update_lrouter_port("},{"line_number":457,"context_line":"                    g_name, gateway_chassis\u003dchassis))"},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"    @staticmethod"},{"line_number":460,"context_line":"    @registry.receives(resources.SUBNET, [events.AFTER_UPDATE])"}],"source_content_type":"text/x-python","patch_set":51,"id":"8a9311f6_ebadc3c6","side":"PARENT","line":457,"updated":"2024-01-26 15:49:37.000000000","message":"Some of this above code has since changed, I\u0027ll manually rebase and update based on current master.","commit_id":"b1a6294c79ccd75007b108f543679f75dfd76311"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"a9e60e09f5fbcb7884b9c24bc733d56f098235d3","unresolved":false,"context_lines":[{"line_number":454,"context_line":"            # as we will rely on those for scheduling subsequent gateways."},{"line_number":455,"context_line":"            with self._nb_ovn.transaction(check_error\u003dTrue) as txn:"},{"line_number":456,"context_line":"                txn.add(self._nb_ovn.update_lrouter_port("},{"line_number":457,"context_line":"                    g_name, gateway_chassis\u003dchassis))"},{"line_number":458,"context_line":""},{"line_number":459,"context_line":"    @staticmethod"},{"line_number":460,"context_line":"    @registry.receives(resources.SUBNET, [events.AFTER_UPDATE])"}],"source_content_type":"text/x-python","patch_set":51,"id":"afcb85df_c6c2c3ab","side":"PARENT","line":457,"in_reply_to":"8a9311f6_ebadc3c6","updated":"2024-01-26 15:50:11.000000000","message":"Done","commit_id":"b1a6294c79ccd75007b108f543679f75dfd76311"}],"neutron/tests/functional/services/ovn_l3/test_plugin.py":[{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"72c4bb6686478ac598f1a665fac2e9cc9ec7ee07","unresolved":true,"context_lines":[{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    def _get_gwc_dict(self):"},{"line_number":95,"context_line":"        sched_info \u003d {}"},{"line_number":96,"context_line":"        for row in self.nb_api.tables["},{"line_number":97,"context_line":"                \u0027Logical_Router_Port\u0027].rows.values():"},{"line_number":98,"context_line":"            for gwc in row.gateway_chassis:"},{"line_number":99,"context_line":"                chassis \u003d sched_info.setdefault(gwc.chassis_name, {})"}],"source_content_type":"text/x-python","patch_set":38,"id":"d617d004_37a9c0a0","line":96,"updated":"2023-06-30 15:05:02.000000000","message":"We do this kind of thing a lot in neutron, so this is really just a nit and a \"for future reference\" kind of comment.\n\nTechnically, from an ovsdbapp perspective, no user code should ever use backend-specific code like `.tables`. All access should be through API methods, e.g. `for row in self.nb_api.db_list(\"Logical_Router_Port\").execute(check_error\u003dTrue):`. Eventually I\u0027m going to go through all of the code and fix these kinds of things, so it\u0027s not a big deal that it is here. I\u0027ve written plenty of code in neutron that uses it too because it is handy and there\u0027s only currently a single backend anyway.","commit_id":"60da20bfd19ed3fbae530effcee341db67c91b5c"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"4a2a0c6028847898dcbbd1894edffde097c8bab9","unresolved":true,"context_lines":[{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    def _get_gwc_dict(self):"},{"line_number":95,"context_line":"        sched_info \u003d {}"},{"line_number":96,"context_line":"        for row in self.nb_api.tables["},{"line_number":97,"context_line":"                \u0027Logical_Router_Port\u0027].rows.values():"},{"line_number":98,"context_line":"            for gwc in row.gateway_chassis:"},{"line_number":99,"context_line":"                chassis \u003d sched_info.setdefault(gwc.chassis_name, {})"}],"source_content_type":"text/x-python","patch_set":38,"id":"fd81150d_2bc6eb82","line":96,"in_reply_to":"d617d004_37a9c0a0","updated":"2023-07-04 13:25:49.000000000","message":"+1","commit_id":"60da20bfd19ed3fbae530effcee341db67c91b5c"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"75070a65e2e956a65be5f5d43821c9080beda721","unresolved":false,"context_lines":[{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    def _get_gwc_dict(self):"},{"line_number":95,"context_line":"        sched_info \u003d {}"},{"line_number":96,"context_line":"        for row in self.nb_api.tables["},{"line_number":97,"context_line":"                \u0027Logical_Router_Port\u0027].rows.values():"},{"line_number":98,"context_line":"            for gwc in row.gateway_chassis:"},{"line_number":99,"context_line":"                chassis \u003d sched_info.setdefault(gwc.chassis_name, {})"}],"source_content_type":"text/x-python","patch_set":38,"id":"8aba4b03_769402b9","line":96,"in_reply_to":"fd81150d_2bc6eb82","updated":"2023-07-06 12:13:58.000000000","message":"Done","commit_id":"60da20bfd19ed3fbae530effcee341db67c91b5c"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"72c4bb6686478ac598f1a665fac2e9cc9ec7ee07","unresolved":true,"context_lines":[{"line_number":129,"context_line":"            chassis_added.append("},{"line_number":130,"context_line":"                self.add_fake_chassis("},{"line_number":131,"context_line":"                    \u0027ovs-host%s\u0027 % i,"},{"line_number":132,"context_line":"                    name\u003d\u0027chassis-%s\u0027 % (f\u00270{i}\u0027 if i \u003c 10 else i),"},{"line_number":133,"context_line":"                    physical_nets\u003dphysical_nets,"},{"line_number":134,"context_line":"                    other_config\u003d{"},{"line_number":135,"context_line":"                        \u0027ovn-cms-options\u0027: \u0027enable-chassis-as-gw\u0027}))"}],"source_content_type":"text/x-python","patch_set":38,"id":"01858b90_064b18cc","line":132,"range":{"start_line":132,"start_character":20,"end_line":132,"end_character":67},"updated":"2023-06-30 15:05:02.000000000","message":"nit: This could just be `name\u003df\"chassis-{i:02d}\"`","commit_id":"60da20bfd19ed3fbae530effcee341db67c91b5c"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"a3c7ee061242563357c824be04d328af8dc728d4","unresolved":false,"context_lines":[{"line_number":129,"context_line":"            chassis_added.append("},{"line_number":130,"context_line":"                self.add_fake_chassis("},{"line_number":131,"context_line":"                    \u0027ovs-host%s\u0027 % i,"},{"line_number":132,"context_line":"                    name\u003d\u0027chassis-%s\u0027 % (f\u00270{i}\u0027 if i \u003c 10 else i),"},{"line_number":133,"context_line":"                    physical_nets\u003dphysical_nets,"},{"line_number":134,"context_line":"                    other_config\u003d{"},{"line_number":135,"context_line":"                        \u0027ovn-cms-options\u0027: \u0027enable-chassis-as-gw\u0027}))"}],"source_content_type":"text/x-python","patch_set":38,"id":"40cfbfee_7233d09b","line":132,"range":{"start_line":132,"start_character":20,"end_line":132,"end_character":67},"in_reply_to":"01858b90_064b18cc","updated":"2023-07-03 14:11:45.000000000","message":"Indeed, thx!","commit_id":"60da20bfd19ed3fbae530effcee341db67c91b5c"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"4a2a0c6028847898dcbbd1894edffde097c8bab9","unresolved":true,"context_lines":[{"line_number":686,"context_line":"            self.assertEqual(ovn_const.MAX_GW_CHASSIS,"},{"line_number":687,"context_line":"                             len(row.gateway_chassis))"},{"line_number":688,"context_line":""},{"line_number":689,"context_line":"    def test_schedule_unhosted_gateways_single_transaction(self):"},{"line_number":690,"context_line":"        ext1 \u003d self._create_ext_network("},{"line_number":691,"context_line":"            \u0027ext1\u0027, \u0027flat\u0027, \u0027physnet6\u0027, None, \"60.0.0.1\", \"60.0.0.0/24\")"},{"line_number":692,"context_line":"        gw_info \u003d {\u0027network_id\u0027: ext1[\u0027network\u0027][\u0027id\u0027]}"}],"source_content_type":"text/x-python","patch_set":39,"id":"8409f559_81b41777","line":689,"range":{"start_line":689,"start_character":8,"end_line":689,"end_character":58},"updated":"2023-07-04 13:25:49.000000000","message":"The main goal of this optimization is to create a proper LRP.gateway_chassis distribution. If that is the case, at the end of this test, you should list all LRP, their \"gateway_chassis\" list and the chassis associated, and check that this distribution is proportional.","commit_id":"ea56ed40ad920c9ade675892659bfcadbbc010f2"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"75070a65e2e956a65be5f5d43821c9080beda721","unresolved":false,"context_lines":[{"line_number":686,"context_line":"            self.assertEqual(ovn_const.MAX_GW_CHASSIS,"},{"line_number":687,"context_line":"                             len(row.gateway_chassis))"},{"line_number":688,"context_line":""},{"line_number":689,"context_line":"    def test_schedule_unhosted_gateways_single_transaction(self):"},{"line_number":690,"context_line":"        ext1 \u003d self._create_ext_network("},{"line_number":691,"context_line":"            \u0027ext1\u0027, \u0027flat\u0027, \u0027physnet6\u0027, None, \"60.0.0.1\", \"60.0.0.0/24\")"},{"line_number":692,"context_line":"        gw_info \u003d {\u0027network_id\u0027: ext1[\u0027network\u0027][\u0027id\u0027]}"}],"source_content_type":"text/x-python","patch_set":39,"id":"7d619bfb_2549ae5f","line":689,"range":{"start_line":689,"start_character":8,"end_line":689,"end_character":58},"in_reply_to":"8409f559_81b41777","updated":"2023-07-06 12:13:58.000000000","message":"The current test also catches the situation, before this change all gateways would be scheduled on the same chassis.\n\nBut I see what you mean. Because of the code that avoids moving primary gateway I\u0027ll rework it so that we\u0027ll be able to consistently check the distribution.","commit_id":"ea56ed40ad920c9ade675892659bfcadbbc010f2"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"4a2a0c6028847898dcbbd1894edffde097c8bab9","unresolved":true,"context_lines":[{"line_number":726,"context_line":"        #"},{"line_number":727,"context_line":"        # Note that due to the primary chassis affinity code, all gateways will"},{"line_number":728,"context_line":"        # stay on the initial chassis for the highest priority. Ref:"},{"line_number":729,"context_line":"        # https://github.com/openstack/neutron/blob/c178c28fb8f21c1ea145f696cf724aa9038012b2/neutron/services/ovn_l3/plugin.py#L439-L454"},{"line_number":730,"context_line":"        for chassis in chassis_list[1:]:"},{"line_number":731,"context_line":"            self.assertNotEqual("},{"line_number":732,"context_line":"                list(self._get_gwc_dict()[chassis].values()),"}],"source_content_type":"text/x-python","patch_set":39,"id":"187557d3_50c1b7a2","line":729,"range":{"start_line":729,"start_character":10,"end_line":729,"end_character":136},"updated":"2023-07-04 13:25:49.000000000","message":"nit: please trunk this line","commit_id":"ea56ed40ad920c9ade675892659bfcadbbc010f2"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"75070a65e2e956a65be5f5d43821c9080beda721","unresolved":false,"context_lines":[{"line_number":726,"context_line":"        #"},{"line_number":727,"context_line":"        # Note that due to the primary chassis affinity code, all gateways will"},{"line_number":728,"context_line":"        # stay on the initial chassis for the highest priority. Ref:"},{"line_number":729,"context_line":"        # https://github.com/openstack/neutron/blob/c178c28fb8f21c1ea145f696cf724aa9038012b2/neutron/services/ovn_l3/plugin.py#L439-L454"},{"line_number":730,"context_line":"        for chassis in chassis_list[1:]:"},{"line_number":731,"context_line":"            self.assertNotEqual("},{"line_number":732,"context_line":"                list(self._get_gwc_dict()[chassis].values()),"}],"source_content_type":"text/x-python","patch_set":39,"id":"6ea94c44_f4b9b112","line":729,"range":{"start_line":729,"start_character":10,"end_line":729,"end_character":136},"in_reply_to":"187557d3_50c1b7a2","updated":"2023-07-06 12:13:58.000000000","message":"It would break the link and make it less readable, which as stated in PEP8 would be a reason to be inconsistent with the line length rule, especially since this is a comment.\n\nAfter the rework of the test, the comment is no longer needed, so removed it.","commit_id":"ea56ed40ad920c9ade675892659bfcadbbc010f2"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"c7d9e7bcaab979955014d06bf9df1f43685d0d70","unresolved":true,"context_lines":[{"line_number":677,"context_line":""},{"line_number":678,"context_line":"    def test_schedule_unhosted_gateways_single_transaction(self):"},{"line_number":679,"context_line":"        ext1 \u003d self._create_ext_network("},{"line_number":680,"context_line":"            \u0027ext1\u0027, \u0027flat\u0027, \u0027physnet6\u0027, None, \"60.0.0.1\", \"60.0.0.0/24\")"},{"line_number":681,"context_line":"        gw_info \u003d {\u0027network_id\u0027: ext1[\u0027network\u0027][\u0027id\u0027]}"},{"line_number":682,"context_line":""},{"line_number":683,"context_line":"        # Attempt to add 4 routers, since there are no chassis all will be"}],"source_content_type":"text/x-python","patch_set":43,"id":"12570689_f697dab0","line":680,"updated":"2023-08-10 02:10:11.000000000","message":"nit: probably better to use something other than 60.* for IPs","commit_id":"718367fa60fd0f1a2ac098e56899041fc698e4a6"},{"author":{"_account_id":13686,"name":"Frode Nordahl","email":"fnordahl@ubuntu.com","username":"fnordahl"},"change_message_id":"9af0ab623c58a395a80fdee83815769ab76ab580","unresolved":false,"context_lines":[{"line_number":677,"context_line":""},{"line_number":678,"context_line":"    def test_schedule_unhosted_gateways_single_transaction(self):"},{"line_number":679,"context_line":"        ext1 \u003d self._create_ext_network("},{"line_number":680,"context_line":"            \u0027ext1\u0027, \u0027flat\u0027, \u0027physnet6\u0027, None, \"60.0.0.1\", \"60.0.0.0/24\")"},{"line_number":681,"context_line":"        gw_info \u003d {\u0027network_id\u0027: ext1[\u0027network\u0027][\u0027id\u0027]}"},{"line_number":682,"context_line":""},{"line_number":683,"context_line":"        # Attempt to add 4 routers, since there are no chassis all will be"}],"source_content_type":"text/x-python","patch_set":43,"id":"85d70e7c_4b97d600","line":680,"in_reply_to":"12570689_f697dab0","updated":"2023-08-22 12:27:21.000000000","message":"I do not disagree, and I just continued the tradition presented in this file.\n\nUpdated this and the other changes in this series so that they use valid RFC 1918 addresses (I guess we could consider using RFC 5737 addresses to be even more diligent).","commit_id":"718367fa60fd0f1a2ac098e56899041fc698e4a6"}],"neutron/tests/unit/services/ovn_l3/test_plugin.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"4e9c72782a656466e8facd84281568d9960546f4","unresolved":true,"context_lines":[{"line_number":1660,"context_line":"                       \u0027chassis2\u0027: [\u0027physnet1\u0027],"},{"line_number":1661,"context_line":"                       \u0027chassis3\u0027: [\u0027physnet1\u0027]},"},{"line_number":1662,"context_line":"                      {}),"},{"line_number":1663,"context_line":"        ])"},{"line_number":1664,"context_line":""},{"line_number":1665,"context_line":"    @mock.patch(\u0027neutron.services.ovn_l3.plugin.OVNL3RouterPlugin.\u0027"},{"line_number":1666,"context_line":"                \u0027_get_gateway_port_physnet_mapping\u0027)"}],"source_content_type":"text/x-python","patch_set":51,"id":"828fe73a_2fe3d026","line":1663,"updated":"2024-01-26 15:49:37.000000000","message":"Similar change has to happen in test_schedule_unhosted_gateways_rebalances_lower_prios() which is new in master","commit_id":"6127824bdd5d2bfbdf5c22e992a8d5f3978578dd"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"a9e60e09f5fbcb7884b9c24bc733d56f098235d3","unresolved":false,"context_lines":[{"line_number":1660,"context_line":"                       \u0027chassis2\u0027: [\u0027physnet1\u0027],"},{"line_number":1661,"context_line":"                       \u0027chassis3\u0027: [\u0027physnet1\u0027]},"},{"line_number":1662,"context_line":"                      {}),"},{"line_number":1663,"context_line":"        ])"},{"line_number":1664,"context_line":""},{"line_number":1665,"context_line":"    @mock.patch(\u0027neutron.services.ovn_l3.plugin.OVNL3RouterPlugin.\u0027"},{"line_number":1666,"context_line":"                \u0027_get_gateway_port_physnet_mapping\u0027)"}],"source_content_type":"text/x-python","patch_set":51,"id":"ec6b8143_08909890","line":1663,"in_reply_to":"828fe73a_2fe3d026","updated":"2024-01-26 15:50:11.000000000","message":"Done","commit_id":"6127824bdd5d2bfbdf5c22e992a8d5f3978578dd"}]}