)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"78258a92fa9e3fbf3cdd4367029128c1cb6dc835","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"0e6f29ba_2957b8dc","updated":"2026-07-03 15:06:09.000000000","message":"Ack, make sense to add them since they going to be used by zone_migration in a follow up patch.\nI would like to see first if there is a way of avoid some additional calls, or avoid getting all volume_type.","commit_id":"9a43811fbb104d4040c00c01f12463a59a78ae48"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"3918a9c705c4dd4418ee347e0192bd81ce3d4067","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"b9df825c_a89ce27d","updated":"2026-06-25 08:58:51.000000000","message":"lgtm, I\u0027m also working in the Cinder Helper to move it to use openstacksdk https://review.opendev.org/c/openstack/watcher/+/994841, I think we should push this series first, and then I\u0027ll rebase my work","commit_id":"9a43811fbb104d4040c00c01f12463a59a78ae48"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"1b8c88e7f92d30de416399d25ffb67233964b116","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"07f2acc0_f023d8d6","updated":"2026-07-21 14:36:25.000000000","message":"recheck\n\nalso approving this changes, which is also validated in the next patch in the series, where zone_migration is consume info from new model attributes instead of issuing cinder api.\nresults from the next patch are good:\nhttps://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_965/openstack/965a56f034e147f99181b7d497203685/testr_results.html","commit_id":"16ecaafaadce2d25aaa9b350c00fcb9b645349be"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"d9bdba47e4d52372d14120525975c5f0b5d794fc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"07fa3b20_17fa0f5b","updated":"2026-07-21 17:41:34.000000000","message":"recheck unrelated test failure","commit_id":"16ecaafaadce2d25aaa9b350c00fcb9b645349be"}],"watcher/common/cinder_helper.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":"32358c608f06e1abac8a4704f54111bc0be1614b","unresolved":false,"context_lines":[{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    def get_volume_type_name_by_id(self, volume_type_id):"},{"line_number":80,"context_line":"        \"\"\"Return the volume type name for a given volume type ID.\"\"\""},{"line_number":81,"context_line":"        for vtype in self.get_volume_type_list():"},{"line_number":82,"context_line":"            if vtype.id \u003d\u003d volume_type_id:"},{"line_number":83,"context_line":"                return vtype.name"},{"line_number":84,"context_line":"        raise exception.VolumeTypeNotFound(name\u003dvolume_type_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"989e4633_1b9f2789","line":81,"updated":"2026-06-24 16:47:31.000000000","message":"get_volume_type_name_by_id calls self.cinder.volume_types.list() (a full API round-trip) on every single volume notification with no caching. In a busy cloud this means one Cinder API call per volume create/update/attach/detach/resize event.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: Caching the volume type list or an id-to-name mapping with a TTL would eliminate redundant API calls during bursts of volume notifications, reducing latency and load on the Cinder API.\n\n**Recommendation**:\nConsider caching the volume type list or building an id-to-name dict refreshed periodically. Since volume types rarely change, a simple instance-level cache could suffice. This can be addressed in a follow-up patch.","commit_id":"d1606633053143ee714a3c77597f57cf9145e3ba"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"7ac0eedcf33f438b97131db18e04c700328ba049","unresolved":false,"context_lines":[{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    def get_volume_type_name_by_id(self, volume_type_id):"},{"line_number":80,"context_line":"        \"\"\"Return the volume type name for a given volume type ID.\"\"\""},{"line_number":81,"context_line":"        for vtype in self.get_volume_type_list():"},{"line_number":82,"context_line":"            if vtype.id \u003d\u003d volume_type_id:"},{"line_number":83,"context_line":"                return vtype.name"},{"line_number":84,"context_line":"        raise exception.VolumeTypeNotFound(name\u003dvolume_type_id)"}],"source_content_type":"text/x-python","patch_set":1,"id":"f1eec6d1_9837effe","line":81,"in_reply_to":"989e4633_1b9f2789","updated":"2026-06-25 09:29:04.000000000","message":"Instead of managing the map in cache, the alternative may be to add a new element VolumeType to the Model. I avoided it because we only need the name-id so far which seems doable via this API call, but I can add it in this patch if others think it\u0027s the best way to go.","commit_id":"d1606633053143ee714a3c77597f57cf9145e3ba"},{"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":"415cd9f19358123e3d4281cdccd40a259e219485","unresolved":false,"context_lines":[{"line_number":76,"context_line":"    def get_volume_type_list(self):"},{"line_number":77,"context_line":"        return self.cinder.volume_types.list()"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    def get_volume_type_name_by_id(self, volume_type_id):"},{"line_number":80,"context_line":"        \"\"\"Return the volume type name for a given volume type ID.\"\"\""},{"line_number":81,"context_line":"        for vtype in self.get_volume_type_list():"},{"line_number":82,"context_line":"            if vtype.id \u003d\u003d volume_type_id:"}],"source_content_type":"text/x-python","patch_set":2,"id":"c753c072_580488c7","line":79,"updated":"2026-06-25 10:55:17.000000000","message":"The get_volume_type_name_by_id method fetches the full volume type list from the Cinder API on every invocation. In the notification handler this is called on every volume create/update/attach/detach/resize event.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Caching the volume type list (or building an ID-to-name dict) within the CinderHelper instance would reduce redundant API round-trips during high-volume notification processing, improving notification throughput.\n\n**Recommendation**:\nConsider caching the volume type list as a dict keyed by ID on first access within the CinderHelper, or reuse the existing get_volume_type_list result if the helper is short-lived. This aligns with the pattern used in storage_capacity_balance strategy which caches pool types.","commit_id":"9a43811fbb104d4040c00c01f12463a59a78ae48"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"78258a92fa9e3fbf3cdd4367029128c1cb6dc835","unresolved":true,"context_lines":[{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    def get_volume_type_name_by_id(self, volume_type_id):"},{"line_number":80,"context_line":"        \"\"\"Return the volume type name for a given volume type ID.\"\"\""},{"line_number":81,"context_line":"        for vtype in self.get_volume_type_list():"},{"line_number":82,"context_line":"            if vtype.id \u003d\u003d volume_type_id:"},{"line_number":83,"context_line":"                return vtype.name"},{"line_number":84,"context_line":"        raise exception.VolumeTypeNotFound(name\u003dvolume_type_id)"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def get_volume_snapshots_list(self):"},{"line_number":87,"context_line":"        return self.cinder.volume_snapshots.list("}],"source_content_type":"text/x-python","patch_set":2,"id":"2f6fa377_a89e720a","line":84,"range":{"start_line":81,"start_character":0,"end_line":84,"end_character":63},"updated":"2026-07-03 15:06:09.000000000","message":"Wouldn\u0027t self.cinder.volume_types.get(volume_type_id) work here? It seems that the id that you check here is the same info that is returned in list op.\nhttps://docs.openstack.org/api-ref/block-storage/v3/#volume-types-types","commit_id":"9a43811fbb104d4040c00c01f12463a59a78ae48"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"cebfeacabd4cc4bb457decfc5429b2610ee32a81","unresolved":false,"context_lines":[{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    def get_volume_type_name_by_id(self, volume_type_id):"},{"line_number":80,"context_line":"        \"\"\"Return the volume type name for a given volume type ID.\"\"\""},{"line_number":81,"context_line":"        for vtype in self.get_volume_type_list():"},{"line_number":82,"context_line":"            if vtype.id \u003d\u003d volume_type_id:"},{"line_number":83,"context_line":"                return vtype.name"},{"line_number":84,"context_line":"        raise exception.VolumeTypeNotFound(name\u003dvolume_type_id)"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def get_volume_snapshots_list(self):"},{"line_number":87,"context_line":"        return self.cinder.volume_snapshots.list("}],"source_content_type":"text/x-python","patch_set":2,"id":"e92f5373_a9b1e257","line":84,"range":{"start_line":81,"start_character":0,"end_line":84,"end_character":63},"in_reply_to":"2f6fa377_a89e720a","updated":"2026-07-21 12:01:52.000000000","message":"Done","commit_id":"9a43811fbb104d4040c00c01f12463a59a78ae48"}],"watcher/decision_engine/model/collector/cinder.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":"1f4b76bd20930306ce818c2daf87522b80fce4c4","unresolved":false,"context_lines":[{"line_number":303,"context_line":"            \"metadata\": volume.metadata,"},{"line_number":304,"context_line":"            \"bootable\": volume.bootable,"},{"line_number":305,"context_line":"            \"volume_type\": volume.volume_type,"},{"line_number":306,"context_line":"            \"created_at\": volume.created_at,"},{"line_number":307,"context_line":"            \"host\": getattr(volume, \u0027os-vol-host-attr:host\u0027),"},{"line_number":308,"context_line":"        }"},{"line_number":309,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"286bc821_51c6ba08","line":306,"updated":"2026-07-21 12:32:46.000000000","message":"The collector assigns volume.created_at from the Cinder API (a datetime object) to a StringField, which coerces it via str() producing \u00272017-10-30 00:00:00\u0027. The notification handler assigns data[\u0027created_at\u0027] from the JSON payload, which is \u00272017-10-30T00:00:00\u0027. The two population paths produce...\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Downstream consumers (e.g. zone_migrate strategy) comparing or parsing created_at values will encounter two different string formats depending on whether a volume was last updated by the periodic collector sync or by a real-time notification. This could cause incorrect sorting, comparison failure...\n\n**Suggestion**:\nNormalize created_at to a consistent format in both paths. In the collector, explicitly convert the datetime: volume.created_at.isoformat() or str(volume.created_at).replace(\u0027 \u0027, \u0027T\u0027). Alternatively, use a DateTimeField in the Volume model.","commit_id":"16ecaafaadce2d25aaa9b350c00fcb9b645349be"}],"watcher/decision_engine/model/notification/cinder.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":"32358c608f06e1abac8a4704f54111bc0be1614b","unresolved":false,"context_lines":[{"line_number":190,"context_line":"            volume_type_name \u003d self.cinder.get_volume_type_name_by_id("},{"line_number":191,"context_line":"                data[\u0027volume_type\u0027]"},{"line_number":192,"context_line":"            )"},{"line_number":193,"context_line":"        except exception.VolumeTypeNotFound as exc:"},{"line_number":194,"context_line":"            # If volume_type name can not be find for any reason"},{"line_number":195,"context_line":"            # let\u0027s not update it but not block the rest of updates."},{"line_number":196,"context_line":"            # Periodic collector sync will update."}],"source_content_type":"text/x-python","patch_set":1,"id":"9ed886de_8de641b3","line":193,"updated":"2026-06-24 16:47:31.000000000","message":"The try/except around get_volume_type_name_by_id only catches VolumeTypeNotFound, but the underlying volume_types.list() is a network API call that can raise cinderclient exceptions (auth, connection, timeout). These propagate uncaught and crash the notification handler.\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Risk**: Any transient Cinder API failure during volume notification processing will cause an unhandled exception, dropping the notification entirely instead of falling back gracefully as intended.\n\n**Priority**: Before merge\n**Why This Matters**: The notification handler runs on every volume lifecycle event. Cinder API blips are common in production, and the comment explicitly says the code should \u0027not block the rest of updates\u0027 but the narrow except violates that contract for the most likely failure mode (network errors, not missing types).\n\n**Recommendation**:\nWiden the except clause to also catch cinderclient exceptions. For example: except (exception.VolumeTypeNotFound, cinder_exception.ClientException) as exc: or catch Exception to match the pattern used in create_storage_node and create_pool in the same file. Import cinderclient.exceptions at the top of the module.","commit_id":"d1606633053143ee714a3c77597f57cf9145e3ba"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"7ac0eedcf33f438b97131db18e04c700328ba049","unresolved":false,"context_lines":[{"line_number":190,"context_line":"            volume_type_name \u003d self.cinder.get_volume_type_name_by_id("},{"line_number":191,"context_line":"                data[\u0027volume_type\u0027]"},{"line_number":192,"context_line":"            )"},{"line_number":193,"context_line":"        except exception.VolumeTypeNotFound as exc:"},{"line_number":194,"context_line":"            # If volume_type name can not be find for any reason"},{"line_number":195,"context_line":"            # let\u0027s not update it but not block the rest of updates."},{"line_number":196,"context_line":"            # Periodic collector sync will update."}],"source_content_type":"text/x-python","patch_set":1,"id":"26bca871_d9d1f478","line":193,"in_reply_to":"9ed886de_8de641b3","updated":"2026-06-25 09:29:04.000000000","message":"Maybe I should just remove any try/catch and leave the update to fail and be reported in case of any Error including VolumeTypeNotFound. wdyt?","commit_id":"d1606633053143ee714a3c77597f57cf9145e3ba"},{"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":"415cd9f19358123e3d4281cdccd40a259e219485","unresolved":false,"context_lines":[{"line_number":187,"context_line":"        try:"},{"line_number":188,"context_line":"            # Notifications send volume_type as ID, not name;"},{"line_number":189,"context_line":"            # resolve to name for consistency with the collector."},{"line_number":190,"context_line":"            volume_type_name \u003d self.cinder.get_volume_type_name_by_id("},{"line_number":191,"context_line":"                data[\u0027volume_type\u0027]"},{"line_number":192,"context_line":"            )"},{"line_number":193,"context_line":"        except exception.VolumeTypeNotFound as exc:"}],"source_content_type":"text/x-python","patch_set":2,"id":"430db76d_928daf3a","line":190,"updated":"2026-06-25 10:55:17.000000000","message":"Cinder notifications actually send volume_type as a name, not an ID as the comment claims. get_volume_type_name_by_id compares the name against vtype.id, will likely always fail and fall back silently, making the per-notification Cinder API call unnecessary overhead.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Every volume notification triggers a full volume_type list API call that will likely always fail to find a match, adding latency and Cinder API load for no benefit. The graceful fallback prevents incorrect behavior but masks the ineffective lookup.\n\n**Suggestion**:\nVerify whether Cinder notification payloads actually send volume_type as ID or name for the targeted Cinder versions. If they send the name (as is standard), the get_volume_type_name_by_id call can be removed and data[\u0027volume_type\u0027] used directly. If ID is used in some versions, consider caching the volume type list to avoid an API call per notification.","commit_id":"9a43811fbb104d4040c00c01f12463a59a78ae48"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"78258a92fa9e3fbf3cdd4367029128c1cb6dc835","unresolved":true,"context_lines":[{"line_number":187,"context_line":"        try:"},{"line_number":188,"context_line":"            # Notifications send volume_type as ID, not name;"},{"line_number":189,"context_line":"            # resolve to name for consistency with the collector."},{"line_number":190,"context_line":"            volume_type_name \u003d self.cinder.get_volume_type_name_by_id("},{"line_number":191,"context_line":"                data[\u0027volume_type\u0027]"},{"line_number":192,"context_line":"            )"},{"line_number":193,"context_line":"        except exception.VolumeTypeNotFound as exc:"}],"source_content_type":"text/x-python","patch_set":2,"id":"e58976c9_ec289af2","line":190,"range":{"start_line":190,"start_character":0,"end_line":190,"end_character":70},"updated":"2026-07-03 15:06:09.000000000","message":"if we also save the volume_type_id","commit_id":"9a43811fbb104d4040c00c01f12463a59a78ae48"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"78258a92fa9e3fbf3cdd4367029128c1cb6dc835","unresolved":true,"context_lines":[{"line_number":187,"context_line":"        try:"},{"line_number":188,"context_line":"            # Notifications send volume_type as ID, not name;"},{"line_number":189,"context_line":"            # resolve to name for consistency with the collector."},{"line_number":190,"context_line":"            volume_type_name \u003d self.cinder.get_volume_type_name_by_id("},{"line_number":191,"context_line":"                data[\u0027volume_type\u0027]"},{"line_number":192,"context_line":"            )"},{"line_number":193,"context_line":"        except exception.VolumeTypeNotFound as exc:"}],"source_content_type":"text/x-python","patch_set":2,"id":"75b7db28_3ea01b73","line":190,"range":{"start_line":190,"start_character":0,"end_line":190,"end_character":70},"updated":"2026-07-03 15:06:09.000000000","message":"if we also save the volume_type_id in the model, we could check if the id changed, before issuing a new API call. Not sure if worth the cost of having an additional atribute for that.","commit_id":"9a43811fbb104d4040c00c01f12463a59a78ae48"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"84e9a6ad4dcdc7d9523758ea76ca54720d374962","unresolved":false,"context_lines":[{"line_number":187,"context_line":"        try:"},{"line_number":188,"context_line":"            # Notifications send volume_type as ID, not name;"},{"line_number":189,"context_line":"            # resolve to name for consistency with the collector."},{"line_number":190,"context_line":"            volume_type_name \u003d self.cinder.get_volume_type_name_by_id("},{"line_number":191,"context_line":"                data[\u0027volume_type\u0027]"},{"line_number":192,"context_line":"            )"},{"line_number":193,"context_line":"        except exception.VolumeTypeNotFound as exc:"}],"source_content_type":"text/x-python","patch_set":2,"id":"86eee296_f829ecf2","line":190,"in_reply_to":"430db76d_928daf3a","updated":"2026-07-02 14:06:38.000000000","message":"teim is wrong here, cinder notifications use id in volume_type in payload (https://github.com/openstack/cinder/blob/master/cinder/volume/volume_utils.py#L109).","commit_id":"9a43811fbb104d4040c00c01f12463a59a78ae48"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"1b8c88e7f92d30de416399d25ffb67233964b116","unresolved":false,"context_lines":[{"line_number":187,"context_line":"        try:"},{"line_number":188,"context_line":"            # Notifications send volume_type as ID, not name;"},{"line_number":189,"context_line":"            # resolve to name for consistency with the collector."},{"line_number":190,"context_line":"            volume_type_name \u003d self.cinder.get_volume_type_name_by_id("},{"line_number":191,"context_line":"                data[\u0027volume_type\u0027]"},{"line_number":192,"context_line":"            )"},{"line_number":193,"context_line":"        except exception.VolumeTypeNotFound as exc:"}],"source_content_type":"text/x-python","patch_set":2,"id":"142a04f1_fef46bdf","line":190,"range":{"start_line":190,"start_character":0,"end_line":190,"end_character":70},"in_reply_to":"75b7db28_3ea01b73","updated":"2026-07-21 14:36:25.000000000","message":"Done","commit_id":"9a43811fbb104d4040c00c01f12463a59a78ae48"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"e2b507c424f68c8025113414fdfca51e57fc4e92","unresolved":false,"context_lines":[{"line_number":187,"context_line":"        try:"},{"line_number":188,"context_line":"            # Notifications send volume_type as ID, not name;"},{"line_number":189,"context_line":"            # resolve to name for consistency with the collector."},{"line_number":190,"context_line":"            volume_type_name \u003d self.cinder.get_volume_type_name_by_id("},{"line_number":191,"context_line":"                data[\u0027volume_type\u0027]"},{"line_number":192,"context_line":"            )"},{"line_number":193,"context_line":"        except exception.VolumeTypeNotFound as exc:"}],"source_content_type":"text/x-python","patch_set":2,"id":"94fae58b_6de83578","line":190,"range":{"start_line":190,"start_character":0,"end_line":190,"end_character":70},"in_reply_to":"e58976c9_ec289af2","updated":"2026-07-03 15:06:51.000000000","message":"gerrit is slow and this one got duplicated.","commit_id":"9a43811fbb104d4040c00c01f12463a59a78ae48"},{"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":"30d96f6ecb3bb27d35ba08f1536ce8dd68e1d842","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        if \u0027glance_metadata\u0027 in data:"},{"line_number":185,"context_line":"            bootable \u003d True"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"        try:"},{"line_number":188,"context_line":"            # Notifications send volume_type as ID, not name;"},{"line_number":189,"context_line":"            # resolve to name for consistency with the collector."},{"line_number":190,"context_line":"            volume_type_name \u003d self.cinder.get_volume_type_name_by_id("}],"source_content_type":"text/x-python","patch_set":3,"id":"6d0758a0_cc0102ef","line":187,"updated":"2026-07-21 17:54:20.000000000","message":"In Cinder volume notifications, the payload field \u0027volume_type\u0027 contains the type name, not the UUID. The handler calls get_volume_type_name_by_id(data[\u0027volume_type\u0027]) which passes this name as a UUID to the Cinder API. This lookup fails in production for every notification, triggering the Volume...\n\n**Severity**: HIGH | **Confidence**: 0.8\n\n**Risk**: Every volume notification triggers an unnecessary failed Cinder API call. The volume_type field is never correctly updated from notifications. For VolumeCreateEnd on new volumes, the fallback accesses volume.volume_type on a Volume with only uuid set, which raises NotImplementedError and crashes...\n\n**Priority**: Before merge\n**Why This Matters**: Every volume notification triggers an unnecessary failed Cinder API call. The volume_type field is never correctly updated from notifications. For VolumeCreateEnd on new volumes, the fallback accesses volume.volume_type on a Volume with only uuid set, which raises NotImplementedError and crashes...\n\n**Recommendation**:\nSince Cinder notifications already include volume_type as the name, use data[\u0027volume_type\u0027] directly without the lookup. If ID resolution is truly needed, use data.get(\u0027volume_type_id\u0027) for the UUID. Add a test for the VolumeTypeNotFound fallback path and fix the pass-through mock.","commit_id":"16ecaafaadce2d25aaa9b350c00fcb9b645349be"},{"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":"1f4b76bd20930306ce818c2daf87522b80fce4c4","unresolved":false,"context_lines":[{"line_number":190,"context_line":"            volume_type_name \u003d self.cinder.get_volume_type_name_by_id("},{"line_number":191,"context_line":"                data[\u0027volume_type\u0027]"},{"line_number":192,"context_line":"            )"},{"line_number":193,"context_line":"        except exception.VolumeTypeNotFound as exc:"},{"line_number":194,"context_line":"            # If volume_type name can not be find for any reason"},{"line_number":195,"context_line":"            # let\u0027s not update it but not block the rest of updates."},{"line_number":196,"context_line":"            # Periodic collector sync will update."}],"source_content_type":"text/x-python","patch_set":3,"id":"8ec0fcd9_a3111cb0","line":193,"updated":"2026-07-21 12:32:46.000000000","message":"The update_volume method in cinder.py catches VolumeTypeNotFound and falls back to the existing volume.volume_type value. No unit test exercises this fallback path — all notification tests mock get_volume_type_name_by_id with side_effect\u003dlambda type_id: type_id which never raises.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: If the fallback logic has a bug (e.g. volume.volume_type being None for a newly created volume), it would go undetected. The error-handling path is critical because it determines whether the volume_type field is correctly preserved when the Cinder API call fails.\n\n**Suggestion**:\nAdd a unit test in test_cinder_notifications.py that configures get_volume_type_name_by_id to raise VolumeTypeNotFound and asserts that the volume\u0027s volume_type falls back to the pre-existing value and that the rest of the volume update still proceeds.","commit_id":"16ecaafaadce2d25aaa9b350c00fcb9b645349be"}],"watcher/tests/unit/decision_engine/model/notification/test_cinder_notifications.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":"32358c608f06e1abac8a4704f54111bc0be1614b","unresolved":false,"context_lines":[{"line_number":397,"context_line":"        )"},{"line_number":398,"context_line":"        # check that volume00 was added to the model"},{"line_number":399,"context_line":"        volume_00_name \u003d \u0027990a723f-6c19-4f83-8526-6383c9e9389f\u0027"},{"line_number":400,"context_line":"        volume_00 \u003d storage_model.get_volume_by_uuid(volume_00_name)"},{"line_number":401,"context_line":"        self.assertEqual(volume_00_name, volume_00.uuid)"},{"line_number":402,"context_line":"        self.assertFalse(volume_00.bootable)"},{"line_number":403,"context_line":"        # check that capacity was updated"}],"source_content_type":"text/x-python","patch_set":1,"id":"6d0c071e_3678925c","line":400,"updated":"2026-06-24 16:47:31.000000000","message":"Notification tests test_cinder_volume_create, test_cinder_bootable_volume_create, and test_cinder_volume_update never assert that the three new fields (volume_type, created_at, host) are populated on the Volume. The mocks pass through values but the resulting volume state is never verified.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: The core feature of this patch, populating volume_type, created_at, and host from notifications, has zero test verification. A regression breaking field population would pass all existing tests silently.\n\n**Suggestion**:\nAdd assertions after processing in at least test_cinder_volume_create and test_cinder_volume_update. For example: self.assertEqual(\u0027type_0\u0027, volume_00.volume_type), self.assertEqual(\u00272017-10-30T00:00:00\u0027, volume_00.created_at), self.assertEqual(\u0027host_0@backend_0#pool_0\u0027, volume_00.host).","commit_id":"d1606633053143ee714a3c77597f57cf9145e3ba"},{"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":"6ec8d2cfdf8451357a2f527c4843d8be5788a38b","unresolved":false,"context_lines":[{"line_number":378,"context_line":""},{"line_number":379,"context_line":"        m_cinder_helper.return_value \u003d mock.Mock("},{"line_number":380,"context_line":"            get_storage_pool_by_name\u003dm_get_storage_pool_by_name,"},{"line_number":381,"context_line":"            get_volume_type_name_by_id\u003dmock.Mock("},{"line_number":382,"context_line":"                side_effect\u003dlambda type_id: type_id"},{"line_number":383,"context_line":"            ),"},{"line_number":384,"context_line":"        )"}],"source_content_type":"text/x-python","patch_set":3,"id":"b961f7fc_c5fc12e6","line":381,"updated":"2026-07-21 14:52:16.000000000","message":"All notification tests mock get_volume_type_name_by_id with side_effect\u003dlambda type_id: type_id, which returns the input unchanged. This means the tests never verify actual ID-to-name resolution, and the VolumeTypeNotFound fallback path in update_volume() is never exercised through the notificati...\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: The tests give a false sense of coverage for the volume_type resolution feature. If the resolution logic is incorrect (see CF-001), the tests would still pass because the mock bypasses real Cinder API interaction. The VolumeTypeNotFound fallback is untested in the notification context.\n\n**Recommendation**:\nAt least one notification test should mock get_volume_type_name_by_id to return a distinct name (e.g., return \u0027gold_type\u0027 for input \u0027abc-123\u0027) and assert the model contains the resolved name. Add a test that mocks get_volume_type_name_by_id to raise VolumeTypeNotFound and verify the fallback uses the existing model value without crashing.","commit_id":"16ecaafaadce2d25aaa9b350c00fcb9b645349be"}]}