)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"f7dd077f3dbbfe501d667f7afebad43a856c8c9d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"77a70c79_2848b372","updated":"2026-04-28 16:31:10.000000000","message":"I\u0027m fine with the change, did have one comment.","commit_id":"74262448bf252628cda58afb24cb2994189f41a5"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"65f7187e24a529927114fc68d2ac1d7138eb5b48","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"211e43bc_e183ed0f","updated":"2026-04-29 14:07:27.000000000","message":"Soft -1, thanks for the update.\n\nSo I think at least two of the others I mentioned can be changed as well?\n\ncheck_network_ports_by_binding_types() has this:\n    return bool(query.count())\n\nSeems it can be:\n    return query.first() is not None\n\n\nAlso validate_rbac_policy_create() has this code:\n\n            matching_policies \u003d model_query.query_with_hooks(\n                context, rbac_db_models.AddressScopeRBAC\n            ).filter(shared_to_target_project_or_to_all).count()\n\n        if matching_policies \u003d\u003d 0:\n            raise ext_rbac.RbacPolicyInitError(...\n\nSeems that could use first() as well, just needs a slight change to the unit test to pass:\n\n-            return_value\u003dmock.Mock(count\u003dmock.Mock(return_value\u003d0))\n+            return_value\u003dmock.Mock(first\u003dmock.Mock(return_value\u003dNone))\n\nOf course I could be wrong.","commit_id":"395e69ace71d88e7c34407ac24c7c0fcb842d34b"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"cde704957e31cdcc87aa9865cccb709f8068259e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"52f4d477_574a4d27","in_reply_to":"211e43bc_e183ed0f","updated":"2026-05-06 11:17:20.000000000","message":"Done","commit_id":"395e69ace71d88e7c34407ac24c7c0fcb842d34b"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"5d98a4eb9ded64b90ea34fdf1ed9aae98612220b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"3a32dba6_31d5acc0","updated":"2026-05-07 16:37:48.000000000","message":"some nits, I can +2 if you think they are not worth the respin","commit_id":"2072dce7349b0f39e6702600e8d2faa6cb468519"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"92c5b8b7573cddf8ddc1104799ae4bccd4357446","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"0ae2a48e_bdca5ab5","updated":"2026-05-08 08:00:29.000000000","message":"just a question to consider, do you think it could be beneficial to somehow add check for that in the hacking rules?","commit_id":"be5d6f44d4ecdb6c432c7b303fc8508f0be9464d"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"d3d1b8f04932c819ca54630143cf2ac267b40661","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"ca815c27_a9974617","updated":"2026-05-10 13:03:18.000000000","message":"recheck neutron-ovn-grenade-multinode","commit_id":"be5d6f44d4ecdb6c432c7b303fc8508f0be9464d"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"0029700557af4ede11bdeeffada1ffeb1547d642","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"b337d808_9141ae19","updated":"2026-05-08 15:20:25.000000000","message":"recheck zuul error","commit_id":"be5d6f44d4ecdb6c432c7b303fc8508f0be9464d"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"b738ca5ffb50f4d7fdc2a39e223070dc82538346","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"45520f49_3cf8a194","in_reply_to":"0ae2a48e_bdca5ab5","updated":"2026-05-08 09:38:47.000000000","message":"The point is that `query.count()` is a valid operation in some places. We can\u0027t prevent from using it. The problem here is the necessity of having the number of registers or just if there is at least one. This kind of logic is more complex and not suitable (I think) for the hacking rules.","commit_id":"be5d6f44d4ecdb6c432c7b303fc8508f0be9464d"}],"neutron/db/db_base_plugin_v2.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"f7dd077f3dbbfe501d667f7afebad43a856c8c9d","unresolved":true,"context_lines":[{"line_number":321,"context_line":"                        target_project\u003d\u0027*\u0027):"},{"line_number":322,"context_line":"                    return"},{"line_number":323,"context_line":"                ports \u003d ports.filter(models_v2.Port.project_id \u003d\u003d project_id)"},{"line_number":324,"context_line":"            if ports.first() is not None:"},{"line_number":325,"context_line":"                raise exc.InvalidSharedSetting(network\u003dnetwork_id)"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"    def set_ipam_backend(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"b318f940_360fbfe9","line":324,"updated":"2026-04-28 16:31:10.000000000","message":"So I took a look and also see this being used (from subnet.py):\n\nif not query.limit(1).count()\n\nShould we settle on one way to do this?\n\nThese other places seem to do similar things using count():\n\nrabc_db.py: _validate_rbac_policy_delete()\nnetwork_segment_range.py: new_default()\nnetwork.py: count_segments() uses it, but only caller - network_segments_exist_in_range() returns bool(count_segments()) so doesn\u0027t care about actual count\nports.py: check_network_ports_by_binding_types()\nrouter.py: check_routers_not_owned_by_projects()\nsubnetpool.py: validate_rbac_policy_create()\nl3_agent_scheduler.py: _router_has_binding()\nml2/db.py: is_dhcp_active_on_any_subnet()\n\nSome of those really are just checking \"any\".","commit_id":"74262448bf252628cda58afb24cb2994189f41a5"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"e2fcd2ad375aa8b0aaaf56ae8f88a4a35df1068d","unresolved":false,"context_lines":[{"line_number":321,"context_line":"                        target_project\u003d\u0027*\u0027):"},{"line_number":322,"context_line":"                    return"},{"line_number":323,"context_line":"                ports \u003d ports.filter(models_v2.Port.project_id \u003d\u003d project_id)"},{"line_number":324,"context_line":"            if ports.first() is not None:"},{"line_number":325,"context_line":"                raise exc.InvalidSharedSetting(network\u003dnetwork_id)"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"    def set_ipam_backend(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9e30612f_cf715424","line":324,"in_reply_to":"849d06a6_c29e5ddb","updated":"2026-04-29 09:08:40.000000000","message":"Done","commit_id":"74262448bf252628cda58afb24cb2994189f41a5"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"8580621d53926ec66a2f22ed64d79f11407b22e9","unresolved":true,"context_lines":[{"line_number":321,"context_line":"                        target_project\u003d\u0027*\u0027):"},{"line_number":322,"context_line":"                    return"},{"line_number":323,"context_line":"                ports \u003d ports.filter(models_v2.Port.project_id \u003d\u003d project_id)"},{"line_number":324,"context_line":"            if ports.first() is not None:"},{"line_number":325,"context_line":"                raise exc.InvalidSharedSetting(network\u003dnetwork_id)"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"    def set_ipam_backend(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"849d06a6_c29e5ddb","line":324,"in_reply_to":"b318f940_360fbfe9","updated":"2026-04-29 07:43:46.000000000","message":"From Claude (what a fantastic tool):\n```\nquery.first() is faster. Here\u0027s why:\n\nquery.first() generates a simple SELECT ... LIMIT 1 and returns the row (or None).\n\nquery.limit(1).count() generates a nested subquery: SELECT COUNT(*) FROM (SELECT ... LIMIT 1). The extra subquery wrapping adds overhead at the database level for no benefit — the result is always 0 or 1.\n\nBoth hit the database once, but first() produces simpler SQL. The count() path also goes through SQLAlchemy\u0027s count machinery, adding Python-side overhead on top.\n\nIf you\u0027re checking for existence, the most efficient approach is actually:\n\nfrom sqlalchemy import exists\nsession.query(exists().where(Model.id \u003d\u003d value)).scalar()\nThis generates SELECT EXISTS (SELECT 1 FROM ... WHERE ...), which lets the database short-circuit as soon as it finds a match — no row data is fetched, no ORM object is instantiated.\n\nIn the Neutron codebase you\u0027ll commonly see query.first() used for existence checks, which is fine and readable. Just avoid the limit(1).count() pattern — it\u0027s strictly worse in both readability and performance.\n```\n\nSo I\u0027ll replace the `limit(1).count()` with `first()`.","commit_id":"74262448bf252628cda58afb24cb2994189f41a5"}],"neutron/objects/router.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"5d98a4eb9ded64b90ea34fdf1ed9aae98612220b","unresolved":true,"context_lines":[{"line_number":266,"context_line":"        query \u003d query.filter("},{"line_number":267,"context_line":"            ~l3.Router.project_id.in_(projects))"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"        return bool(query.first())"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def _attach_qos_policy(self, qos_policy_id):"},{"line_number":272,"context_line":"        qos_binding.QosPolicyRouterGatewayIPBinding.delete_objects("}],"source_content_type":"text/x-python","patch_set":6,"id":"5eb08a4b_37682d49","line":269,"range":{"start_line":269,"start_character":15,"end_line":269,"end_character":34},"updated":"2026-05-07 16:37:48.000000000","message":"nit: It\u0027s `return query.first() is not None` in the other files, maybe should be unified","commit_id":"2072dce7349b0f39e6702600e8d2faa6cb468519"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"2776e0853c6475a9a9beca36447bb7b2aa37f931","unresolved":false,"context_lines":[{"line_number":266,"context_line":"        query \u003d query.filter("},{"line_number":267,"context_line":"            ~l3.Router.project_id.in_(projects))"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"        return bool(query.first())"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def _attach_qos_policy(self, qos_policy_id):"},{"line_number":272,"context_line":"        qos_binding.QosPolicyRouterGatewayIPBinding.delete_objects("}],"source_content_type":"text/x-python","patch_set":6,"id":"5cd8cc2a_bb518eb4","line":269,"range":{"start_line":269,"start_character":15,"end_line":269,"end_character":34},"in_reply_to":"5eb08a4b_37682d49","updated":"2026-05-08 06:08:26.000000000","message":"Done","commit_id":"2072dce7349b0f39e6702600e8d2faa6cb468519"}],"neutron/objects/subnetpool.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"5d98a4eb9ded64b90ea34fdf1ed9aae98612220b","unresolved":true,"context_lines":[{"line_number":139,"context_line":""},{"line_number":140,"context_line":"            matching_policies \u003d model_query.query_with_hooks("},{"line_number":141,"context_line":"                context, rbac_db_models.AddressScopeRBAC"},{"line_number":142,"context_line":"            ).filter(shared_to_target_project_or_to_all).first() is not None"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        if not matching_policies:"},{"line_number":145,"context_line":"            raise ext_rbac.RbacPolicyInitError("}],"source_content_type":"text/x-python","patch_set":6,"id":"d8667ade_d1a374f0","line":142,"range":{"start_line":142,"start_character":68,"end_line":142,"end_character":71},"updated":"2026-05-07 16:37:48.000000000","message":"Maybe rename the variable to no_matching_policies and remove the `not` here and on L144","commit_id":"2072dce7349b0f39e6702600e8d2faa6cb468519"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"2776e0853c6475a9a9beca36447bb7b2aa37f931","unresolved":false,"context_lines":[{"line_number":139,"context_line":""},{"line_number":140,"context_line":"            matching_policies \u003d model_query.query_with_hooks("},{"line_number":141,"context_line":"                context, rbac_db_models.AddressScopeRBAC"},{"line_number":142,"context_line":"            ).filter(shared_to_target_project_or_to_all).first() is not None"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        if not matching_policies:"},{"line_number":145,"context_line":"            raise ext_rbac.RbacPolicyInitError("}],"source_content_type":"text/x-python","patch_set":6,"id":"933a13dd_7b9980e8","line":142,"range":{"start_line":142,"start_character":68,"end_line":142,"end_character":71},"in_reply_to":"d8667ade_d1a374f0","updated":"2026-05-08 06:08:26.000000000","message":"Done","commit_id":"2072dce7349b0f39e6702600e8d2faa6cb468519"}]}
