)]}'
{"placement/objects/allocation_candidate.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"74b58e29311bb5cc5e694cfe8dd6d7d75e53e1b3","unresolved":false,"context_lines":[{"line_number":954,"context_line":"    return len(common_ancestors.intersection(rp_uuids)) !\u003d 0"},{"line_number":955,"context_line":""},{"line_number":956,"context_line":""},{"line_number":957,"context_line":"def memoize(func):"},{"line_number":958,"context_line":"    cache \u003d {}"},{"line_number":959,"context_line":""},{"line_number":960,"context_line":"    def helper(rp_uuid, parent_uuid_by_rp_uuid):"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_17e26d02","line":957,"updated":"2019-07-10 09:10:58.000000000","message":"Since the helper within is specific to the args passed by _get_ancestors... would be best to give this function a less generic name. memoize_get_ancestors or something like","commit_id":"ea777c772540ab0827c2807bd4042c4e5768e6d8"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"74b58e29311bb5cc5e694cfe8dd6d7d75e53e1b3","unresolved":false,"context_lines":[{"line_number":955,"context_line":""},{"line_number":956,"context_line":""},{"line_number":957,"context_line":"def memoize(func):"},{"line_number":958,"context_line":"    cache \u003d {}"},{"line_number":959,"context_line":""},{"line_number":960,"context_line":"    def helper(rp_uuid, parent_uuid_by_rp_uuid):"},{"line_number":961,"context_line":"        if rp_uuid not in cache:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_525d53e7","line":958,"updated":"2019-07-10 09:10:58.000000000","message":"This cache persists so does that mean that if we get a query for the same rp_uuid later (after the current request) and after the parent topology has changed (which we currently don\u0027t reparenting, but we do allow callers to set rp uuid when creating providers, so reuse is possible) will we break?\n\nmy experiment, not guaranteed to be correct: http://paste.openstack.org/show/754246/","commit_id":"ea777c772540ab0827c2807bd4042c4e5768e6d8"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"2a23ff9fbf8d4a6a965f83beed4b6393de274cd2","unresolved":false,"context_lines":[{"line_number":955,"context_line":""},{"line_number":956,"context_line":""},{"line_number":957,"context_line":"def memoize(func):"},{"line_number":958,"context_line":"    cache \u003d {}"},{"line_number":959,"context_line":""},{"line_number":960,"context_line":"    def helper(rp_uuid, parent_uuid_by_rp_uuid):"},{"line_number":961,"context_line":"        if rp_uuid not in cache:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7faddb67_88c5cc5d","line":958,"in_reply_to":"7faddb67_525d53e7","updated":"2019-07-10 21:36:24.000000000","message":"\u003e This cache persists so does that mean that if we get a query for\n \u003e the same rp_uuid later (after the current request)\n\nSo far, it would be great if this were true.\n\n \u003e and after the\n \u003e parent topology has changed (which we currently don\u0027t reparenting,\n \u003e but we do allow callers to set rp uuid when creating providers, so\n \u003e reuse is possible) will we break?\n\nYes. To clarify/restate what I think you\u0027re saying:\n\nFor this to break today, someone would have to create a provider with no parent, do a GET /a_c request involving that provider, set the parent on that provider, and call GET /a_c again.\n\nWe know this doesn\u0027t happen from Nova, but that doesn\u0027t mean we should ignore the problem.\n\nWhen I was toying with this, I used a cache that only persists for one GET /a_c call. Which is far less powerful, probably to the point of not even being worth doing.\n\nBut to make it persist longer, we would have to (selectively) invalidate the cache (entry/ies) whenever a provider is parented (or deleted for that matter, so we don\u0027t leak). Which means the cache should really live somewhere like objects/resource_provider.py rather than embedded in a closure like this.\n\nThat would also be future-proof if we ever do allow reparenting an already-parented provider.","commit_id":"ea777c772540ab0827c2807bd4042c4e5768e6d8"}]}