)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ff6e289d1d1753514409139634cba6fbe1973f9c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"9a3cfefb_191d7dfb","updated":"2023-08-31 15:00:06.000000000","message":"+2 with comments, nothing holding the merge.","commit_id":"395501c8766d3c1fefabd54ac9ebc363eabce3ea"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"5bc67809d8691282d1b3ea67b6e0e6e59245f617","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"a4e9be79_42079aa1","updated":"2023-08-31 15:52:45.000000000","message":"I don\u0027t feel qualified to +W, but +2 as this looks awesome.\n\nThank you actually making this unified limits stuff happen! I have been testing it recently, and it working really well (although I should put a fix in for -1 not meaning unlimited any more in oslo.limit, assuming that hasn\u0027t been fixed by someone else already).","commit_id":"395501c8766d3c1fefabd54ac9ebc363eabce3ea"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"15a3e729be46448c25507842b1376e22287ecbdd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"d7992132_731c9909","updated":"2023-08-31 21:50:12.000000000","message":"this does have an issue in how its managing vcpu quotas.but i think this could be adresss in the same followup patch that is going to fix the docs for vcpu class\nso im goign to approve this and we can follow up in a diffent patch.","commit_id":"395501c8766d3c1fefabd54ac9ebc363eabce3ea"}],"doc/source/cli/nova-manage.rst":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ff6e289d1d1753514409139634cba6fbe1973f9c","unresolved":true,"context_lines":[{"line_number":1811,"context_line":"   * - 1"},{"line_number":1812,"context_line":"     - An unexpected error occurred"},{"line_number":1813,"context_line":"   * - 2"},{"line_number":1814,"context_line":"     - Failed to connect to the database"},{"line_number":1815,"context_line":""},{"line_number":1816,"context_line":""},{"line_number":1817,"context_line":"See Also"}],"source_content_type":"text/x-rst","patch_set":5,"id":"d25abb2f_f7a35ae0","line":1814,"updated":"2023-08-31 15:00:06.000000000","message":"any reason why you want to specifically return an exception only for the db connection failure ?\nYou could in this case also return another code for being able to connect to Keystone, since both (Nova DB and Keystone API) are seen as datastores.","commit_id":"395501c8766d3c1fefabd54ac9ebc363eabce3ea"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"15a3e729be46448c25507842b1376e22287ecbdd","unresolved":true,"context_lines":[{"line_number":1811,"context_line":"   * - 1"},{"line_number":1812,"context_line":"     - An unexpected error occurred"},{"line_number":1813,"context_line":"   * - 2"},{"line_number":1814,"context_line":"     - Failed to connect to the database"},{"line_number":1815,"context_line":""},{"line_number":1816,"context_line":""},{"line_number":1817,"context_line":"See Also"}],"source_content_type":"text/x-rst","patch_set":5,"id":"382c1817_d8751436","line":1814,"in_reply_to":"b89adfeb_04c6057e","updated":"2023-08-31 21:50:12.000000000","message":"generally return code are only useful if you can use them to make decissions.\n\nim not sure what automation is going to do if it cant connect ot the db or api so 1 an 2 are mostly the same i think.\n\nwe could split something more granualar out for keystone but i think this is ok.","commit_id":"395501c8766d3c1fefabd54ac9ebc363eabce3ea"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"8b3f0fe96cc140c5a1adb9e0cf417fcd76d58b2e","unresolved":true,"context_lines":[{"line_number":1811,"context_line":"   * - 1"},{"line_number":1812,"context_line":"     - An unexpected error occurred"},{"line_number":1813,"context_line":"   * - 2"},{"line_number":1814,"context_line":"     - Failed to connect to the database"},{"line_number":1815,"context_line":""},{"line_number":1816,"context_line":""},{"line_number":1817,"context_line":"See Also"}],"source_content_type":"text/x-rst","patch_set":5,"id":"b89adfeb_04c6057e","line":1814,"in_reply_to":"d25abb2f_f7a35ae0","updated":"2023-08-31 19:12:01.000000000","message":"No particular reason, I think I didn\u0027t consider about Keystone API as a datastore but it makes sense that it really is.","commit_id":"395501c8766d3c1fefabd54ac9ebc363eabce3ea"}],"nova/cmd/manage.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"21e1a2eada27e6732ded9f5b3693e5dda573ba40","unresolved":true,"context_lines":[{"line_number":1,"context_line":"# Copyright (c) 2011 X.commerce, a business unit of eBay Inc."},{"line_number":2,"context_line":"# from keystoneauth1 import loading as ks_loading"},{"line_number":3,"context_line":"# Copyright 2010 United States Government as represented by the"},{"line_number":4,"context_line":"# Administrator of the National Aeronautics and Space Administration."},{"line_number":5,"context_line":"# All Rights Reserved."}],"source_content_type":"text/x-python","patch_set":4,"id":"aa557cf8_f20eb216","line":2,"updated":"2023-08-30 19:00:22.000000000","message":"Copy paste damage.","commit_id":"3b86d5100b10e3840c9da0bd24bf3efd92981e83"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"ce44372a764cd82f29ab15c76129cb9f78b1ca87","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright (c) 2011 X.commerce, a business unit of eBay Inc."},{"line_number":2,"context_line":"# from keystoneauth1 import loading as ks_loading"},{"line_number":3,"context_line":"# Copyright 2010 United States Government as represented by the"},{"line_number":4,"context_line":"# Administrator of the National Aeronautics and Space Administration."},{"line_number":5,"context_line":"# All Rights Reserved."}],"source_content_type":"text/x-python","patch_set":4,"id":"ba80d92b_9a0b8560","line":2,"in_reply_to":"aa557cf8_f20eb216","updated":"2023-08-30 19:25:31.000000000","message":"Done","commit_id":"3b86d5100b10e3840c9da0bd24bf3efd92981e83"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ff6e289d1d1753514409139634cba6fbe1973f9c","unresolved":false,"context_lines":[{"line_number":3403,"context_line":"        service_id \u003d keystone_api.find_service(\u0027nova\u0027).id"},{"line_number":3404,"context_line":""},{"line_number":3405,"context_line":"        # Retrieve the existing resource limits from Keystone."},{"line_number":3406,"context_line":"        registered_limits \u003d keystone_api.registered_limits(region_id\u003dregion_id)"},{"line_number":3407,"context_line":""},{"line_number":3408,"context_line":"        unified_defaults \u003d {"},{"line_number":3409,"context_line":"            rl.resource_name: rl.default_limit for rl in registered_limits}"}],"source_content_type":"text/x-python","patch_set":5,"id":"8fb078f5_c573ef07","line":3406,"updated":"2023-08-31 15:00:06.000000000","message":"we could fail here, shall we rather return a code instead of bubbling up the horrible exception ?\n\n// LATER : ah but the call is wrapped by a try/except clause, my bad.","commit_id":"395501c8766d3c1fefabd54ac9ebc363eabce3ea"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ff6e289d1d1753514409139634cba6fbe1973f9c","unresolved":false,"context_lines":[{"line_number":3424,"context_line":"                output(_(msg))"},{"line_number":3425,"context_line":"                if not dry_run:"},{"line_number":3426,"context_line":"                    try:"},{"line_number":3427,"context_line":"                        keystone_api.create_registered_limit("},{"line_number":3428,"context_line":"                            resource_name\u003dresource_name,"},{"line_number":3429,"context_line":"                            default_limit\u003drlimit, region_id\u003dregion_id,"},{"line_number":3430,"context_line":"                            service_id\u003dservice_id)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1ab824b5_7980b7ee","line":3427,"updated":"2023-08-31 15:00:06.000000000","message":"maybe a stupid question, but is this method idempotent ?\n\nI just don\u0027t want an operator creating duplicate limits if we somehow failed once and then they run again.\n\n//LATER : oh doh, we don\u0027t need it to be idempotent, see my comment below.","commit_id":"395501c8766d3c1fefabd54ac9ebc363eabce3ea"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ff6e289d1d1753514409139634cba6fbe1973f9c","unresolved":false,"context_lines":[{"line_number":3428,"context_line":"                            resource_name\u003dresource_name,"},{"line_number":3429,"context_line":"                            default_limit\u003drlimit, region_id\u003dregion_id,"},{"line_number":3430,"context_line":"                            service_id\u003dservice_id)"},{"line_number":3431,"context_line":"                    except Exception as e:"},{"line_number":3432,"context_line":"                        msg \u003d f\u0027Failed to create default limit: {str(e)}\u0027"},{"line_number":3433,"context_line":"                        print(_(msg))"},{"line_number":3434,"context_line":"                        return_code \u003d 1"}],"source_content_type":"text/x-python","patch_set":5,"id":"8426390b_6bf67d64","line":3431,"updated":"2023-08-31 15:00:06.000000000","message":"same point here, do we raise an exception if the limit already exists ?\n\n\n// LATER : oh, but indeed stupid I am, you already check whether the limit exists, when looping.\n\n(resolving this comment and the one above)","commit_id":"395501c8766d3c1fefabd54ac9ebc363eabce3ea"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ff6e289d1d1753514409139634cba6fbe1973f9c","unresolved":true,"context_lines":[{"line_number":3440,"context_line":""},{"line_number":3441,"context_line":"        # Create project limits if there are any."},{"line_number":3442,"context_line":"        if not project_id:"},{"line_number":3443,"context_line":"            return return_code"},{"line_number":3444,"context_line":""},{"line_number":3445,"context_line":"        output(_(\u0027Reading project limits from the Nova API database ...\u0027))"},{"line_number":3446,"context_line":"        legacy_projects \u003d objects.Quotas.get_all_by_project(ctxt, project_id)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5dffc8eb_2fd5bda7","line":3443,"updated":"2023-08-31 15:00:06.000000000","message":"it could be a specific return code.","commit_id":"395501c8766d3c1fefabd54ac9ebc363eabce3ea"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"8b3f0fe96cc140c5a1adb9e0cf417fcd76d58b2e","unresolved":true,"context_lines":[{"line_number":3440,"context_line":""},{"line_number":3441,"context_line":"        # Create project limits if there are any."},{"line_number":3442,"context_line":"        if not project_id:"},{"line_number":3443,"context_line":"            return return_code"},{"line_number":3444,"context_line":""},{"line_number":3445,"context_line":"        output(_(\u0027Reading project limits from the Nova API database ...\u0027))"},{"line_number":3446,"context_line":"        legacy_projects \u003d objects.Quotas.get_all_by_project(ctxt, project_id)"}],"source_content_type":"text/x-python","patch_set":5,"id":"75d85273_43814947","line":3443,"in_reply_to":"5dffc8eb_2fd5bda7","updated":"2023-08-31 19:12:01.000000000","message":"I did this because the code is going to continue on to try creating the next limit in the list if the previous one fails ... so return_code starts as 0 but if any failure happens it switches to 1. If we get through the entire list with no failures we want to return 0.\n\nAlthough, as I write that, I realize it could also be done with a boolean flag like \"failed \u003d False\" and if a fail happens set it failed \u003d True. Then return 0 if not failed else 1.\n\nIf this were capable of return more than two return codes (for example if I made return code 3 mean \"Call to Keystone API failed\") then I think the return_code may be necessary.","commit_id":"395501c8766d3c1fefabd54ac9ebc363eabce3ea"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ff6e289d1d1753514409139634cba6fbe1973f9c","unresolved":false,"context_lines":[{"line_number":3450,"context_line":""},{"line_number":3451,"context_line":"        # Retrieve existing limits from Keystone."},{"line_number":3452,"context_line":"        project_limits \u003d keystone_api.limits("},{"line_number":3453,"context_line":"            project_id\u003dproject_id, region_id\u003dregion_id)"},{"line_number":3454,"context_line":"        unified_projects \u003d {"},{"line_number":3455,"context_line":"            pl.resource_name: pl.resource_limit for pl in project_limits}"},{"line_number":3456,"context_line":"        msg \u003d f\u0027Found project limits in Keystone: {unified_projects} ...\u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"96f6e8f8_8cc5a0fe","line":3453,"updated":"2023-08-31 15:00:06.000000000","message":"we may fail on this API call, right ? Shall we handle the exception if so ?\n\n// LATER : the caller of this private method already wraps it into a try/clause.","commit_id":"395501c8766d3c1fefabd54ac9ebc363eabce3ea"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"15a3e729be46448c25507842b1376e22287ecbdd","unresolved":true,"context_lines":[{"line_number":3457,"context_line":"        output(_(msg))"},{"line_number":3458,"context_line":""},{"line_number":3459,"context_line":"        output(_(\u0027Creating project limits in Keystone ...\u0027))"},{"line_number":3460,"context_line":"        for resource, plimit in legacy_projects.items():"},{"line_number":3461,"context_line":"            resource_name \u003d legacy_to_unified_names[resource]"},{"line_number":3462,"context_line":"            if resource_name not in unified_projects:"},{"line_number":3463,"context_line":"                msg \u003d ("},{"line_number":3464,"context_line":"                    f\u0027Creating project limit: {resource_name} \u003d {plimit} \u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"0efc6d15_60642c98","line":3461,"range":{"start_line":3460,"start_character":3,"end_line":3461,"end_character":61},"updated":"2023-08-31 21:50:12.000000000","message":"so this is not going to crrectly hand the VCPU quota\n\nthe exisitng vcpu quota is calulated based on falvor.vcpus\nflavor.vcpu consumes from both the VCPU and PCPU resouce class\ndepending on extra specs.\n\n\nso you need to convert the old vcpu quota to both VCPU and PCPU limits\n\nthat will technially doubel the cpu limits vs the old behavior but if we do anything else it will reulst in over quota issues.","commit_id":"395501c8766d3c1fefabd54ac9ebc363eabce3ea"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ff6e289d1d1753514409139634cba6fbe1973f9c","unresolved":true,"context_lines":[{"line_number":3525,"context_line":"            return 2"},{"line_number":3526,"context_line":""},{"line_number":3527,"context_line":"        # Remove obsolete resource limits."},{"line_number":3528,"context_line":"        for resource in (\u0027fixed_ips\u0027, \u0027floating_ips\u0027, \u0027security_groups\u0027,"},{"line_number":3529,"context_line":"                         \u0027security_group_rules\u0027):"},{"line_number":3530,"context_line":"            if resource in legacy_defaults:"},{"line_number":3531,"context_line":"                msg \u003d f\u0027Skipping obsolete limit for {resource} ...\u0027"},{"line_number":3532,"context_line":"                output(_(msg))"}],"source_content_type":"text/x-python","patch_set":5,"id":"76f04032_d7254079","line":3529,"range":{"start_line":3528,"start_character":24,"end_line":3529,"end_character":49},"updated":"2023-08-31 15:00:06.000000000","message":"could be a global var, but fair.","commit_id":"395501c8766d3c1fefabd54ac9ebc363eabce3ea"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"5bc67809d8691282d1b3ea67b6e0e6e59245f617","unresolved":true,"context_lines":[{"line_number":3525,"context_line":"            return 2"},{"line_number":3526,"context_line":""},{"line_number":3527,"context_line":"        # Remove obsolete resource limits."},{"line_number":3528,"context_line":"        for resource in (\u0027fixed_ips\u0027, \u0027floating_ips\u0027, \u0027security_groups\u0027,"},{"line_number":3529,"context_line":"                         \u0027security_group_rules\u0027):"},{"line_number":3530,"context_line":"            if resource in legacy_defaults:"},{"line_number":3531,"context_line":"                msg \u003d f\u0027Skipping obsolete limit for {resource} ...\u0027"},{"line_number":3532,"context_line":"                output(_(msg))"}],"source_content_type":"text/x-python","patch_set":5,"id":"b5c611a9_6bd0f992","line":3529,"range":{"start_line":3528,"start_character":24,"end_line":3529,"end_character":49},"in_reply_to":"76f04032_d7254079","updated":"2023-08-31 15:52:45.000000000","message":"FWIW, I like it this way.","commit_id":"395501c8766d3c1fefabd54ac9ebc363eabce3ea"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ff6e289d1d1753514409139634cba6fbe1973f9c","unresolved":false,"context_lines":[{"line_number":3539,"context_line":"        try:"},{"line_number":3540,"context_line":"            return self._create_unified_limits("},{"line_number":3541,"context_line":"                ctxt, legacy_defaults, project_id, region_id, output, dry_run)"},{"line_number":3542,"context_line":"        except Exception as e:"},{"line_number":3543,"context_line":"            msg \u003d (f\u0027Unexpected error, see nova-manage.log for the full \u0027"},{"line_number":3544,"context_line":"                   f\u0027trace: {str(e)}\u0027)"},{"line_number":3545,"context_line":"            print(_(msg))"}],"source_content_type":"text/x-python","patch_set":5,"id":"c5e0686f_73ab84e2","line":3542,"updated":"2023-08-31 15:00:06.000000000","message":"ah, ok, you wrap all the exceptions here.","commit_id":"395501c8766d3c1fefabd54ac9ebc363eabce3ea"}],"nova/tests/functional/test_nova_manage.py":[{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"ff6e289d1d1753514409139634cba6fbe1973f9c","unresolved":true,"context_lines":[{"line_number":2439,"context_line":"    def test_migrate_to_unified_limits_unexpected_error(self, mock_sdk):"},{"line_number":2440,"context_line":"        # Simulate an error creating limits."},{"line_number":2441,"context_line":"        mock_sdk.return_value.create_registered_limit.side_effect \u003d ("},{"line_number":2442,"context_line":"            test.TestingException(\u0027oops!\u0027))"},{"line_number":2443,"context_line":"        mock_sdk.return_value.create_limit.side_effect \u003d ("},{"line_number":2444,"context_line":"            test.TestingException(\u0027oops!\u0027))"},{"line_number":2445,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"35b6da4e_62cd8574","line":2442,"updated":"2023-08-31 15:00:06.000000000","message":"ok, this test mocks a keystone exception","commit_id":"395501c8766d3c1fefabd54ac9ebc363eabce3ea"}]}