)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"d4b156da1fa8bcc5cd2decb2dada663b91e36e8d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"16a222f4_644f2579","updated":"2025-07-09 11:10:36.000000000","message":"check-rdo","commit_id":"77d45a24093eaf3428217d9cf80d5b3f47922ace"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"19aad0163b656f8a270457585739db09aaba5115","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"2ef0eda8_5b1379b0","updated":"2025-07-09 10:22:42.000000000","message":"check-rdo","commit_id":"77d45a24093eaf3428217d9cf80d5b3f47922ace"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"f420e37bb83aa805cdc1b4de07fd95aa37439030","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"8b17d454_27d6a0fc","updated":"2025-07-10 15:37:02.000000000","message":"I think that makes sense, and I saw that promtool doens\u0027t provide a way of delete all data too.\nprometheus job is failing to execute the cmd here: https://84a83418fe08abe99649-be6253c0e82f1539fed391a5717e06a0.ssl.cf2.rackcdn.com/openstack/f663076f15624b5f89fb517baafc2d98/testr_results.html\nI saw that prometheus is already running with admin api enabled[1]:\n```\n/usr/local/bin/prometheus --config.file\u003d/etc/prometheus/prometheus.yml --storage.tsdb.path\u003d/var/lib/prometheus --web.enable-admin-api --web.enable-remote-write-receiver\n```\n[1] https://84a83418fe08abe99649-be6253c0e82f1539fed391a5717e06a0.ssl.cf2.rackcdn.com/openstack/f663076f15624b5f89fb517baafc2d98/controller/logs/services.txt","commit_id":"aef929867f8b689a0dbcd89a3c083d1ad1183495"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"bf45342e701a07c83fc8579c505d05ca38dd2dff","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"be3f2102_3e1dea46","updated":"2025-07-10 03:22:53.000000000","message":"need to fix pep8 issues","commit_id":"aef929867f8b689a0dbcd89a3c083d1ad1183495"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"f9677eef443547db630df414a26b5bbb7901d344","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"bc9a7f1b_07f679b2","updated":"2025-07-09 14:27:12.000000000","message":"tested here: https://github.com/openstack-k8s-operators/watcher-operator/pull/214#issuecomment-3052877191 make sense based on commit message.","commit_id":"aef929867f8b689a0dbcd89a3c083d1ad1183495"},{"author":{"_account_id":28647,"name":"David","display_name":"morenod","email":"dsanzmor@redhat.com","username":"morenod"},"change_message_id":"caf44e7dc853776154607b23a287e017b9b160e9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"5b095713_0eb0b514","updated":"2025-07-16 16:15:56.000000000","message":"@viroel@gmail.com @chandan.kumar@imaginea.com this is now finally really to review. Delete process is done as cleanup and I have verified it is working fine on podified and devstack installations (direct curl or curl on ssh). Please take a look","commit_id":"380572db57798530b64dcac14c6b01b0382c5d8e"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"2405cc51e17509b0d75054d4f835b7308fea69cb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"6322da93_11dd8330","updated":"2025-07-16 17:35:03.000000000","message":"lgtm, it is possible to see in tempest logs the cleanup running, from both environments.\nThanks for working in this improvement David!","commit_id":"380572db57798530b64dcac14c6b01b0382c5d8e"},{"author":{"_account_id":28647,"name":"David","display_name":"morenod","email":"dsanzmor@redhat.com","username":"morenod"},"change_message_id":"993c4e2e485a5e0c0b2abb7e4ee50368f1857690","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"3ef20218_4df0c575","updated":"2025-07-16 13:40:49.000000000","message":"recheck","commit_id":"380572db57798530b64dcac14c6b01b0382c5d8e"}],"watcher_tempest_plugin/tests/scenario/base.py":[{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"bf45342e701a07c83fc8579c505d05ca38dd2dff","unresolved":true,"context_lines":[{"line_number":424,"context_line":"        \"\"\""},{"line_number":425,"context_line":"        LOG.debug(\"Deleting old injected metrics from Datastore\")"},{"line_number":426,"context_line":"        if CONF.optimize.datasource \u003d\u003d \"gnocchi\":"},{"line_number":427,"context_line":"            # TODO: Add function for deleting injected metrics"},{"line_number":428,"context_line":"            # from Gnocchi."},{"line_number":429,"context_line":"            pass"},{"line_number":430,"context_line":"        elif CONF.optimize.datasource \u003d\u003d \"prometheus\":"}],"source_content_type":"text/x-python","patch_set":3,"id":"4a81de9a_cddd679d","line":427,"range":{"start_line":427,"start_character":12,"end_line":427,"end_character":62},"updated":"2025-07-10 03:22:53.000000000","message":"```suggestion\n            # TODO(David): Add function for deleting injected metrics\n```","commit_id":"aef929867f8b689a0dbcd89a3c083d1ad1183495"}],"watcher_tempest_plugin/tests/scenario/test_execute_basic_optim.py":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"f420e37bb83aa805cdc1b4de07fd95aa37439030","unresolved":true,"context_lines":[{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        self.addCleanup(self.rollback_compute_nodes_status)"},{"line_number":73,"context_line":"        self.addCleanup(self.wait_delete_instances_from_model)"},{"line_number":74,"context_line":"        self.clean_old_injected_metrics()"},{"line_number":75,"context_line":"        instances \u003d self._create_one_instance_per_host()"},{"line_number":76,"context_line":"        # wait for compute model updates"},{"line_number":77,"context_line":"        self.wait_for_instances_in_model(instances)"}],"source_content_type":"text/x-python","patch_set":3,"id":"f13107d5_0a7d7025","line":74,"range":{"start_line":74,"start_character":13,"end_line":74,"end_character":39},"updated":"2025-07-10 15:37:02.000000000","message":"so in general, the test should cleanup everything that it creates.\nIt makes sense to cleanup the metrics generated, but I think that each test that generate the metric, should cleanup at the end, and not cleanup as a preventive action.\nSo if this test generate metrics, we would add:\n```\nself.addCleanup(self.clean_old_injected_metrics())\n```","commit_id":"aef929867f8b689a0dbcd89a3c083d1ad1183495"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"1b6a14f8ac686116c01a74ead04502f0fb2bdfc0","unresolved":true,"context_lines":[{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        self.addCleanup(self.rollback_compute_nodes_status)"},{"line_number":73,"context_line":"        self.addCleanup(self.wait_delete_instances_from_model)"},{"line_number":74,"context_line":"        self.clean_old_injected_metrics()"},{"line_number":75,"context_line":"        instances \u003d self._create_one_instance_per_host()"},{"line_number":76,"context_line":"        # wait for compute model updates"},{"line_number":77,"context_line":"        self.wait_for_instances_in_model(instances)"}],"source_content_type":"text/x-python","patch_set":3,"id":"c1442992_3c93ad12","line":74,"range":{"start_line":74,"start_character":13,"end_line":74,"end_character":39},"in_reply_to":"015da2a8_073d3d4c","updated":"2025-07-14 12:24:29.000000000","message":"That\u0027s how it works, we assume that resources do not exist before starting the test. It is how it is done with instances too. We do not start cleaning instances before each test.","commit_id":"aef929867f8b689a0dbcd89a3c083d1ad1183495"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"dfdc9dc801e59cc8870c5b56c8698cd7f56f6b4f","unresolved":true,"context_lines":[{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        self.addCleanup(self.rollback_compute_nodes_status)"},{"line_number":73,"context_line":"        self.addCleanup(self.wait_delete_instances_from_model)"},{"line_number":74,"context_line":"        self.clean_old_injected_metrics()"},{"line_number":75,"context_line":"        instances \u003d self._create_one_instance_per_host()"},{"line_number":76,"context_line":"        # wait for compute model updates"},{"line_number":77,"context_line":"        self.wait_for_instances_in_model(instances)"}],"source_content_type":"text/x-python","patch_set":3,"id":"d796bbca_f35132af","line":74,"range":{"start_line":74,"start_character":13,"end_line":74,"end_character":39},"in_reply_to":"b31bb369_9b2d8aaa","updated":"2025-07-14 11:31:00.000000000","message":"I don\u0027t get what you mean about \"that do not ensure that prometheus is going to be clean\".\nCleanup functions should be add with addCleanup, as per [1].\n\n[1] https://docs.openstack.org/tempest/latest/HACKING.html#test-fixtures-and-resources","commit_id":"aef929867f8b689a0dbcd89a3c083d1ad1183495"},{"author":{"_account_id":28647,"name":"David","display_name":"morenod","email":"dsanzmor@redhat.com","username":"morenod"},"change_message_id":"b944970fcec5515c38cc604773c38a18a9518dca","unresolved":true,"context_lines":[{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        self.addCleanup(self.rollback_compute_nodes_status)"},{"line_number":73,"context_line":"        self.addCleanup(self.wait_delete_instances_from_model)"},{"line_number":74,"context_line":"        self.clean_old_injected_metrics()"},{"line_number":75,"context_line":"        instances \u003d self._create_one_instance_per_host()"},{"line_number":76,"context_line":"        # wait for compute model updates"},{"line_number":77,"context_line":"        self.wait_for_instances_in_model(instances)"}],"source_content_type":"text/x-python","patch_set":3,"id":"2ec2f46a_5974a716","line":74,"range":{"start_line":74,"start_character":13,"end_line":74,"end_character":39},"in_reply_to":"c1442992_3c93ad12","updated":"2025-07-14 12:36:44.000000000","message":"ok, as we manage where we are injecting metrics, we can identify the functions where to add the cleanup.\n\nI have moved the function to addCleanup, I agree that it is more clear now","commit_id":"aef929867f8b689a0dbcd89a3c083d1ad1183495"},{"author":{"_account_id":28647,"name":"David","display_name":"morenod","email":"dsanzmor@redhat.com","username":"morenod"},"change_message_id":"86b371080d25b5a9f5edc6d8fae5f439d4693978","unresolved":true,"context_lines":[{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        self.addCleanup(self.rollback_compute_nodes_status)"},{"line_number":73,"context_line":"        self.addCleanup(self.wait_delete_instances_from_model)"},{"line_number":74,"context_line":"        self.clean_old_injected_metrics()"},{"line_number":75,"context_line":"        instances \u003d self._create_one_instance_per_host()"},{"line_number":76,"context_line":"        # wait for compute model updates"},{"line_number":77,"context_line":"        self.wait_for_instances_in_model(instances)"}],"source_content_type":"text/x-python","patch_set":3,"id":"015da2a8_073d3d4c","line":74,"range":{"start_line":74,"start_character":13,"end_line":74,"end_character":39},"in_reply_to":"d796bbca_f35132af","updated":"2025-07-14 11:52:58.000000000","message":"Yes, I know how to use addcleanup function, but they run after the test, and we need that prometheus will be empty before the test, so if we use the addcleanup, we are asuming that prometheus is empty before executing the test, but we are not ensuring, we are ensuring that we left prometheus empty after executing the test","commit_id":"aef929867f8b689a0dbcd89a3c083d1ad1183495"},{"author":{"_account_id":28647,"name":"David","display_name":"morenod","email":"dsanzmor@redhat.com","username":"morenod"},"change_message_id":"d2d323fabd5731ac48c78e7c661141a27d4ad9f8","unresolved":true,"context_lines":[{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        self.addCleanup(self.rollback_compute_nodes_status)"},{"line_number":73,"context_line":"        self.addCleanup(self.wait_delete_instances_from_model)"},{"line_number":74,"context_line":"        self.clean_old_injected_metrics()"},{"line_number":75,"context_line":"        instances \u003d self._create_one_instance_per_host()"},{"line_number":76,"context_line":"        # wait for compute model updates"},{"line_number":77,"context_line":"        self.wait_for_instances_in_model(instances)"}],"source_content_type":"text/x-python","patch_set":3,"id":"b31bb369_9b2d8aaa","line":74,"range":{"start_line":74,"start_character":13,"end_line":74,"end_character":39},"in_reply_to":"f13107d5_0a7d7025","updated":"2025-07-14 09:07:40.000000000","message":"I had the same thinking when developing this. My first idea was to add a cleanup, to ensure everything is deleted after the test. but that do not ensure that prometheus is going to be clean on the current test, we rely on other stuff to ensure that current test is going to work, so Im not sure where to add the clean function.\n\nMaybe the best option is to add both, even if that is redundant, because we ensure that the test is going to be executed on a clean environment, and we also do not left anything behind after the test is executed","commit_id":"aef929867f8b689a0dbcd89a3c083d1ad1183495"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"e39680fa8e2fbba3120a98f2a154e2878b94a8c0","unresolved":true,"context_lines":[{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        self.addCleanup(self.rollback_compute_nodes_status)"},{"line_number":73,"context_line":"        self.addCleanup(self.wait_delete_instances_from_model)"},{"line_number":74,"context_line":"        self.addCleanup(self.clean_injected_metrics())"},{"line_number":75,"context_line":"        instances \u003d self._create_one_instance_per_host()"},{"line_number":76,"context_line":"        # wait for compute model updates"},{"line_number":77,"context_line":"        self.wait_for_instances_in_model(instances)"}],"source_content_type":"text/x-python","patch_set":8,"id":"847cdf4c_06192c47","line":74,"range":{"start_line":74,"start_character":29,"end_line":74,"end_character":53},"updated":"2025-07-16 08:06:31.000000000","message":"I think `self.clean_injected_metrics()` function is returning \u0027None\u0027 as function has no return value specified. That\u0027s why all the tests are failing with `TypeError: \u0027NoneType\u0027 object is not callable`.\n\nI think it should be called like \"self.clean_injected_metrics\"(Adding a function reference to call it later during cleanup phase). So it should not return \u0027None\u0027 here. And the test should work.","commit_id":"6b06a4f823be311c2c8215aa5ad2178860df556a"},{"author":{"_account_id":28647,"name":"David","display_name":"morenod","email":"dsanzmor@redhat.com","username":"morenod"},"change_message_id":"3ae36a819b7ff1e55848b4271fe0d008f07fd689","unresolved":true,"context_lines":[{"line_number":71,"context_line":""},{"line_number":72,"context_line":"        self.addCleanup(self.rollback_compute_nodes_status)"},{"line_number":73,"context_line":"        self.addCleanup(self.wait_delete_instances_from_model)"},{"line_number":74,"context_line":"        self.addCleanup(self.clean_injected_metrics())"},{"line_number":75,"context_line":"        instances \u003d self._create_one_instance_per_host()"},{"line_number":76,"context_line":"        # wait for compute model updates"},{"line_number":77,"context_line":"        self.wait_for_instances_in_model(instances)"}],"source_content_type":"text/x-python","patch_set":8,"id":"5b99977a_953b422f","line":74,"range":{"start_line":74,"start_character":29,"end_line":74,"end_character":53},"in_reply_to":"847cdf4c_06192c47","updated":"2025-07-16 09:33:50.000000000","message":"you are totally right, thanks for the tip!","commit_id":"6b06a4f823be311c2c8215aa5ad2178860df556a"}]}