)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e762ea062aa737f9cd9d46125e51e74f8abd2d6a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"02b8b94e_18170fb9","updated":"2025-06-23 10:42:58.000000000","message":"recheck\nnova-next: timed out after build the mdev samples\n```\n2025-06-20 15:42:04.242675 | compute1 | [28769 Async compile_mdev_samples:28769]: finished successfully\n2025-06-20 18:12:14.892854 | RUN END RESULT_TIMED_OUT: [untrusted : opendev.org/openstack/tempest/playbooks/devstack-tempest.yaml@master]\n```\nnova-live-migraton: two test case failed due to scheduling failed with nova capacity on the target host\n```\nDetails: {\u0027code\u0027: 400, \u0027message\u0027: \u0027No valid host was found. Unable to move instance 267d6cfd-6a47-4418-996d-0d8aced99aa3 to host np0041181074. There is not enough capacity on the host for the instance.\u0027}\n```\ngrenade-skip-level-always: timed out dumping databases","commit_id":"7132efb7eacab875e8e3400a365ddd2933ca481f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"92300061c518bcf43ae6eecbaf010b60c87743c4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"bdc0ac65_c5418c90","updated":"2025-06-20 15:09:02.000000000","message":"recheck nova-mutlicell one test failed with volume issue","commit_id":"7132efb7eacab875e8e3400a365ddd2933ca481f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8aa79c13ead57a5ad46c4eb6aed99b83d0153c3b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"7152bc38_92503546","updated":"2025-06-25 14:06:32.000000000","message":"i think im fine with the current iteration but we could refine it more if dan or other prefer.\n\nin general i think this is a net positive improment and if it unblocks 3.13 in its current form i think that has value. to consider backporting to 2025.1","commit_id":"7d946c45350be935a964990e60bd5213c9aa1423"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e59a58bea5a4cf00c8da22334d4adbfa0362a511","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"b705f02a_43f25db7","updated":"2025-06-25 11:28:51.000000000","message":"recheck bug/2114732","commit_id":"7d946c45350be935a964990e60bd5213c9aa1423"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"89857147ee3c80de1e2c6f3528179bfecdaa6d87","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"b602500c_befdd5f1","updated":"2025-06-25 13:59:39.000000000","message":"recheck etcd failed to start.","commit_id":"7d946c45350be935a964990e60bd5213c9aa1423"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6380cb92239b85c961d482cb3a24fd3f89b53036","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"b11a0867_affac190","in_reply_to":"7152bc38_92503546","updated":"2025-06-25 14:15:51.000000000","message":"by the way i mention 2025.1 just because that was planend to ship in ubuntu 25.04 which has 3.13 so canonicall will want to backprot it to that branch anyway so we might as well just do it upstream.","commit_id":"7d946c45350be935a964990e60bd5213c9aa1423"}],"nova/network/neutron.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e56fab4a95fa4ae195bf5a0bc6b4c42c558d0ff9","unresolved":true,"context_lines":[{"line_number":182,"context_line":"        self.base_client \u003d base_client"},{"line_number":183,"context_line":"        self.admin \u003d admin"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    def __getattr__(self, name):"},{"line_number":186,"context_line":"        # Expose all attributes from the base_client instance"},{"line_number":187,"context_line":"        return getattr(self.base_client, name)"},{"line_number":188,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"77404172_ad9b702c","line":185,"updated":"2025-06-24 17:48:26.000000000","message":"I think this deserves a healthy comment about why you\u0027re doing this seemingly silly thing.\n\nThat said, do you know why we need this re-referencing `__dict__` in the first place? Is it a memory efficiency thing?","commit_id":"7132efb7eacab875e8e3400a365ddd2933ca481f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"8efa796a112d77e69470dcc5d5c54f5caa0531e1","unresolved":true,"context_lines":[{"line_number":182,"context_line":"        self.base_client \u003d base_client"},{"line_number":183,"context_line":"        self.admin \u003d admin"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    def __getattr__(self, name):"},{"line_number":186,"context_line":"        # Expose all attributes from the base_client instance"},{"line_number":187,"context_line":"        return getattr(self.base_client, name)"},{"line_number":188,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"6173aba5_7be7739f","line":185,"in_reply_to":"3fa40534_926ceb33","updated":"2025-06-25 08:31:15.000000000","message":"After playing with it more I think I understand it better. This code wants to wrap all function call on the base client and do the exception transformation logic when the function called. so it injects a wrapper for each callable when the the callable is accessed, hence the __getattribute__ magic. Once could create a wrapper at the class level but I don\u0027t think it would be significantly less complex.","commit_id":"7132efb7eacab875e8e3400a365ddd2933ca481f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"bd118d62d0693a9ccdfd52bc8542a61532e26553","unresolved":true,"context_lines":[{"line_number":182,"context_line":"        self.base_client \u003d base_client"},{"line_number":183,"context_line":"        self.admin \u003d admin"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    def __getattr__(self, name):"},{"line_number":186,"context_line":"        # Expose all attributes from the base_client instance"},{"line_number":187,"context_line":"        return getattr(self.base_client, name)"},{"line_number":188,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"69ce477b_84ed01e2","line":185,"in_reply_to":"54f7b269_62162762","updated":"2025-06-24 19:09:22.000000000","message":"What I meant is.. I\u0027m not sure why we need to share the reference to the single object. Why not just `copy.deepcopy()` the thing instead of this very subversive \"my __dict__ is actually your __dict__\" behavior, which is obviously fragile?","commit_id":"7132efb7eacab875e8e3400a365ddd2933ca481f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8f7eb651abe61064acc420f046993fb9eb4f9b90","unresolved":true,"context_lines":[{"line_number":182,"context_line":"        self.base_client \u003d base_client"},{"line_number":183,"context_line":"        self.admin \u003d admin"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    def __getattr__(self, name):"},{"line_number":186,"context_line":"        # Expose all attributes from the base_client instance"},{"line_number":187,"context_line":"        return getattr(self.base_client, name)"},{"line_number":188,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"90010846_6396fe30","line":185,"in_reply_to":"5aef6c1f_6eaea02d","updated":"2025-06-25 11:44:57.000000000","message":"ya so to me this looks similr but distinct form how we use proxies to interact with libvirt and ceph.\n\n@gibi i agree with your analasy of the orginal intent, it was to wrap any exceptions that might be raised while also providing wrapped client interface as it own.\n\nthe code you are propsoeing is now very similrl to the execute mixin patterin i was using in my eventlet removal poc for talking to ceph \n\nhttps://review.opendev.org/c/openstack/nova/+/917962/8/nova/storage/rbd_utils.py#47\n\n\nThe only real delta is i moved the callable logic into the wrapper function, which would be the proxy in this case.\n\n\nI\u0027m fine with either approach. im undecided if adding functools.wraps to imporve the taceback is worthign. i kind fo fell like not extendign the scope of the change too much more.\n\nwe may want to consider if we wnat to back port this too.\nits not strictly required but the aliaising of __dic__ is iter ill defined or porely tested it seams in core python so i find this approch much cleaner then what we had. im fine with not backporting this until we are forced too by some other breakage but we could apply it to all sable branche premtively fi we felt like it.","commit_id":"7132efb7eacab875e8e3400a365ddd2933ca481f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0912536e0c8a71abf46bbd16baac29dd03bb14e2","unresolved":true,"context_lines":[{"line_number":182,"context_line":"        self.base_client \u003d base_client"},{"line_number":183,"context_line":"        self.admin \u003d admin"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    def __getattr__(self, name):"},{"line_number":186,"context_line":"        # Expose all attributes from the base_client instance"},{"line_number":187,"context_line":"        return getattr(self.base_client, name)"},{"line_number":188,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5aef6c1f_6eaea02d","line":185,"in_reply_to":"6173aba5_7be7739f","updated":"2025-06-25 08:35:11.000000000","message":"I was able to simplify the logic a bit more. And added a comment. I don\u0027t see further simplification at the moment. (no I don\u0027t consider a metaclass based solution simpler)","commit_id":"7132efb7eacab875e8e3400a365ddd2933ca481f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"52843748fcf4b9d11877adf9bdb2106a0033cb00","unresolved":true,"context_lines":[{"line_number":182,"context_line":"        self.base_client \u003d base_client"},{"line_number":183,"context_line":"        self.admin \u003d admin"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    def __getattr__(self, name):"},{"line_number":186,"context_line":"        # Expose all attributes from the base_client instance"},{"line_number":187,"context_line":"        return getattr(self.base_client, name)"},{"line_number":188,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3fa40534_926ceb33","line":185,"in_reply_to":"69ce477b_84ed01e2","updated":"2025-06-25 07:51:15.000000000","message":"\u003e That said, do you know why we need this re-referencing __dict__ in the first place? Is it a memory efficiency thing?\n\nWe need a code historian here :)\n\nThe story starts with https://review.opendev.org/c/openstack/nova/+/312014 introducing the construct almost 9 years ago. \n\nI think the original goal of that change was to wrap the existing client and handle expired admin token. If the client is used with admin creds then it triggers retry and refresh of the token, otherwise it raises back to the caller.\n\nThe implementation indeed looks pretty convoluted for such a goal. \n\nI tried to drop the whole `__dict__` stuff but it result in missing attributes so indeed the baseclient is used. \n\nWhat I noticed now is that it inherits from clientv20.Client but it does not call super in `__init__` to initialize the base so maybe this is caused misbehavior and forced the original author to seek for some hacks to make it work. \n\nI\u0027m not sure how much functional test coverage we have for the whole behavior so I\"m a bit afraid of going in blindly and refactor this to a more sane implementation. But I can try and we can debate if we want that.","commit_id":"7132efb7eacab875e8e3400a365ddd2933ca481f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2dcb7e980614628defae412daa73fed24cb997a0","unresolved":true,"context_lines":[{"line_number":182,"context_line":"        self.base_client \u003d base_client"},{"line_number":183,"context_line":"        self.admin \u003d admin"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    def __getattr__(self, name):"},{"line_number":186,"context_line":"        # Expose all attributes from the base_client instance"},{"line_number":187,"context_line":"        return getattr(self.base_client, name)"},{"line_number":188,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"54f7b269_62162762","line":185,"in_reply_to":"77404172_ad9b702c","updated":"2025-06-24 19:05:55.000000000","message":"i doubt it was memory related althoug it may have been to slightly shorten the callstack and do a direct hash look without indirecting via the virual function call to do getattr(self.base_client, name)\n\n\ni did breifly loook and it as added as part of \n\nhttps://github.com/openstack/nova/commit/80d39a65062a424c9efe42257a38a09c6af2f3e9\nhttps://bugs.launchpad.net/nova/+bug/1571722\n\nlooking at the comment history there was some quetion over the complexity form jay on the def __getattribute__(self, name): lines buyt not this explictly.\n\ni dont see anything in the orginal review that indeicats why we didn use __getattr__ orginally.","commit_id":"7132efb7eacab875e8e3400a365ddd2933ca481f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"8194535c48e537218f39418878ade328365e05bc","unresolved":true,"context_lines":[{"line_number":182,"context_line":"        self.base_client \u003d base_client"},{"line_number":183,"context_line":"        self.admin \u003d admin"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"    def __getattr__(self, name):"},{"line_number":186,"context_line":"        # Expose all attributes from the base_client instance"},{"line_number":187,"context_line":"        return getattr(self.base_client, name)"},{"line_number":188,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1a83099c_cc23bf45","line":185,"in_reply_to":"90010846_6396fe30","updated":"2025-06-25 17:29:18.000000000","message":"Okay I was thinking this was for a data object (despite the name) until looking closer. I think it was insane to use this `self.__dict__ \u003d base.__dict__` hack to accomplish the goal here in the first place. So bug aside, this is an improvement, IMHO. Very glad to see the original merged without a +2 from me :D","commit_id":"7132efb7eacab875e8e3400a365ddd2933ca481f"}]}