)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6a9781c59650127e66bdd6ec171bc8b48f6f0c07","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"[FEAT]: using quota_details to optimize validate networks"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Optimizes the validate_networks method by reducing networks calls to the database."},{"line_number":10,"context_line":"Here we utilise the quota_details extension to gather the list of uses ports using the neutron extension, this is a smarter query as we don\u0027t have to list down all the ports which are in use."},{"line_number":11,"context_line":"Instead we now utilise the quota details extension to get the count of used ports directly."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: I4e99ddb3801b7e447e25bff1cadaed595dc759f1"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"37d00520_0c0d30c4","line":11,"range":{"start_line":9,"start_character":0,"end_line":11,"end_character":91},"updated":"2025-05-16 10:08:12.000000000","message":"Optimizes the validate_networks method by reducing using less expensive\nAPI calls to neutron.\n\nNow we utilise the quota_details extension to gather the port quota and usage\ninformation, this is a smarter query as we don\u0027t have to list all the ports which are in use by the project reducing neutron db load.\n\nInstead we now utilise the quota details extension to get the count of used ports directly.\n\n\nCloses-Bug: #2027156","commit_id":"78a89f1883009efbf76595aabbd93038368e4cef"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"90591f4c4ea7909db0093475fbfc2a9bd69fba6c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"a03631e2_60fc7e6c","updated":"2025-03-21 13:47:46.000000000","message":"This is going to fail pep8. I didn\u0027t check tests yet since there\u0027s stuff obviously not covered (if the `get` calls return None) that the tests are passing.","commit_id":"fa5ab18eb3367b6bf7da210f43f8636195e2bfbf"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6a9781c59650127e66bdd6ec171bc8b48f6f0c07","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":17,"id":"b50a4893_a0552f39","updated":"2025-05-16 10:08:12.000000000","message":"we could proceed with your approch if we add extra test coverage\n\nhowever i think can do this cleaner.\n\ni have tried to explain my thinking inline.","commit_id":"78a89f1883009efbf76595aabbd93038368e4cef"}],"nova/network/neutron.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"90591f4c4ea7909db0093475fbfc2a9bd69fba6c","unresolved":true,"context_lines":[{"line_number":2727,"context_line":"            if quota_details_extension:"},{"line_number":2728,"context_line":"                # if quota details extension is available"},{"line_number":2729,"context_line":"                data \u003d neutron.show_quota_details(project_id\u003dcontext.project_id)[\u0027quota\u0027]"},{"line_number":2730,"context_line":"                ports_quota \u003d data.get(\u0027port\u0027)"},{"line_number":2731,"context_line":"                free_ports \u003d quotas.get(\u0027port\u0027) - ports_quota.get(\u0027used\u0027)"},{"line_number":2732,"context_line":"            else:"},{"line_number":2733,"context_line":"                # else if quota details extension is unavailable, list all the ports"}],"source_content_type":"text/x-python","patch_set":2,"id":"a0bbf7d8_901dec78","line":2730,"updated":"2025-03-21 13:47:46.000000000","message":"Since you use get here, this could be `None` and thus the following line may explode with `AttributeError`","commit_id":"fa5ab18eb3367b6bf7da210f43f8636195e2bfbf"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"90591f4c4ea7909db0093475fbfc2a9bd69fba6c","unresolved":true,"context_lines":[{"line_number":2728,"context_line":"                # if quota details extension is available"},{"line_number":2729,"context_line":"                data \u003d neutron.show_quota_details(project_id\u003dcontext.project_id)[\u0027quota\u0027]"},{"line_number":2730,"context_line":"                ports_quota \u003d data.get(\u0027port\u0027)"},{"line_number":2731,"context_line":"                free_ports \u003d quotas.get(\u0027port\u0027) - ports_quota.get(\u0027used\u0027)"},{"line_number":2732,"context_line":"            else:"},{"line_number":2733,"context_line":"                # else if quota details extension is unavailable, list all the ports"},{"line_number":2734,"context_line":"                # We only need the port count so only ask for ids back."}],"source_content_type":"text/x-python","patch_set":2,"id":"16dc08ba_1e3f49d3","line":2731,"updated":"2025-03-21 13:47:46.000000000","message":"Because you use `.get()` this may end up being `None - None` or `17 - None` or `None - 17`, all of which will explode.","commit_id":"fa5ab18eb3367b6bf7da210f43f8636195e2bfbf"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"15b97c7a788efee854d9bfa0b8c973db1ed8419f","unresolved":true,"context_lines":[{"line_number":2712,"context_line":"        neutron \u003d get_client(context)"},{"line_number":2713,"context_line":"        ports_needed_per_instance \u003d self._ports_needed_per_instance("},{"line_number":2714,"context_line":"            context, neutron, requested_networks)"},{"line_number":2715,"context_line":"        quota_details_extension \u003d self.has_quota_details_extension("},{"line_number":2716,"context_line":"            context, neutron)"},{"line_number":2717,"context_line":""},{"line_number":2718,"context_line":"        # Note(PhilD): Ideally Nova would create all required ports as part of"}],"source_content_type":"text/x-python","patch_set":5,"id":"e3309d14_fff192fc","line":2715,"updated":"2025-05-07 10:54:23.000000000","message":"i mentioned this on irc but incase you missed my respocne\n\nthe unit test failreu are because we have addtional test beyond the one you added that call validate_networks\n\nthey are now failing because you have not mocked this call so you need to add mocs the same as you did for your new unit test","commit_id":"41a85e06512f8ba91cfd6ba2d62da22c8f9fd9a7"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"91ae274d89297e63a3d0e97d226c989e351f4b91","unresolved":false,"context_lines":[{"line_number":2712,"context_line":"        neutron \u003d get_client(context)"},{"line_number":2713,"context_line":"        ports_needed_per_instance \u003d self._ports_needed_per_instance("},{"line_number":2714,"context_line":"            context, neutron, requested_networks)"},{"line_number":2715,"context_line":"        quota_details_extension \u003d self.has_quota_details_extension("},{"line_number":2716,"context_line":"            context, neutron)"},{"line_number":2717,"context_line":""},{"line_number":2718,"context_line":"        # Note(PhilD): Ideally Nova would create all required ports as part of"}],"source_content_type":"text/x-python","patch_set":5,"id":"16de3d27_51fa295d","line":2715,"in_reply_to":"e3309d14_fff192fc","updated":"2025-05-08 11:36:22.000000000","message":"Done","commit_id":"41a85e06512f8ba91cfd6ba2d62da22c8f9fd9a7"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6a9781c59650127e66bdd6ec171bc8b48f6f0c07","unresolved":true,"context_lines":[{"line_number":2710,"context_line":"        LOG.debug(\u0027validate_networks() for %s\u0027, requested_networks)"},{"line_number":2711,"context_line":""},{"line_number":2712,"context_line":"        neutron \u003d get_client(context)"},{"line_number":2713,"context_line":"        ports_needed_per_instance \u003d self._ports_needed_per_instance("},{"line_number":2714,"context_line":"            context, neutron, requested_networks)"},{"line_number":2715,"context_line":"        quota_details_extension \u003d self.has_quota_details_extension("},{"line_number":2716,"context_line":"            context, neutron)"}],"source_content_type":"text/x-python","patch_set":17,"id":"f27a60ff_e860fdc6","line":2713,"updated":"2025-05-16 10:08:12.000000000","message":"if we do not need any ports we can just return\n\n```\n# if no ports are needed, we can\u0027t exceed quota.\nif ports_needed_per_instance \u003d\u003d 0:\n    return num_instances\n```","commit_id":"78a89f1883009efbf76595aabbd93038368e4cef"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6a9781c59650127e66bdd6ec171bc8b48f6f0c07","unresolved":true,"context_lines":[{"line_number":2719,"context_line":"        # from the hypervisor.  So we just check the quota and return"},{"line_number":2720,"context_line":"        # how many of the requested number of instances can be created"},{"line_number":2721,"context_line":"        if ports_needed_per_instance:"},{"line_number":2722,"context_line":"            quotas \u003d neutron.show_quota(context.project_id)[\u0027quota\u0027]"},{"line_number":2723,"context_line":"            total_ports \u003d quotas.get(\u0027port\u0027, -1)"},{"line_number":2724,"context_line":"            if total_ports \u003d\u003d -1:"},{"line_number":2725,"context_line":"                # Unlimited Port Quota"}],"source_content_type":"text/x-python","patch_set":17,"id":"ca91c07a_ec078cf2","line":2722,"range":{"start_line":2722,"start_character":12,"end_line":2722,"end_character":68},"updated":"2025-05-16 10:08:12.000000000","message":"if the quota details extension is aviablable we can remove this call as the limit is returned by the details extension.\n\nso we probably should refactor this as follows\n\nembediogn these nested if inline is kind of messy it may be better to factor this out into two seperate functions that return the same data\n\n\nsomething like this\n\n\nfirst let create a fucntion to encapsulate get the quota usage\nvia the new api\n\n```\nNOT_FOUND_SENTINAL \u003d object()\nUNLIMITED_PORT_QUOTA_SENTINAL \u003d object()\n\ndef get_quota_usage_details(self, context):\n    data \u003d neutron.show_quota_details(\n        project_id\u003dcontext.project_id)[\u0027quota\u0027]\n    used \u003d data.get(\u0027used\u0027, self.NOT_FOUND_SENTINAL)\n    limit \u003d data.get(\u0027limit\u0027, self.NOT_FOUND_SENTINAL)\n    reserved \u003d data.get(\u0027limit\u0027, self.NOT_FOUND_SENTINAL)\n    \n    # This should not happen but if we get a response and we \n    # just fallback.\n    if self.NOT_FOUND_SENTINAL in (limit, reserved):\n        return self.get_quota_usage_fallback(context)\n\n    # if there is no limit return the data we have but dont calulate\n    # the free ports. \n    if limit \u003d\u003d -1\n       return return self.UNLIMITED_PORT_QUOTA_SENTINAL, used_ports, limit\n\n    # otherwise, there is a limit and we should calulate the free ports\n    free \u003d limit - reserved - used\n    return free, used, limit\n```\n\nnow if the extension is aviable we can  initalise the limits like this\n\n```\nfree_ports \u003d -1\nused_ports \u003d -1\nlimit \u003d -1\nif quota_details_extension:\n  free_ports, used_ports, limit \u003d self.get_quota_usage_details()\n```\nif quota_details_extension is not supported we should also wrap the old logic in \na new fallback function\n\n\n```\nelse:\n    free_ports, used_ports, limit \u003d self.get_quota_usage_fallback(context)\n\n```\n\nwhere get_quota_usage_fallback is\n\n\n```\ndef get_quota_usage_fallback(self, context):\n    quotas \u003d neutron.show_quota(context.project_id)[\u0027quota\u0027]\n    limit \u003d  quotas.get(\u0027port\u0027, -1)\n    \n    # first we early out if there is unlmited quota\n    if limit \u003d\u003d -1:\n        return self.UNLIMITED_PORT_QUOTA_SENTINAL, None, limit\n    \n    # otherwise we need to calulate the current usage and free ports.\n    \n    params \u003d dict(tenant_id\u003dcontext.project_id, fields\u003d[\u0027id\u0027])\n    ports \u003d neutron.list_ports(**params)[\u0027ports\u0027]\n    used \u003d len(ports)\n    # NOTE(sean-k-mooney): The legacy code did not have any awareness\n    # of reserved ports so for backward compatibility, we keep\n    # the logic the same.\n    free_ports \u003d limit - used\n    \n    return free_ports, used, limit\n        \n\n```\n\nif free_ports is UNLIMITED_PORT_QUOTA_SENTINAL we know that ports are unlimited.\nso before going any futher we should check that.\n\n```\n# we could also check limit \u003d\u003d -1 here but in either case\n# if there is no port limit we just say all vms can be created.\nif free_ports is self.UNLIMITED_PORT_QUOTA_SENTINAL:\n  return num_instances\n```\n\n\nget_quota_usage_details internally falls back to get_quota_usage_fallback if we had any missing data so at this point we now if we have not returned \nwe know free_ports, used_ports, limit are set to actual values that we need to compare to the requested number of instnaces which is just hte remainder of the existing function\n\n```\n            # If we are oversubscribed, report that as an error.\n            if free_ports \u003c 0:\n                msg \u003d (_(\"The number of defined ports: %(ports)d \"\n                         \"is over the limit: %(quota)d\") %\n                       {\u0027ports\u0027: used_ports,\n                        \u0027quota\u0027: quotas.get(\u0027port\u0027)})\n                raise exception.PortLimitExceeded(msg)\n            ports_needed \u003d ports_needed_per_instance * num_instances\n            # if we have free ports and we need less then is available\n            if free_ports \u003e\u003d ports_needed:\n                return num_instances\n            # Otherwise we calculate how many will fit, rounding down\n            # by using integer divide. \n            else:\n                return free_ports // ports_needed_per_instance\n```","commit_id":"78a89f1883009efbf76595aabbd93038368e4cef"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6a9781c59650127e66bdd6ec171bc8b48f6f0c07","unresolved":true,"context_lines":[{"line_number":2732,"context_line":"                data \u003d neutron.show_quota_details("},{"line_number":2733,"context_line":"                    project_id\u003dcontext.project_id)[\u0027quota\u0027]"},{"line_number":2734,"context_line":"                ports_quota \u003d data.get(\u0027port\u0027, -1)"},{"line_number":2735,"context_line":"                if ports_quota !\u003d -1:"},{"line_number":2736,"context_line":"                    used_ports \u003d ports_quota.get(\u0027used\u0027, -1)"},{"line_number":2737,"context_line":"                    if used_ports !\u003d -1:"},{"line_number":2738,"context_line":"                        extension_fallback \u003d False"},{"line_number":2739,"context_line":"            # if quota details extension is unavailable or does not work"},{"line_number":2740,"context_line":"            # list all the ports, We only need the port count"},{"line_number":2741,"context_line":"            # so only ask for ids back."}],"source_content_type":"text/x-python","patch_set":17,"id":"12211007_d29e2bf0","line":2738,"range":{"start_line":2735,"start_character":0,"end_line":2738,"end_character":50},"updated":"2025-05-16 10:08:12.000000000","message":"this code is not tested by the new unit test that you added.\n\nif we do not refactor as i propose above can you add two more test cases \n\none that mocks out the reponce as \n\n```\n mocked_client.show_quota_details.return_value \u003d {\n            \u0027quota\u0027: {\n            }\n        }\n```\nto test the first if and this to test the second\n\n```\n mocked_client.show_quota_details.return_value \u003d {\n            \u0027quota\u0027: {\n                \u0027port\u0027: {\n                 \"limit\": 500,\n                  \"reserved\": 3\n                }\n            }\n        }\n```\n\nI\u0027m not sure that we will ever have a port quota object without \"used\", so I expect that in the real world, we will never need the second if branch. However, since we could compute it ourselves, I\u0027m okay with the current approach.","commit_id":"78a89f1883009efbf76595aabbd93038368e4cef"}],"nova/tests/unit/network/test_neutron.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"91ae274d89297e63a3d0e97d226c989e351f4b91","unresolved":true,"context_lines":[{"line_number":4098,"context_line":"                                      {\u0027ports\u0027: 5,"},{"line_number":4099,"context_line":"                                       \u0027quota\u0027: 1})"},{"line_number":4100,"context_line":"            self.assertEqual(expected_exception_msg, str(exc))"},{"line_number":4101,"context_line":"    "},{"line_number":4102,"context_line":"    @mock.patch("},{"line_number":4103,"context_line":"        \"nova.network.neutron.API.has_quota_details_extension\","},{"line_number":4104,"context_line":"        new\u003dmock.Mock(return_value\u003dTrue),"}],"source_content_type":"text/x-python","patch_set":6,"id":"df68b83c_f1af820e","line":4101,"updated":"2025-05-08 11:36:22.000000000","message":"This is going to fail our style checks.\n\nYou can run those locally with `tox -e pep8` or you can use the pre-commit tool.\n\neither will fix this for you.","commit_id":"1e56dfa2aaa4cf6207e69cda85632d6d31ed06ad"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6a9781c59650127e66bdd6ec171bc8b48f6f0c07","unresolved":true,"context_lines":[{"line_number":2211,"context_line":"        ids \u003d [uuids.my_netid1, uuids.my_netid2]"},{"line_number":2212,"context_line":"        mocked_client.list_networks.return_value \u003d {\u0027networks\u0027: self.nets2}"},{"line_number":2213,"context_line":"        mocked_client.show_quota.return_value \u003d {\u0027quota\u0027: {\u0027port\u0027: 5}}"},{"line_number":2214,"context_line":"        mocked_client.show_quota_details.return_value \u003d {"},{"line_number":2215,"context_line":"            \u0027quota\u0027: {"},{"line_number":2216,"context_line":"                \u0027port\u0027: {"},{"line_number":2217,"context_line":"                    \u0027used\u0027: len(self.port_data2)"},{"line_number":2218,"context_line":"                }"},{"line_number":2219,"context_line":"            }"},{"line_number":2220,"context_line":"        }"},{"line_number":2221,"context_line":"        max_count \u003d self.api.validate_networks(self.context,"},{"line_number":2222,"context_line":"                                               requested_networks, 2)"},{"line_number":2223,"context_line":"        self.assertEqual(1, max_count)"}],"source_content_type":"text/x-python","patch_set":17,"id":"8ebb1010_4ab5a7c3","line":2220,"range":{"start_line":2214,"start_character":7,"end_line":2220,"end_character":9},"updated":"2025-05-16 10:08:12.000000000","message":"this is not a realsitic respocne\n\nlets at least provide a full port object\n\n\"port\": {\n            \"used\": 21,\n            \"limit\": 500,\n            \"reserved\": 3\n        },\n        \n        \n        \n```suggestion\n        mocked_client.show_quota_details.return_value \u003d {\n            \u0027quota\u0027: {\n                \"port\": {\n                    \"used\": 21,\n                    \"limit\": 500,\n                    \"reserved\": 3\n                },\n            }\n        }\n```","commit_id":"78a89f1883009efbf76595aabbd93038368e4cef"}]}