)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"2815dbdbf93f6e160a076ec2ab95ffca0f2dcf10","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"d80efb0a_ee75d9e8","updated":"2026-07-28 12:08:57.000000000","message":"recheck","commit_id":"593bd642b3307c8d6e578b8dcd8a9caf77c69e3d"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"f2c2d06448d03a30fd10421a885aca9238c6e981","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"b0891811_17f30077","updated":"2026-08-12 11:10:11.000000000","message":"I think that we can merge if nobody disagree with the approach","commit_id":"3201a7461cd2b84951015fd1ea36e7e739dcacdc"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"076f522c40b255ac5fcbfeff1cb125262fa33b53","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"41b92193_7ecec20d","updated":"2026-08-10 15:24:09.000000000","message":"Makes sense, the model cache implementation looks good. I have a comment about the cleanup process. But looks good in general.","commit_id":"3201a7461cd2b84951015fd1ea36e7e739dcacdc"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"80279e95f225aa88acefa9f9cd8899c73a344240","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"da223297_d4d73413","updated":"2026-07-29 15:48:05.000000000","message":"lgtm, thanks!","commit_id":"3201a7461cd2b84951015fd1ea36e7e739dcacdc"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"5ecf11ed08304978c603123769043337e603545a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7a5b6207_80c8853d","updated":"2026-08-14 10:58:20.000000000","message":"recheck","commit_id":"3201a7461cd2b84951015fd1ea36e7e739dcacdc"}],"watcher/decision_engine/model/model_root.py":[{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"460f798fa3ccbae3673d6bbe449b54e34684d718","unresolved":true,"context_lines":[{"line_number":112,"context_line":"        self._extended_attributes_enabled \u003d value"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    def _add_to_resource_cache(self, node_uuid, instance):"},{"line_number":115,"context_line":"        if node_uuid in self._node_resource_cache:"},{"line_number":116,"context_line":"            cached \u003d self._node_resource_cache[node_uuid]"},{"line_number":117,"context_line":"            cached[\u0027vcpu\u0027] +\u003d instance.vcpus"},{"line_number":118,"context_line":"            cached[\u0027memory\u0027] +\u003d instance.memory"}],"source_content_type":"text/x-python","patch_set":1,"id":"2598284c_af1259ed","line":115,"updated":"2026-07-28 11:14:27.000000000","message":"won\u0027t this miss adding the first instance created on the node? e.g we have an empty compute node when we first build the model so the node will be created but have no entry in the cache. After some time, the first instance is created and this is called but the instance resources are not added since the node is not in the cache, if I\u0027m correct we could easily fix this by adding an else or using a default dict","commit_id":"593bd642b3307c8d6e578b8dcd8a9caf77c69e3d"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"d983eddbb980b166ca5d43c8d3d53a9b1184a666","unresolved":true,"context_lines":[{"line_number":112,"context_line":"        self._extended_attributes_enabled \u003d value"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    def _add_to_resource_cache(self, node_uuid, instance):"},{"line_number":115,"context_line":"        if node_uuid in self._node_resource_cache:"},{"line_number":116,"context_line":"            cached \u003d self._node_resource_cache[node_uuid]"},{"line_number":117,"context_line":"            cached[\u0027vcpu\u0027] +\u003d instance.vcpus"},{"line_number":118,"context_line":"            cached[\u0027memory\u0027] +\u003d instance.memory"}],"source_content_type":"text/x-python","patch_set":1,"id":"90a34c66_fbcfff98","line":115,"in_reply_to":"2598284c_af1259ed","updated":"2026-07-28 12:08:06.000000000","message":"I\u0027m not sure I\u0027m following you. The idea is that the cache is not initialized on each node creation but until the first time that the `get_node_used_resources` method is called for the node (line 335). Once the cache is created for the node, then we want to maintain it updated but while it\u0027s not used, we don\u0027t want to maintain it at all.","commit_id":"593bd642b3307c8d6e578b8dcd8a9caf77c69e3d"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"859e4a048e7dbe2cf48a9dd6bce4f5e7d813a0e8","unresolved":false,"context_lines":[{"line_number":112,"context_line":"        self._extended_attributes_enabled \u003d value"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    def _add_to_resource_cache(self, node_uuid, instance):"},{"line_number":115,"context_line":"        if node_uuid in self._node_resource_cache:"},{"line_number":116,"context_line":"            cached \u003d self._node_resource_cache[node_uuid]"},{"line_number":117,"context_line":"            cached[\u0027vcpu\u0027] +\u003d instance.vcpus"},{"line_number":118,"context_line":"            cached[\u0027memory\u0027] +\u003d instance.memory"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1cac33_b63553dd","line":115,"in_reply_to":"90a34c66_fbcfff98","updated":"2026-07-28 13:55:04.000000000","message":"ok I see it now, we only care about this cache once we\u0027ve called get_node_used_resources, for other strategies it\u0027s irrelevant so the scenario I mentioned it\u0027s not a problem","commit_id":"593bd642b3307c8d6e578b8dcd8a9caf77c69e3d"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"06b451774b3c8fac19018a3394e05065d0343a35","unresolved":false,"context_lines":[{"line_number":319,"context_line":""},{"line_number":320,"context_line":"        return node_instances"},{"line_number":321,"context_line":""},{"line_number":322,"context_line":"    def get_node_used_resources(self, node):"},{"line_number":323,"context_line":"        if node.uuid in self._node_resource_cache:"},{"line_number":324,"context_line":"            return dict(self._node_resource_cache[node.uuid])"},{"line_number":325,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"2d59dac5_a5268ec5","line":322,"updated":"2026-07-28 08:59:20.000000000","message":"get_node_used_resources, get_node_free_resources, and the invalidate_* methods read/write _node_resource_cache without @instance_lock. All topology methods that also modify the cache ARE locked. Notification handlers run on a separate thread, so a strategy calling get_node_used_resources can inte...\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: If a nova notification arrives while a strategy calls get_node_used_resources, the cache can be populated with stale values overwriting a concurrent invalidation, or read a partially-updated entry. Strategies like basic_consolidation and node_resource_consolidation rely on these values for placem...\n\n**Suggestion**:\nAdd @instance_lock to get_node_used_resources and get_node_free_resources. ModelRoot uses threading.RLock (reentrant), so no deadlock when get_node_used_resources calls the already-locked get_node_instances. Also consider adding @instance_lock to invalidate_node_resource_cache and invalidate_resource_cache for consistency.","commit_id":"593bd642b3307c8d6e578b8dcd8a9caf77c69e3d"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"d7504ea4ce9df81bc7716bc27cfa5dd784c1753d","unresolved":true,"context_lines":[{"line_number":319,"context_line":""},{"line_number":320,"context_line":"        return node_instances"},{"line_number":321,"context_line":""},{"line_number":322,"context_line":"    def get_node_used_resources(self, node):"},{"line_number":323,"context_line":"        if node.uuid in self._node_resource_cache:"},{"line_number":324,"context_line":"            return dict(self._node_resource_cache[node.uuid])"},{"line_number":325,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"58940e1c_f8cc8be1","line":322,"in_reply_to":"0e7904d0_84491a47","updated":"2026-07-29 15:35:42.000000000","message":"You are right that in the current usage pattern we probably won\u0027t hit issues without the locks but the concern is technically valid and adding the lock in get_node_used_resources and in the invalidate methos will be harmless \nand will make it safer to run in multithreaded cases. I will add it. I think we don\u0027t need it in get_node_free_resources, as it calls get_node_used_resources.","commit_id":"593bd642b3307c8d6e578b8dcd8a9caf77c69e3d"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"15f88b6c1ba5ade14e46fe6910ec015716f11340","unresolved":true,"context_lines":[{"line_number":319,"context_line":""},{"line_number":320,"context_line":"        return node_instances"},{"line_number":321,"context_line":""},{"line_number":322,"context_line":"    def get_node_used_resources(self, node):"},{"line_number":323,"context_line":"        if node.uuid in self._node_resource_cache:"},{"line_number":324,"context_line":"            return dict(self._node_resource_cache[node.uuid])"},{"line_number":325,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"0e7904d0_84491a47","line":322,"in_reply_to":"2d59dac5_a5268ec5","updated":"2026-07-28 13:59:11.000000000","message":"I\u0027m not sure if this is correct. At first glance it seems like we\u0027d need to add it to `invalidate_node_resource_cache`, but considering each strategy would have a private copy of the cache I don\u0027t think the scenario proposed would actually be a problem","commit_id":"593bd642b3307c8d6e578b8dcd8a9caf77c69e3d"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"80279e95f225aa88acefa9f9cd8899c73a344240","unresolved":false,"context_lines":[{"line_number":319,"context_line":""},{"line_number":320,"context_line":"        return node_instances"},{"line_number":321,"context_line":""},{"line_number":322,"context_line":"    def get_node_used_resources(self, node):"},{"line_number":323,"context_line":"        if node.uuid in self._node_resource_cache:"},{"line_number":324,"context_line":"            return dict(self._node_resource_cache[node.uuid])"},{"line_number":325,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"8e64b923_07d8426a","line":322,"in_reply_to":"58940e1c_f8cc8be1","updated":"2026-07-29 15:48:05.000000000","message":"I agree wrt get_node_free_resources, the current version seems safe, thanks!","commit_id":"593bd642b3307c8d6e578b8dcd8a9caf77c69e3d"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"58a568f8d2ac938a5881ddaf7b5d490cbea5af92","unresolved":false,"context_lines":[{"line_number":201,"context_line":"        self.assert_node(node)"},{"line_number":202,"context_line":"        self.assert_instance(instance)"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"        already_mapped \u003d self.has_edge(instance.uuid, node.uuid)"},{"line_number":205,"context_line":"        self.add_edge(instance.uuid, node.uuid)"},{"line_number":206,"context_line":"        # Make map_instance idempotent in terms of _node_resource_cache"},{"line_number":207,"context_line":"        if not already_mapped:"}],"source_content_type":"text/x-python","patch_set":2,"id":"00ab027f_c88413e5","line":204,"updated":"2026-08-14 11:13:22.000000000","message":"instance_created() calls map_instance() on a fresh element.Instance(uuid\u003d...) whose vcpus/memory/disk are unset; map_instance now reads those fields to add to _node_resource_cache, so notification-driven cache accumulation breaks or excludes the instance\u0027s resources.\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Risk**: Any \u0027instance.create.end\u0027 notification for a node whose cache entry is already populated makes the notification handler raise NotImplementedError (caught by process_info and logged), leaving the instance unmapped; if it does not raise, resources are excluded and node used-resources are under-repo...\n\n**Priority**: Before merge\n**Why This Matters**: Any \u0027instance.create.end\u0027 notification for a node whose cache entry is already populated makes the notification handler raise NotImplementedError (caught by process_info and logged), leaving the instance unmapped; if it does not raise, resources are excluded and node used-resources are under-repo...\n\n**Recommendation**:\nSwap the order in instance_created to call update_instance() before map_instance(), or make map_instance tolerate unset resource fields (skip cache update when a field is unset) so the cache add is deferred.","commit_id":"3201a7461cd2b84951015fd1ea36e7e739dcacdc"}],"watcher/decision_engine/model/notification/nova.py":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"076f522c40b255ac5fcbfeff1cb125262fa33b53","unresolved":true,"context_lines":[{"line_number":275,"context_line":"                # Invalidate resource cache: instance attributes (vcpus,"},{"line_number":276,"context_line":"                # memory, disk) may have changed before this call (e.g."},{"line_number":277,"context_line":"                # resize), so cached totals for the node would be stale."},{"line_number":278,"context_line":"                self.cluster_data_model.invalidate_node_resource_cache("},{"line_number":279,"context_line":"                    current_node"},{"line_number":280,"context_line":"                )"},{"line_number":281,"context_line":"            if current_node and node !\u003d current_node:"},{"line_number":282,"context_line":"                LOG.debug("}],"source_content_type":"text/x-python","patch_set":2,"id":"b55d6c31_608177f5","line":279,"range":{"start_line":278,"start_character":0,"end_line":279,"end_character":20},"updated":"2026-08-10 15:24:09.000000000","message":"this looks correct to do, but IIUC in the end we will never use the original model cache, since we always copy the model and clear the cache of the copy. So we may not even need this cache updates and all.","commit_id":"3201a7461cd2b84951015fd1ea36e7e739dcacdc"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"f2c2d06448d03a30fd10421a885aca9238c6e981","unresolved":false,"context_lines":[{"line_number":275,"context_line":"                # Invalidate resource cache: instance attributes (vcpus,"},{"line_number":276,"context_line":"                # memory, disk) may have changed before this call (e.g."},{"line_number":277,"context_line":"                # resize), so cached totals for the node would be stale."},{"line_number":278,"context_line":"                self.cluster_data_model.invalidate_node_resource_cache("},{"line_number":279,"context_line":"                    current_node"},{"line_number":280,"context_line":"                )"},{"line_number":281,"context_line":"            if current_node and node !\u003d current_node:"},{"line_number":282,"context_line":"                LOG.debug("}],"source_content_type":"text/x-python","patch_set":2,"id":"a3c6a914_93a96565","line":279,"range":{"start_line":278,"start_character":0,"end_line":279,"end_character":20},"in_reply_to":"55a93811_f8baf106","updated":"2026-08-12 11:10:11.000000000","message":"Acknowledged","commit_id":"3201a7461cd2b84951015fd1ea36e7e739dcacdc"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"1daf468bc0c62cdda9b740b3ae3844f011b69419","unresolved":true,"context_lines":[{"line_number":275,"context_line":"                # Invalidate resource cache: instance attributes (vcpus,"},{"line_number":276,"context_line":"                # memory, disk) may have changed before this call (e.g."},{"line_number":277,"context_line":"                # resize), so cached totals for the node would be stale."},{"line_number":278,"context_line":"                self.cluster_data_model.invalidate_node_resource_cache("},{"line_number":279,"context_line":"                    current_node"},{"line_number":280,"context_line":"                )"},{"line_number":281,"context_line":"            if current_node and node !\u003d current_node:"},{"line_number":282,"context_line":"                LOG.debug("}],"source_content_type":"text/x-python","patch_set":2,"id":"55a93811_f8baf106","line":279,"range":{"start_line":278,"start_character":0,"end_line":279,"end_character":20},"in_reply_to":"b55d6c31_608177f5","updated":"2026-08-12 06:16:49.000000000","message":"That\u0027s correct. In the current usage of the model, we wouldn\u0027t need this as the main model, the one receiving the notifications, is not used in the strategies which are the ones using the get_node_used_resources calls. However, i prefer to add this as defensive measure in case we use it in a different way, i.e. using get_node_used_resource from the main model for some reason.","commit_id":"3201a7461cd2b84951015fd1ea36e7e739dcacdc"}],"watcher/decision_engine/strategy/strategies/base.py":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"076f522c40b255ac5fcbfeff1cb125262fa33b53","unresolved":true,"context_lines":[{"line_number":310,"context_line":"            self._compute_model \u003d audit_scope_handler.get_scoped_model("},{"line_number":311,"context_line":"                collector.get_latest_cluster_data_model()"},{"line_number":312,"context_line":"            )"},{"line_number":313,"context_line":"            # Resources cache is not intended to be shared among strategies"},{"line_number":314,"context_line":"            # executions but on each audit. This is clearing the cache on"},{"line_number":315,"context_line":"            # initializing of the model when starting each strategy execution."},{"line_number":316,"context_line":"            if self._compute_model:"},{"line_number":317,"context_line":"                self._compute_model.invalidate_resource_cache()"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        if not self._compute_model:"},{"line_number":320,"context_line":"            raise exception.ClusterStateNotDefined()"}],"source_content_type":"text/x-python","patch_set":2,"id":"67c5798a_49756426","line":317,"range":{"start_line":313,"start_character":0,"end_line":317,"end_character":63},"updated":"2026-08-10 15:24:09.000000000","message":"what about doing this in get_latest_cluster_data_model()? since we always do a deep copy there, we could call the clear cache too. We could implement the clear cache method in Base model class as noop, and implement the clear cache for compute model only.","commit_id":"3201a7461cd2b84951015fd1ea36e7e739dcacdc"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"f2c2d06448d03a30fd10421a885aca9238c6e981","unresolved":true,"context_lines":[{"line_number":310,"context_line":"            self._compute_model \u003d audit_scope_handler.get_scoped_model("},{"line_number":311,"context_line":"                collector.get_latest_cluster_data_model()"},{"line_number":312,"context_line":"            )"},{"line_number":313,"context_line":"            # Resources cache is not intended to be shared among strategies"},{"line_number":314,"context_line":"            # executions but on each audit. This is clearing the cache on"},{"line_number":315,"context_line":"            # initializing of the model when starting each strategy execution."},{"line_number":316,"context_line":"            if self._compute_model:"},{"line_number":317,"context_line":"                self._compute_model.invalidate_resource_cache()"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        if not self._compute_model:"},{"line_number":320,"context_line":"            raise exception.ClusterStateNotDefined()"}],"source_content_type":"text/x-python","patch_set":2,"id":"03066b0d_22de7aa4","line":317,"range":{"start_line":313,"start_character":0,"end_line":317,"end_character":63},"in_reply_to":"3083d6ea_1779582f","updated":"2026-08-12 11:10:11.000000000","message":"I think that both works, I would say that seems better to put this logic closer to the cluster data model. The way that you added also guarantee that all strategies invalidate the cache before start using. I think that is fine as is too","commit_id":"3201a7461cd2b84951015fd1ea36e7e739dcacdc"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"1daf468bc0c62cdda9b740b3ae3844f011b69419","unresolved":true,"context_lines":[{"line_number":310,"context_line":"            self._compute_model \u003d audit_scope_handler.get_scoped_model("},{"line_number":311,"context_line":"                collector.get_latest_cluster_data_model()"},{"line_number":312,"context_line":"            )"},{"line_number":313,"context_line":"            # Resources cache is not intended to be shared among strategies"},{"line_number":314,"context_line":"            # executions but on each audit. This is clearing the cache on"},{"line_number":315,"context_line":"            # initializing of the model when starting each strategy execution."},{"line_number":316,"context_line":"            if self._compute_model:"},{"line_number":317,"context_line":"                self._compute_model.invalidate_resource_cache()"},{"line_number":318,"context_line":""},{"line_number":319,"context_line":"        if not self._compute_model:"},{"line_number":320,"context_line":"            raise exception.ClusterStateNotDefined()"}],"source_content_type":"text/x-python","patch_set":2,"id":"3083d6ea_1779582f","line":317,"range":{"start_line":313,"start_character":0,"end_line":317,"end_character":63},"in_reply_to":"67c5798a_49756426","updated":"2026-08-12 06:16:49.000000000","message":"It would also work, but at some point we may want to implement some use case of \"copying the model including the cache\" so i preferred to make the clean as part of the strategy initialization instead of as part of get_latest_cluster_data_model(). I can change it if you think it\u0027s appropiate.","commit_id":"3201a7461cd2b84951015fd1ea36e7e739dcacdc"}],"watcher/tests/unit/decision_engine/model/test_model.py":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"9ea67463ef4bf4a9d2930a9b1a0a95c0a8aa49d6","unresolved":false,"context_lines":[{"line_number":403,"context_line":"        mock_get_node_instances \u003d mock.patch.object("},{"line_number":404,"context_line":"            model, \u0027get_node_instances\u0027"},{"line_number":405,"context_line":"        )"},{"line_number":406,"context_line":"        mock_get_node_instances.return_value \u003d []"},{"line_number":407,"context_line":"        with mock_get_node_instances:"},{"line_number":408,"context_line":"            used_b \u003d model.get_node_used_resources(node_b)"},{"line_number":409,"context_line":"            self.assertEqual(8, used_b[\u0027vcpu\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"cc85a1b7_43e0f17a","line":406,"updated":"2026-07-28 12:20:12.000000000","message":"In test_resource_cache_map_instance, mock_get_node_instances.return_value \u003d [] sets return_value on the _patch object before entering the context manager. This does not configure the mock\u0027s return value. When the mock is activated, it returns a default MagicMock, not an empty list. The test still...\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: The test validates the correct behavior (cache hit skips get_node_instances, cache miss calls it), but the return_value assignment is a no-op. A future maintainer may believe the mock returns [] and be confused when modifying the test. If the test is later changed to check return values explicitl...\n\n**Recommendation**:\nUse the conventional context manager pattern: \u0027with mock.patch.object(model, \u0027get_node_instances\u0027) as mock_get_instances: mock_get_instances.return_value \u003d []\u0027 and indent the test body accordingly. Alternatively, remove the dead return_value line since MagicMock iteration already yields nothing.","commit_id":"593bd642b3307c8d6e578b8dcd8a9caf77c69e3d"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"58a568f8d2ac938a5881ddaf7b5d490cbea5af92","unresolved":false,"context_lines":[{"line_number":400,"context_line":""},{"line_number":401,"context_line":"        # Validate that get_node_used_resources does not read the instances"},{"line_number":402,"context_line":"        # when the cache is populated"},{"line_number":403,"context_line":"        mock_get_node_instances \u003d mock.patch.object("},{"line_number":404,"context_line":"            model, \u0027get_node_instances\u0027"},{"line_number":405,"context_line":"        )"},{"line_number":406,"context_line":"        mock_get_node_instances.return_value \u003d []"}],"source_content_type":"text/x-python","patch_set":2,"id":"0080e46e_3a72f75c","line":403,"updated":"2026-08-14 11:13:22.000000000","message":"In test_resource_cache_map_instance, mock_get_node_instances.return_value \u003d [] is set on the un-started mock.patch.object object, not on the mock bound by the context manager, so it has no effect.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: The stub does not express the author\u0027s intent and the test would silently keep passing if the mock wiring changed; future readers may copy the pattern.\n\n**Recommendation**:\nUse \u0027with mock.patch.object(model, \u0027get_node_instances\u0027, return_value\u003d[]) as m:\u0027 and add m.assert_not_called() for the cache-hit branch.","commit_id":"3201a7461cd2b84951015fd1ea36e7e739dcacdc"}]}