)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"c49a3010bb551eef7e73b098813620b6273fc703","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When an issue is encountered in the alarm or rgw pollsters, log error"},{"line_number":10,"context_line":"instead of throwing an exception with the whole traceback. This follows"},{"line_number":11,"context_line":"the example of other pollsters (like manila or designate), which just"},{"line_number":12,"context_line":"log an error line about skipping that particullar pollster. This"},{"line_number":13,"context_line":"significantly improves the logs especially in situations where all"},{"line_number":14,"context_line":"pollsters are enabled at once with the \"*\" wildcard."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"e0bfe4aa_8c9217b4","line":11,"range":{"start_line":11,"start_character":32,"end_line":11,"end_character":57},"updated":"2026-07-23 03:38:22.000000000","message":"Could you point where such handling is implemented for designate/manila ? I checked the code but these looks just calling list API without try-except.","commit_id":"2ec76805b73e28ba6749a743bd61a3080eec891b"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"8ec3e854dc62b2d07aa19473ebe35646d06e0953","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When an issue is encountered in the alarm or rgw pollsters, log error"},{"line_number":10,"context_line":"instead of throwing an exception with the whole traceback. This follows"},{"line_number":11,"context_line":"the example of other pollsters (like manila or designate), which just"},{"line_number":12,"context_line":"log an error line about skipping that particullar pollster. This"},{"line_number":13,"context_line":"significantly improves the logs especially in situations where all"},{"line_number":14,"context_line":"pollsters are enabled at once with the \"*\" wildcard."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"b04911a1_616851b1","line":11,"range":{"start_line":11,"start_character":32,"end_line":11,"end_character":57},"in_reply_to":"e0bfe4aa_8c9217b4","updated":"2026-07-23 18:24:06.000000000","message":"So the end result is similar, which is what I was going for. The underlying mechanism I observed for manila and designate works a little differently on the inside though. For those 2, you have the `KEYSTONE_REQUIRED_FOR_SERVICE`: https://opendev.org/openstack/ceilometer/src/branch/master/ceilometer/share/discovery.py#L21 and that is then handled in https://opendev.org/openstack/ceilometer/src/branch/master/ceilometer/polling/manager.py#L945 We don\u0027t really have an easy way to determine whether the aodh metric endpoint is enabled or whether the rgw config is complete from the manager, so including the code into the alarm/discovery.py and objectstore/rgw.py seemed OK.","commit_id":"2ec76805b73e28ba6749a743bd61a3080eec891b"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":13177,"name":"Emma Foley","email":"efoley@redhat.com","username":"emma-l-foley"},"change_message_id":"d6723d4227c0fde3b1cc0c39d29c4768ca05c597","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"b4655602_69938c14","updated":"2026-07-24 15:23:58.000000000","message":"Please add some unittests","commit_id":"29099e706d0e92f041e65293c775ee8e5df61831"}],"ceilometer/alarm/discovery.py":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"c49a3010bb551eef7e73b098813620b6273fc703","unresolved":true,"context_lines":[{"line_number":43,"context_line":"        \"\"\"Discover resources to monitor.\"\"\""},{"line_number":44,"context_line":"        try:"},{"line_number":45,"context_line":"            return [self.aodh_client.metrics.get(all_projects\u003dTrue)]"},{"line_number":46,"context_line":"        except aodh_exc.Forbidden as e:"},{"line_number":47,"context_line":"            LOG.error(\u0027Skipping alarm discovery, aodh issue: %s\u0027, e)"},{"line_number":48,"context_line":"            return []"}],"source_content_type":"text/x-python","patch_set":1,"id":"81843f7a_04a1575b","line":46,"range":{"start_line":46,"start_character":24,"end_line":46,"end_character":33},"updated":"2026-07-23 03:38:22.000000000","message":"Do you know when we expect 403 from aodh ? Should we rather catch any client exceptions instead ?","commit_id":"2ec76805b73e28ba6749a743bd61a3080eec891b"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"9ec80f4ec70b02639e0a468e4caf32de1c0b8a82","unresolved":true,"context_lines":[{"line_number":43,"context_line":"        \"\"\"Discover resources to monitor.\"\"\""},{"line_number":44,"context_line":"        try:"},{"line_number":45,"context_line":"            return [self.aodh_client.metrics.get(all_projects\u003dTrue)]"},{"line_number":46,"context_line":"        except aodh_exc.Forbidden as e:"},{"line_number":47,"context_line":"            LOG.error(\u0027Skipping alarm discovery, aodh issue: %s\u0027, e)"},{"line_number":48,"context_line":"            return []"}],"source_content_type":"text/x-python","patch_set":1,"id":"e7267e4e_c2404ddc","line":46,"range":{"start_line":46,"start_character":24,"end_line":46,"end_character":33},"in_reply_to":"81843f7a_04a1575b","updated":"2026-07-23 07:28:30.000000000","message":"OK according to the bug report aodh returns 403 when metrics endpoint is disabled. Can we leave a note to mention that because Forbidden usually indicates authorization problem and it\u0027s not clear why 403 is specifically caught here.","commit_id":"2ec76805b73e28ba6749a743bd61a3080eec891b"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"8ec3e854dc62b2d07aa19473ebe35646d06e0953","unresolved":true,"context_lines":[{"line_number":43,"context_line":"        \"\"\"Discover resources to monitor.\"\"\""},{"line_number":44,"context_line":"        try:"},{"line_number":45,"context_line":"            return [self.aodh_client.metrics.get(all_projects\u003dTrue)]"},{"line_number":46,"context_line":"        except aodh_exc.Forbidden as e:"},{"line_number":47,"context_line":"            LOG.error(\u0027Skipping alarm discovery, aodh issue: %s\u0027, e)"},{"line_number":48,"context_line":"            return []"}],"source_content_type":"text/x-python","patch_set":1,"id":"f75711cc_9fd53c7d","line":46,"range":{"start_line":46,"start_character":24,"end_line":46,"end_character":33},"in_reply_to":"e7267e4e_c2404ddc","updated":"2026-07-23 18:24:06.000000000","message":"Yes, Aodh will return 403 when the metric endpoint is disabled ([DEFAULT].enable_evaluation_results_metrics\u003dFalse in aodh.conf). I\u0027m not aware of it returning other codes on purpose, so I\u0027d rather catch just this one and modify the exception handling further in the future if we have the need instead of missing some legitimate issue. I\u0027ll add the comment though.","commit_id":"2ec76805b73e28ba6749a743bd61a3080eec891b"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"f519b0ccbe73742a8f415a0d6d3ef087dc6b9430","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        \"\"\"Discover resources to monitor.\"\"\""},{"line_number":44,"context_line":"        try:"},{"line_number":45,"context_line":"            return [self.aodh_client.metrics.get(all_projects\u003dTrue)]"},{"line_number":46,"context_line":"        except aodh_exc.Forbidden as e:"},{"line_number":47,"context_line":"            LOG.error(\u0027Skipping alarm discovery, aodh issue: %s\u0027, e)"},{"line_number":48,"context_line":"            return []"}],"source_content_type":"text/x-python","patch_set":1,"id":"01c32484_cf18e321","line":46,"range":{"start_line":46,"start_character":24,"end_line":46,"end_character":33},"in_reply_to":"f75711cc_9fd53c7d","updated":"2026-07-23 18:27:59.000000000","message":"Done","commit_id":"2ec76805b73e28ba6749a743bd61a3080eec891b"},{"author":{"_account_id":13177,"name":"Emma Foley","email":"efoley@redhat.com","username":"emma-l-foley"},"change_message_id":"d6723d4227c0fde3b1cc0c39d29c4768ca05c597","unresolved":true,"context_lines":[{"line_number":43,"context_line":"        \"\"\"Discover resources to monitor.\"\"\""},{"line_number":44,"context_line":"        try:"},{"line_number":45,"context_line":"            return [self.aodh_client.metrics.get(all_projects\u003dTrue)]"},{"line_number":46,"context_line":"        except aodh_exc.Forbidden as e:"},{"line_number":47,"context_line":"            # NOTE(jwysogla): aodh will return 403 when the metric"},{"line_number":48,"context_line":"            # endpoint is disabled in aodh.conf"},{"line_number":49,"context_line":"            LOG.error(\u0027Skipping alarm discovery, aodh issue: %s\u0027, e)"}],"source_content_type":"text/x-python","patch_set":2,"id":"370c80ea_28c4c16a","line":46,"updated":"2026-07-24 15:23:58.000000000","message":"Please add a unittest to verify this behaviour. \nIf we migrate aodh into the openstacksdk, and use the sdk here, we want to be have sufficient unittests to ensure that the expected behaviour is maintained","commit_id":"29099e706d0e92f041e65293c775ee8e5df61831"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"71ea1bf2f082f70ca8101fcb176c75b581ffe7a3","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        \"\"\"Discover resources to monitor.\"\"\""},{"line_number":44,"context_line":"        try:"},{"line_number":45,"context_line":"            return [self.aodh_client.metrics.get(all_projects\u003dTrue)]"},{"line_number":46,"context_line":"        except aodh_exc.Forbidden as e:"},{"line_number":47,"context_line":"            # NOTE(jwysogla): aodh will return 403 when the metric"},{"line_number":48,"context_line":"            # endpoint is disabled in aodh.conf"},{"line_number":49,"context_line":"            LOG.error(\u0027Skipping alarm discovery, aodh issue: %s\u0027, e)"}],"source_content_type":"text/x-python","patch_set":2,"id":"60cb35cd_032b4728","line":46,"in_reply_to":"370c80ea_28c4c16a","updated":"2026-07-27 08:14:23.000000000","message":"Done","commit_id":"29099e706d0e92f041e65293c775ee8e5df61831"}],"ceilometer/objectstore/rgw.py":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"c49a3010bb551eef7e73b098813620b6273fc703","unresolved":false,"context_lines":[{"line_number":111,"context_line":"            service_name \u003d conf.rgw_client.rgw_service_name"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"            if service_type is None and service_name is None:"},{"line_number":114,"context_line":"                LOG.error("},{"line_number":115,"context_line":"                    \"Skipping radosgw polling, config issue: Required \""},{"line_number":116,"context_line":"                    \"config not found, need either \""},{"line_number":117,"context_line":"                    \"rgw_client.rgw_service_name or \""}],"source_content_type":"text/x-python","patch_set":1,"id":"946b121d_6560a877","line":114,"range":{"start_line":114,"start_character":16,"end_line":114,"end_character":25},"updated":"2026-07-23 03:38:22.000000000","message":"I agree logging at this layer makes clear sense rather than raising an exception and log that traceback there.","commit_id":"2ec76805b73e28ba6749a743bd61a3080eec891b"},{"author":{"_account_id":13177,"name":"Emma Foley","email":"efoley@redhat.com","username":"emma-l-foley"},"change_message_id":"d6723d4227c0fde3b1cc0c39d29c4768ca05c597","unresolved":true,"context_lines":[{"line_number":111,"context_line":"            service_name \u003d conf.rgw_client.rgw_service_name"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"            if service_type is None and service_name is None:"},{"line_number":114,"context_line":"                LOG.error("},{"line_number":115,"context_line":"                    \"Skipping radosgw polling, config issue: Required \""},{"line_number":116,"context_line":"                    \"config not found, need either \""},{"line_number":117,"context_line":"                    \"rgw_client.rgw_service_name or \""}],"source_content_type":"text/x-python","patch_set":2,"id":"0bf168da_f2d2140c","line":114,"updated":"2026-07-24 15:23:58.000000000","message":"Please add some unittests to verify this behaviour.","commit_id":"29099e706d0e92f041e65293c775ee8e5df61831"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"71ea1bf2f082f70ca8101fcb176c75b581ffe7a3","unresolved":false,"context_lines":[{"line_number":111,"context_line":"            service_name \u003d conf.rgw_client.rgw_service_name"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"            if service_type is None and service_name is None:"},{"line_number":114,"context_line":"                LOG.error("},{"line_number":115,"context_line":"                    \"Skipping radosgw polling, config issue: Required \""},{"line_number":116,"context_line":"                    \"config not found, need either \""},{"line_number":117,"context_line":"                    \"rgw_client.rgw_service_name or \""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa4d3dd5_4be185ea","line":114,"in_reply_to":"0bf168da_f2d2140c","updated":"2026-07-27 08:14:23.000000000","message":"Done","commit_id":"29099e706d0e92f041e65293c775ee8e5df61831"}],"ceilometer/tests/unit/alarm/test_discovery.py":[{"author":{"_account_id":13177,"name":"Emma Foley","email":"efoley@redhat.com","username":"emma-l-foley"},"change_message_id":"5bb61464cf25f2b518e7d3a2a50e66cc79cb85b1","unresolved":true,"context_lines":[{"line_number":29,"context_line":"        self.useFixture("},{"line_number":30,"context_line":"            fixtures.MockPatch(\u0027ceilometer.keystone_client.get_session\u0027))"},{"line_number":31,"context_line":"        self.mock_aodh_client \u003d self.useFixture("},{"line_number":32,"context_line":"            fixtures.MockPatch(\u0027aodhclient.client.Client\u0027)).mock"},{"line_number":33,"context_line":"        self.aodh_instance \u003d self.mock_aodh_client.return_value"},{"line_number":34,"context_line":"        self.discovery \u003d discovery.AlarmDiscovery(self.conf)"},{"line_number":35,"context_line":"        self.manager \u003d mock.Mock()"},{"line_number":36,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5bfb17ea_790234bb","line":33,"range":{"start_line":32,"start_character":0,"end_line":33,"end_character":63},"updated":"2026-07-27 17:53:27.000000000","message":"However, you should remove mock_aodh_client as an instance var. We don\u0027t need access to it outside of setUp.\n\n(Other suggestions)\n\nThis part feels a bit awkward, and I\u0027d usually suggest dropping the mock_aodh_client and embed the return value into the fixture statement.\n\nSomething like what\u0027s done in tests/unit/publisher/test_gnocchi.py:\n\n    self.useFixture(fixtures.MockPatch(\n        \u0027ceilometer.keystone_client.get_session\u0027,\n        return_value\u003dmock.Mock()))\n\nHowever, since you\u0027re updating the return_value and side effects, updating the mock after patching will not have the intended effect, so to update the return values in the test methods, you go in via the dscovery vars, and you can still use a spy pattern if you want.\n\nHave a look at how it\u0027s done in tests/unit/volumes/test_discovery.\n\n```\n    def test_discover_empty(self):\n        self.discovery.client._client.volumes.list \u003d mock.Mock(\n            return_value\u003d[])\n```\n\nhttps://github.com/openstack/ceilometer/blob/b4ccf24479b3a4b55bccb38169bfefd871a2d9cb/ceilometer/tests/unit/volume/test_discovery.py#L57-L73","commit_id":"23225c74909e39b45346cc419b41170548359b4f"}]}
