)]}' {"/PATCHSET_LEVEL":[{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"0a8cadf6acf5a990d2118e3f222577f209a619bc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"39e159e1_7bff4c2d","updated":"2024-09-04 16:09:00.000000000","message":"I had a conversation with Erno, during which we decided on the following:\n\n - Split the metric into tenants\n - Implement a reset mechanism\n - Add a config option to the aodh.conf (maybe [DEFAULT]/enable_metrics ?) to allow to disable this feature.\n\nAlso the original intention was to gather and export/publish these metrics with ceilometer. We\u0027re currently unsure about that and it\u0027ll need further discussions.","commit_id":"435700d9dc128fb4085594eb1ab69a087b416341"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"a5b2dad663a084cdb95ef1b9bdf464f6af92b7dc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6e767221_e1bf8e47","in_reply_to":"39e159e1_7bff4c2d","updated":"2024-09-04 16:35:03.000000000","message":"I\u0027m setting this as WIP until then.","commit_id":"435700d9dc128fb4085594eb1ab69a087b416341"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"fcd01c537313cfda13e1803e3cafb8d074e9a0bc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"8a8f8d23_0250160b","updated":"2024-10-02 14:10:31.000000000","message":"Took a good pass on this, couple of things popped up. I think it\u0027s close \u0027though.","commit_id":"c6ad61152cadd57e95117d0ca5e9c6c8eda3c85c"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"d8d1adc4563c6c0832d5a112967d530a48515954","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"c7f6fd7a_3b3678ab","updated":"2024-10-15 14:21:20.000000000","message":"recheck - postgresql job should work now","commit_id":"d69b1c0b330295517003408b4a01d722d8666da3"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"b8481ec35252490b55ce31e2de62e1e4e482eea5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"0adc8613_f0f9d6ed","updated":"2024-11-07 20:05:16.000000000","message":"@kajinamit@oss.nttdata.com I included code to check policies. You can see a demo of it working here: https://paste.opendev.org/show/bPf7di1LChVKlQsDp94c/","commit_id":"eb71ed7d6da01fb80d468be9207c26a76ae3decd"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"23e3ae8fb95b01dcf6d8eaf13c81742cf3ddd30b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"e54e3465_058cc0a1","updated":"2024-11-19 13:43:34.000000000","message":"@Takashi You probably missed my latest message. I added code for the policy checking. What do you think about the patch now?","commit_id":"974867877b5de189a123b6712780a19a92ab058d"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"20c5f67245b5559296b4c616b59280c9a7edee65","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"79191e00_dd74f199","updated":"2024-11-19 14:14:40.000000000","message":"One more suggestion inline. Sorry I should have noticed this earlier.","commit_id":"974867877b5de189a123b6712780a19a92ab058d"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"965d312e2d1586c4b87d0bb165a7c3b44e155c0f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"b6a57807_93326488","updated":"2024-11-19 13:58:01.000000000","message":"The updated implementation looks better to me but I still think that 400 is not appropriate return code for errors caused by server side configuration.","commit_id":"974867877b5de189a123b6712780a19a92ab058d"},{"author":{"_account_id":4264,"name":"Matthias Runge","email":"mrunge@redhat.com","username":"mrunge"},"change_message_id":"d79267048886fca6ac1276740a44bb0a19a4724a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"79f0962a_96b8f7fe","updated":"2024-11-08 07:52:07.000000000","message":"thank you","commit_id":"974867877b5de189a123b6712780a19a92ab058d"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"07332194bfa1c03d7c5efb3a606ffcc756e5d64e","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":10,"id":"c96fe416_35b65449","updated":"2024-11-20 15:57:48.000000000","message":"Sorry for another late catch but could you add a release note ? I can promote my vote if you submit it as a follow-up","commit_id":"13f52aa2f77fb66107eae346fdd782c9c2131a83"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"43a96ee92168c68d8dfe92e275a3d5f219beaf2d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"12aca91d_cbd5378f","in_reply_to":"b93497de_ba8f2718","updated":"2024-11-22 11:44:47.000000000","message":"Done","commit_id":"13f52aa2f77fb66107eae346fdd782c9c2131a83"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"2fe4ae483cf2fae0e6112a421bf80d4daddbeeab","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":10,"id":"b93497de_ba8f2718","in_reply_to":"c96fe416_35b65449","updated":"2024-11-20 21:28:59.000000000","message":"Sure. I\u0027m on it.","commit_id":"13f52aa2f77fb66107eae346fdd782c9c2131a83"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"b6bd0fa2c38eeb2bcee1f8bd2d6a7f7fc165f9d0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"b9b5f721_661d21a4","updated":"2025-01-13 15:49:42.000000000","message":"@mmagr@redhat.com suggested to split the data by individual alarms instead of aggregating per project. Aggregation can be done later when querying the metrics from the metric store. Splitting by individual alarms could allow for easier troubleshooting. For example if an alarm is flapping between \"insufficient data\" and other states, it could be immediately apparent which alarm is doing that this way.","commit_id":"52fa96700d47724eef1c3cb80598657000e5a7ad"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"43a96ee92168c68d8dfe92e275a3d5f219beaf2d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"c7c985b5_04f286fe","updated":"2024-11-22 11:44:47.000000000","message":"While we still have to find out the way to purge records for deleted projects, I\u0027m ok to move this forward with leaving it for follow-up","commit_id":"52fa96700d47724eef1c3cb80598657000e5a7ad"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"087f4858a9dc41b061ddfbe66e59d498bc0caa90","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"1c8c6fbe_1eb18438","updated":"2025-01-16 18:42:17.000000000","message":"I split the metrics by individual alarms as said in my previous comment. This also allowed me to nicely implement the record deletion as requested by Takashi. When an alarm is deleted, the counters get deleted as well.","commit_id":"b3fbe13b305f354e7b3b6b2ecd091fefcbb13ede"},{"author":{"_account_id":4264,"name":"Matthias Runge","email":"mrunge@redhat.com","username":"mrunge"},"change_message_id":"7a184e084e91f53860b9eccaaca613e0d32be36e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"3635a65c_a8e801bf","updated":"2025-02-05 10:40:24.000000000","message":"recheck","commit_id":"9ab5fb2873a7f8b1abc7e0e8284bad877b25132d"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"076f86a69a2e2b7a5b5a28a43ba73c8d79c0a567","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"5676eeb2_cc4581b8","updated":"2025-02-25 10:25:05.000000000","message":"I\u0027ll revisit the change later, but am leaving some comments to clear some comments about alembic related changes I\u0027ve made.","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"03d57e0b8b7b6f49573d13b5822660cc99abba42","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"1e03bd3f_ee2867af","updated":"2025-02-25 10:41:08.000000000","message":"Sorry for my late feedback, but I think we should clear the concern caused by loose relation between alarm and counter, which may lead stale counter records.","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"},{"author":{"_account_id":4264,"name":"Matthias Runge","email":"mrunge@redhat.com","username":"mrunge"},"change_message_id":"19db027942878590e96c746f8dc30bba2dad3890","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"438f5d54_08306105","updated":"2025-02-20 16:19:19.000000000","message":"The only change between patchset 14 and 15 is the updated commit message. Still looks good to me, thanks a lot!","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"dffad7b1dce030c4a6bdbc9355089676561c9a06","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":15,"id":"55263644_2b3f07d0","in_reply_to":"130929a5_5aa23a79","updated":"2025-03-03 10:53:52.000000000","message":"I\u0027ve submitted https://review.opendev.org/c/openstack/aodh/+/943169 which may address most of my comments. Feel free to squash that into this.","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"8cf1cc8578a8d40d85a525fd5eaa9c1112d741e9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"130929a5_5aa23a79","in_reply_to":"1e03bd3f_ee2867af","updated":"2025-03-03 10:11:08.000000000","message":"Thank you for the suggestions and sorry for late reply as well, it was a busy week. I plan to get to implementing your suggestions during this week.","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"028ad8cb11f78887846d655d9d4ffa1c09619824","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":15,"id":"655b9618_516683cd","in_reply_to":"55263644_2b3f07d0","updated":"2025-03-04 15:14:58.000000000","message":"Thank you for the patch, I used most of it in the latest PS. The 2 differences between your patch and PS 16 are:\n\n- I didn\u0027t include most of the changes in the aodh/storage/sqlalchemy/models.py . They aren\u0027t really related to this change and they should go into a separate change.\n\n- I modified the tests to work with an actual alarm to satisfy the foreign key constraint","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"e1ecafa78b9cf7871476ea1ae20fe7e21782745f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":15,"id":"175b7c21_25a26caf","in_reply_to":"655b9618_516683cd","updated":"2025-03-06 14:37:09.000000000","message":"Thanks !","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"62c1db79c3eabe510931d01833881a4b51ecedcd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"f3944683_6541565b","updated":"2025-04-09 09:04:32.000000000","message":"@kajinamit@oss.nttdata.com I think Epoxy is now fully GAd right? So this could move forward now?","commit_id":"1643abed422374733b3e73b4dcc701fd51114b84"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"e1ecafa78b9cf7871476ea1ae20fe7e21782745f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":16,"id":"29a5bfbc_6464aa1b","updated":"2025-03-06 14:37:09.000000000","message":"The current version looks good to me.\nOn the other hand I wonder if we should consider pushing this to the next cycle, we already passed the feature freeze date for Epoxy. I know that Telemetry may not really strictly follow the guidance and we may merge this one and the subsequent one till the code freeze coming next week. However note that we can\u0027t merge the python-aodhclient change for Epoxy because we already passed the client lib freeze a few weeks ago.\n\nhttps://releases.openstack.org/epoxy/schedule.html","commit_id":"1643abed422374733b3e73b4dcc701fd51114b84"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"52bb46635af33ab7ef66e33239c20a0147390689","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":16,"id":"d0d5a5e3_3644de12","in_reply_to":"0258478f_f77716e2","updated":"2025-03-18 09:08:51.000000000","message":"Acknowledged","commit_id":"1643abed422374733b3e73b4dcc701fd51114b84"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"5cfa4c940a94d4e2ea241c24f5bec00d43b66498","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":16,"id":"75e0bf12_0066bfd4","in_reply_to":"02942600_38de7489","updated":"2025-03-18 08:57:08.000000000","message":"I wonder what\u0027s the timeline around the release here. I noticed the stable/2025.1 branches are created now, so are we free to start merging now? Or should we wait some more time?","commit_id":"1643abed422374733b3e73b4dcc701fd51114b84"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"1161494aab49ae5955e97d61c604a63791b67a3f","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":16,"id":"02942600_38de7489","in_reply_to":"29a5bfbc_6464aa1b","updated":"2025-03-06 17:53:56.000000000","message":"I\u0027m quite OK with waiting for the F release. Especially if we need to wait with the aodhclient. So let\u0027s wait.","commit_id":"1643abed422374733b3e73b4dcc701fd51114b84"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"a651404feec135f83fffbf99e6f4d370f599c318","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":16,"id":"0258478f_f77716e2","in_reply_to":"75e0bf12_0066bfd4","updated":"2025-03-18 09:05:59.000000000","message":"Technically the master branch is open for development for 2025.2, but I tend to keep this open until GA so that we can do backport for critical bug fixes we may need for 2025.2 release more easily.","commit_id":"1643abed422374733b3e73b4dcc701fd51114b84"}],"aodh/api/controllers/v2/metrics.py":[{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"fcd01c537313cfda13e1803e3cafb8d074e9a0bc","unresolved":true,"context_lines":[{"line_number":57,"context_line":" ))"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":" project_id \u003d pecan.request.headers.get(\u0027X-Project-Id\u0027)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":" content \u003d MetricsOutput()"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":" LOG.debug(\u0027Getting evaluation result counters from database\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"92f1d25c_790626df","line":60,"updated":"2024-10-02 14:10:31.000000000","message":"We need access controls here.","commit_id":"c6ad61152cadd57e95117d0ca5e9c6c8eda3c85c"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"55b19179fde4d952ec76639e281781b912707d31","unresolved":false,"context_lines":[{"line_number":57,"context_line":" ))"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":" project_id \u003d pecan.request.headers.get(\u0027X-Project-Id\u0027)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":" content \u003d MetricsOutput()"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":" LOG.debug(\u0027Getting evaluation result counters from database\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"8de66086_0082857f","line":60,"in_reply_to":"416b0bdb_908c1e3c","updated":"2024-10-04 09:01:31.000000000","message":"Thanks for looking into this. Based on our discussion, looks like pecan is handling this already correctly and the user cannot override the project id.","commit_id":"c6ad61152cadd57e95117d0ca5e9c6c8eda3c85c"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"42434200a0e3cc38fc6d5d517402cf58b9ffb669","unresolved":true,"context_lines":[{"line_number":57,"context_line":" ))"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":" project_id \u003d pecan.request.headers.get(\u0027X-Project-Id\u0027)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":" content \u003d MetricsOutput()"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":" LOG.debug(\u0027Getting evaluation result counters from database\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"416b0bdb_908c1e3c","line":60,"in_reply_to":"47e87c2f_f05a464d","updated":"2024-10-03 15:04:00.000000000","message":"I\u0027m pretty sure if I\u0027m reading this correctly that you can pass any valid project id at X-Project-Id and any valid token and get other projects metrics from it. At least I don\u0027t see any other reason why, say v2/alarms.py has full rbac handling [0][1] attached to the API calls.\n\n[0] https://github.com/openstack/aodh/blob/master/aodh/api/controllers/v2/alarms.py#L577-L593\n[1] https://github.com/openstack/aodh/blob/master/aodh/api/controllers/v2/alarms.py#L659","commit_id":"c6ad61152cadd57e95117d0ca5e9c6c8eda3c85c"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"bede010080878f4ecb5fa1790d0ce6bf405f44c3","unresolved":true,"context_lines":[{"line_number":57,"context_line":" ))"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":" project_id \u003d pecan.request.headers.get(\u0027X-Project-Id\u0027)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":" content \u003d MetricsOutput()"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":" LOG.debug(\u0027Getting evaluation result counters from database\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"47e87c2f_f05a464d","line":60,"in_reply_to":"92f1d25c_790626df","updated":"2024-10-02 19:42:37.000000000","message":"I\u0027m not sure what access controls you mean. In the following link you can see how I tested it and the results. First I tried without authentication and it returned Unauthorized. Then I tested with 2 different projects (notice that the 2 tokens are different). I got 2 different result, which is expected as the counters are separate for each project.\n\nhttps://paste.opendev.org/show/bzP9LCBxD7gEtaN4rNDs/","commit_id":"c6ad61152cadd57e95117d0ca5e9c6c8eda3c85c"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"eff3bffb842ce55eb2f775bb8e496aba1fd8fc8c","unresolved":true,"context_lines":[{"line_number":49,"context_line":" \"\"\"Manages operations on the alarms collection.\"\"\""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":" @wsme_pecan.wsexpose(MetricsOutput)"},{"line_number":52,"context_line":" def get_all(self):"},{"line_number":53,"context_line":" \"\"\"Return all metrics\"\"\""},{"line_number":54,"context_line":" if not pecan.request.cfg.enable_metrics:"},{"line_number":55,"context_line":" raise base.ClientSideError(_("}],"source_content_type":"text/x-python","patch_set":4,"id":"24cad397_19deabfb","line":52,"range":{"start_line":52,"start_character":8,"end_line":52,"end_character":15},"updated":"2024-10-16 10:20:27.000000000","message":"The current implementation lacks policy check so this exposes any data from any projects to any user.","commit_id":"d69b1c0b330295517003408b4a01d722d8666da3"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"6ce9895fb93889725024be9e8682db851c95bfc4","unresolved":true,"context_lines":[{"line_number":49,"context_line":" \"\"\"Manages operations on the alarms collection.\"\"\""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":" @wsme_pecan.wsexpose(MetricsOutput)"},{"line_number":52,"context_line":" def get_all(self):"},{"line_number":53,"context_line":" \"\"\"Return all metrics\"\"\""},{"line_number":54,"context_line":" if not pecan.request.cfg.enable_metrics:"},{"line_number":55,"context_line":" raise base.ClientSideError(_("}],"source_content_type":"text/x-python","patch_set":4,"id":"65d983b1_84683c5a","line":52,"range":{"start_line":52,"start_character":8,"end_line":52,"end_character":15},"in_reply_to":"24cad397_19deabfb","updated":"2024-10-16 10:43:45.000000000","message":"See https://github.com/openstack/aodh/blob/master/aodh/api/controllers/v2/alarms.py#L866-L869 to find an example of policy check implementation.","commit_id":"d69b1c0b330295517003408b4a01d722d8666da3"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"965d312e2d1586c4b87d0bb165a7c3b44e155c0f","unresolved":false,"context_lines":[{"line_number":49,"context_line":" \"\"\"Manages operations on the alarms collection.\"\"\""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":" @wsme_pecan.wsexpose(MetricsOutput)"},{"line_number":52,"context_line":" def get_all(self):"},{"line_number":53,"context_line":" \"\"\"Return all metrics\"\"\""},{"line_number":54,"context_line":" if not pecan.request.cfg.enable_metrics:"},{"line_number":55,"context_line":" raise base.ClientSideError(_("}],"source_content_type":"text/x-python","patch_set":4,"id":"c94a61f4_d9643408","line":52,"range":{"start_line":52,"start_character":8,"end_line":52,"end_character":15},"in_reply_to":"3f69d0e0_3c4bcacf","updated":"2024-11-19 13:58:01.000000000","message":"\u003e I determined, that a project from the \"X-Auth-Token\" will always get used, which seems correct.\nX-Project-Id header does not necessarily match the token\u0027s project scope. For example in case a token is un-scoped then the header given by the user is directly passed. So we should ensure that we also check the context, which is built from the environment passed by keystonemiddleware.\n\nYou are correct about the new policy and the current implementation looks good to me.","commit_id":"d69b1c0b330295517003408b4a01d722d8666da3"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"24e766b29b3f109355aaaf95743376fa6a6f78de","unresolved":true,"context_lines":[{"line_number":49,"context_line":" \"\"\"Manages operations on the alarms collection.\"\"\""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":" @wsme_pecan.wsexpose(MetricsOutput)"},{"line_number":52,"context_line":" def get_all(self):"},{"line_number":53,"context_line":" \"\"\"Return all metrics\"\"\""},{"line_number":54,"context_line":" if not pecan.request.cfg.enable_metrics:"},{"line_number":55,"context_line":" raise base.ClientSideError(_("}],"source_content_type":"text/x-python","patch_set":4,"id":"3f69d0e0_3c4bcacf","line":52,"range":{"start_line":52,"start_character":8,"end_line":52,"end_character":15},"in_reply_to":"65d983b1_84683c5a","updated":"2024-10-16 11:56:50.000000000","message":"This kind of touches Erno\u0027s concerns and our discussion. Erno wasn\u0027t sure what would happen if a request had an \"X-Auth-Token\" and \"X-Projec-Id\" headers both at once. I determined, that a project from the \"X-Auth-Token\" will always get used, which seems correct. So the user can\u0027t use a token from project A to get metrics for project B.\n\nYour concern is, that a user could create a token for project C, where he doesn\u0027t have any permissions and still get metrics? Do I understand it correctly? Working with policies and RBAC in general is a little outside of my knowledge, so I\u0027ll probably need some help. Looking at the example you posted and looking further in the code. Here: https://github.com/openstack/aodh/blob/master/aodh/api/rbac.py#L49 I see, that I\u0027ll need to provide a policy name. Since we didn\u0027t have metrics before, my guess is that I\u0027ll need a new policy? I found this file: https://github.com/openstack/aodh/blob/master/aodh/api/policies.py where I guess I could create a new policy, but they\u0027re all deprecated, so I guess I\u0027ll need to do something different?","commit_id":"d69b1c0b330295517003408b4a01d722d8666da3"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"6ce9895fb93889725024be9e8682db851c95bfc4","unresolved":true,"context_lines":[{"line_number":52,"context_line":" def get_all(self):"},{"line_number":53,"context_line":" \"\"\"Return all metrics\"\"\""},{"line_number":54,"context_line":" if not pecan.request.cfg.enable_metrics:"},{"line_number":55,"context_line":" raise base.ClientSideError(_("},{"line_number":56,"context_line":" \"metrics are disabled in aodh configuration\""},{"line_number":57,"context_line":" ))"},{"line_number":58,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"d1960f74_830fcc52","line":55,"range":{"start_line":55,"start_character":23,"end_line":55,"end_character":38},"updated":"2024-10-16 10:43:45.000000000","message":"Should we return 403 rather than 400 ?","commit_id":"d69b1c0b330295517003408b4a01d722d8666da3"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"68d42e31edbfb49a24d729dfede15a54746e7a53","unresolved":false,"context_lines":[{"line_number":52,"context_line":" def get_all(self):"},{"line_number":53,"context_line":" \"\"\"Return all metrics\"\"\""},{"line_number":54,"context_line":" if not pecan.request.cfg.enable_metrics:"},{"line_number":55,"context_line":" raise base.ClientSideError(_("},{"line_number":56,"context_line":" \"metrics are disabled in aodh configuration\""},{"line_number":57,"context_line":" ))"},{"line_number":58,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"7cb56e61_269d175b","line":55,"range":{"start_line":55,"start_character":23,"end_line":55,"end_character":38},"in_reply_to":"07b417c8_ab02e01a","updated":"2024-11-19 14:54:18.000000000","message":"I replaced 400 by 403 as part of the change to add alembic script but feel free to switch it to 404 if you believe that\u0027s better.","commit_id":"d69b1c0b330295517003408b4a01d722d8666da3"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"965d312e2d1586c4b87d0bb165a7c3b44e155c0f","unresolved":true,"context_lines":[{"line_number":52,"context_line":" def get_all(self):"},{"line_number":53,"context_line":" \"\"\"Return all metrics\"\"\""},{"line_number":54,"context_line":" if not pecan.request.cfg.enable_metrics:"},{"line_number":55,"context_line":" raise base.ClientSideError(_("},{"line_number":56,"context_line":" \"metrics are disabled in aodh configuration\""},{"line_number":57,"context_line":" ))"},{"line_number":58,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"95ac262c_a8c005f3","line":55,"range":{"start_line":55,"start_character":23,"end_line":55,"end_character":38},"in_reply_to":"79819112_ef9b0810","updated":"2024-11-19 13:58:01.000000000","message":"yes I meant to use \"403 forbidden\"\n\n400 indicates users give a wrong input and request should succeed if they give the right input. For this case users can\u0027t use the feature because of the setting in the server side so 400 is not appropriate.\n\nFor example in heat we return 403 for abandon request in case abandon feature is disabled, and I think this is the common pattern we have.","commit_id":"d69b1c0b330295517003408b4a01d722d8666da3"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"43a4d200fe787e342702d7eca1d610dde4ef1e1c","unresolved":false,"context_lines":[{"line_number":52,"context_line":" def get_all(self):"},{"line_number":53,"context_line":" \"\"\"Return all metrics\"\"\""},{"line_number":54,"context_line":" if not pecan.request.cfg.enable_metrics:"},{"line_number":55,"context_line":" raise base.ClientSideError(_("},{"line_number":56,"context_line":" \"metrics are disabled in aodh configuration\""},{"line_number":57,"context_line":" ))"},{"line_number":58,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"0e27ecb3_20903aa0","line":55,"range":{"start_line":55,"start_character":23,"end_line":55,"end_character":38},"in_reply_to":"7cb56e61_269d175b","updated":"2024-11-20 13:59:15.000000000","message":"Reading your comments, the 400 indeed doesn\u0027t make sense. Let\u0027s keep the pattern and use 403 then. Thanks for implementing the change.","commit_id":"d69b1c0b330295517003408b4a01d722d8666da3"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"317b3c10173410f20f83fc42adc7e3affc154479","unresolved":true,"context_lines":[{"line_number":52,"context_line":" def get_all(self):"},{"line_number":53,"context_line":" \"\"\"Return all metrics\"\"\""},{"line_number":54,"context_line":" if not pecan.request.cfg.enable_metrics:"},{"line_number":55,"context_line":" raise base.ClientSideError(_("},{"line_number":56,"context_line":" \"metrics are disabled in aodh configuration\""},{"line_number":57,"context_line":" ))"},{"line_number":58,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"07b417c8_ab02e01a","line":55,"range":{"start_line":55,"start_character":23,"end_line":55,"end_character":38},"in_reply_to":"95ac262c_a8c005f3","updated":"2024-11-19 14:01:40.000000000","message":"RFC says\n\n400 Bad Request\n\nThe request could not be understood by the server due to malformed syntax.The client SHOULD NOT repeat the request without modifications.\n\n403 Forbidden\n\nThe server understood the request, but is refusing to fulfill it. Authorization will not help and the request SHOULD NOT be repeated. If the request method was not HEAD and the server wishes to make public why the request has not been fulfilled, it SHOULD describe the reason for the refusal in the entity. If the server does not wish to make this information available to the client, the status code 404 (Not Found) can be used instead.\n\nSo 403 or 404 is correct.","commit_id":"d69b1c0b330295517003408b4a01d722d8666da3"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"24e766b29b3f109355aaaf95743376fa6a6f78de","unresolved":true,"context_lines":[{"line_number":52,"context_line":" def get_all(self):"},{"line_number":53,"context_line":" \"\"\"Return all metrics\"\"\""},{"line_number":54,"context_line":" if not pecan.request.cfg.enable_metrics:"},{"line_number":55,"context_line":" raise base.ClientSideError(_("},{"line_number":56,"context_line":" \"metrics are disabled in aodh configuration\""},{"line_number":57,"context_line":" ))"},{"line_number":58,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"79819112_ef9b0810","line":55,"range":{"start_line":55,"start_character":23,"end_line":55,"end_character":38},"in_reply_to":"d1960f74_830fcc52","updated":"2024-10-16 11:56:50.000000000","message":"403 means forbidden right? This doesn\u0027t make much sense to me in this situation. This exception is raised when a client tries to reach the URL while the functionality is disabled in the config. It doesn\u0027t check any authorization. If we don\u0027t want to use 400 then maybe 404 would make more sense to me?","commit_id":"d69b1c0b330295517003408b4a01d722d8666da3"}],"aodh/opts.py":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"20c5f67245b5559296b4c616b59280c9a7edee65","unresolved":true,"context_lines":[{"line_number":33,"context_line":"import aodh.service"},{"line_number":34,"context_line":"import aodh.storage"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"OPTS \u003d ["},{"line_number":37,"context_line":" cfg.BoolOpt(\u0027enable_metrics\u0027,"},{"line_number":38,"context_line":" default\u003dFalse,"},{"line_number":39,"context_line":" help\u003d(\"Whether metric collection should be enabled.\")),"},{"line_number":40,"context_line":"]"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"def list_opts():"},{"line_number":44,"context_line":" return ["}],"source_content_type":"text/x-python","patch_set":6,"id":"96fa8e68_9348d8c5","line":41,"range":{"start_line":36,"start_character":0,"end_line":41,"end_character":1},"updated":"2024-11-19 14:14:40.000000000","message":"Can we add this to aodh.api.OPTS, (which means we register this option under [api] section) because the option is used only by api ?","commit_id":"974867877b5de189a123b6712780a19a92ab058d"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"a588ccafdc1189bc4f10f6d34d2ecd29e9b81c2e","unresolved":false,"context_lines":[{"line_number":33,"context_line":"import aodh.service"},{"line_number":34,"context_line":"import aodh.storage"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"OPTS \u003d ["},{"line_number":37,"context_line":" cfg.BoolOpt(\u0027enable_metrics\u0027,"},{"line_number":38,"context_line":" default\u003dFalse,"},{"line_number":39,"context_line":" help\u003d(\"Whether metric collection should be enabled.\")),"},{"line_number":40,"context_line":"]"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"def list_opts():"},{"line_number":44,"context_line":" return ["}],"source_content_type":"text/x-python","patch_set":6,"id":"a6b8f241_508d0cc9","line":41,"range":{"start_line":36,"start_character":0,"end_line":41,"end_character":1},"in_reply_to":"96fa8e68_9348d8c5","updated":"2024-11-19 14:15:44.000000000","message":"Ugh. Ignore it. I noticed that I was wrong.","commit_id":"974867877b5de189a123b6712780a19a92ab058d"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"112f430bc1e1283c72d9ea900e160d17c109c11d","unresolved":true,"context_lines":[{"line_number":34,"context_line":"import aodh.storage"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"OPTS \u003d ["},{"line_number":37,"context_line":" cfg.BoolOpt(\u0027enable_metrics\u0027,"},{"line_number":38,"context_line":" default\u003dFalse,"},{"line_number":39,"context_line":" help\u003d(\"Whether metric collection should be enabled.\")),"},{"line_number":40,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":13,"id":"ea90e82d_2c98de2d","line":37,"range":{"start_line":37,"start_character":17,"end_line":37,"end_character":31},"updated":"2025-01-23 09:32:43.000000000","message":"Taken our discussions about service metrics of different granularity, I think we should call this \"enable_granular_alarm_metrics\" or something among those lines. Should there be implementation of service metrics in future, I\u0027m pretty sure these are something operators will want to control individually depending of their environment (think about logging lvl DEBUG, INFO vs. WARN) and tying project specific per alarm metrics under \"enable_metrics\" config option, will very much limit our ability to separate these in non-confusing way.\n\nIf we have a need to ever get granular metrics configuration options, \"enable_metrics\" should be reserved to be blanket do/do not process any and all metrics.","commit_id":"780bcca10b126725c1863de8c060fa02881a00b1"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"68c3cec026e263d11c845d704cef2170fef3f210","unresolved":false,"context_lines":[{"line_number":34,"context_line":"import aodh.storage"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"OPTS \u003d ["},{"line_number":37,"context_line":" cfg.BoolOpt(\u0027enable_metrics\u0027,"},{"line_number":38,"context_line":" default\u003dFalse,"},{"line_number":39,"context_line":" help\u003d(\"Whether metric collection should be enabled.\")),"},{"line_number":40,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":13,"id":"b9d54429_b71ba96f","line":37,"range":{"start_line":37,"start_character":17,"end_line":37,"end_character":31},"in_reply_to":"6def2c0c_62ea9b7f","updated":"2025-01-29 08:52:07.000000000","message":"Done","commit_id":"780bcca10b126725c1863de8c060fa02881a00b1"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"b89738597caca19361dcb1e7ed6e94d61653e95d","unresolved":true,"context_lines":[{"line_number":34,"context_line":"import aodh.storage"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"OPTS \u003d ["},{"line_number":37,"context_line":" cfg.BoolOpt(\u0027enable_metrics\u0027,"},{"line_number":38,"context_line":" default\u003dFalse,"},{"line_number":39,"context_line":" help\u003d(\"Whether metric collection should be enabled.\")),"},{"line_number":40,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":13,"id":"6def2c0c_62ea9b7f","line":37,"range":{"start_line":37,"start_character":17,"end_line":37,"end_character":31},"in_reply_to":"8d021292_519fef18","updated":"2025-01-23 11:18:06.000000000","message":"Yeah I like the later or at least not implementing the \"master switch\" as long as we have only one subtype.\n\nHappy with any related naming too as long as it\u0027s more descriptive than the catch all \"enable_metrics\"","commit_id":"780bcca10b126725c1863de8c060fa02881a00b1"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"a83b8daf0b6abe1c431ed587064c450df84af3b7","unresolved":true,"context_lines":[{"line_number":34,"context_line":"import aodh.storage"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"OPTS \u003d ["},{"line_number":37,"context_line":" cfg.BoolOpt(\u0027enable_metrics\u0027,"},{"line_number":38,"context_line":" default\u003dFalse,"},{"line_number":39,"context_line":" help\u003d(\"Whether metric collection should be enabled.\")),"},{"line_number":40,"context_line":"]"}],"source_content_type":"text/x-python","patch_set":13,"id":"8d021292_519fef18","line":37,"range":{"start_line":37,"start_character":17,"end_line":37,"end_character":31},"in_reply_to":"ea90e82d_2c98de2d","updated":"2025-01-23 10:30:31.000000000","message":"The field reported by the /metrics endpoint is called \"evaluation\\_results\", so maybe calling it \"enable\\_evaluation\\_results\\_metrics\" would make more sense. I see 2 ways to implement your suggestion. Either we have \"enable\\_metrics\" as a master switch to enable and disable all kinds of metrics as well as the /metrics API endpoint and an \"enable\\_evaluation\\_results\\_metrics\" as a switch to enable and disable this particular metric. Both would need to be set to true for this feature to work.\n\nOr maybe we could do without a \"master switch\". We could have only the \"enable\\_evaluation\\_results\\_metrics\" for now and if in the future, if there are other metric types, we\u0027d have other \"enable\\_\u003cmetric\\_type\u003e\\_metrics\" options and the /metrics API endpoint would get enabled when at least one \"enable\\_\u003cmetric\\_type\u003e\\_metrics\" option is set to true.","commit_id":"780bcca10b126725c1863de8c060fa02881a00b1"}],"aodh/storage/impl_log.py":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"03d57e0b8b7b6f49573d13b5822660cc99abba42","unresolved":true,"context_lines":[{"line_number":58,"context_line":" @staticmethod"},{"line_number":59,"context_line":" def increment_counter(alarm_id, project_id, state):"},{"line_number":60,"context_line":" \"\"\"Increment counter.\"\"\""},{"line_number":61,"context_line":" return state"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":" def get_counters(alarm_id, project_id, state):"},{"line_number":64,"context_line":" \"\"\"Get value of a counter.\"\"\""}],"source_content_type":"text/x-python","patch_set":15,"id":"f6e14a03_031d0c4a","line":61,"range":{"start_line":61,"start_character":8,"end_line":61,"end_character":14},"updated":"2025-02-25 10:41:08.000000000","message":"I\u0027d suggest raising aodh.NotImplementedError instead, because the feature is not actually implemented.","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"9233540c9fa8aa0d83cffaaee134f8904ba18c3e","unresolved":true,"context_lines":[{"line_number":58,"context_line":" @staticmethod"},{"line_number":59,"context_line":" def increment_counter(alarm_id, project_id, state):"},{"line_number":60,"context_line":" \"\"\"Increment counter.\"\"\""},{"line_number":61,"context_line":" return state"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":" def get_counters(alarm_id, project_id, state):"},{"line_number":64,"context_line":" \"\"\"Get value of a counter.\"\"\""}],"source_content_type":"text/x-python","patch_set":15,"id":"fe052458_201dda85","line":61,"range":{"start_line":61,"start_character":8,"end_line":61,"end_character":14},"in_reply_to":"f6e14a03_031d0c4a","updated":"2025-03-03 10:32:18.000000000","message":"This may require you to move if check from\n https://review.opendev.org/c/openstack/aodh/+/925775/15/aodh/storage/impl_sqlalchemy.py#339\n\nto\n https://review.opendev.org/c/openstack/aodh/+/925775/15/aodh/evaluator/__init__.py#116\n \notherwise the log driver can\u0027t be used.","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"028ad8cb11f78887846d655d9d4ffa1c09619824","unresolved":false,"context_lines":[{"line_number":58,"context_line":" @staticmethod"},{"line_number":59,"context_line":" def increment_counter(alarm_id, project_id, state):"},{"line_number":60,"context_line":" \"\"\"Increment counter.\"\"\""},{"line_number":61,"context_line":" return state"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":" def get_counters(alarm_id, project_id, state):"},{"line_number":64,"context_line":" \"\"\"Get value of a counter.\"\"\""}],"source_content_type":"text/x-python","patch_set":15,"id":"911bb752_2b9b10fa","line":61,"range":{"start_line":61,"start_character":8,"end_line":61,"end_character":14},"in_reply_to":"fe052458_201dda85","updated":"2025-03-04 15:14:58.000000000","message":"Done","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"03d57e0b8b7b6f49573d13b5822660cc99abba42","unresolved":true,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":" def get_counters(alarm_id, project_id, state):"},{"line_number":64,"context_line":" \"\"\"Get value of a counter.\"\"\""},{"line_number":65,"context_line":" return 0"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":" @staticmethod"},{"line_number":68,"context_line":" def clear_expired_alarm_history_data(ttl, max_count\u003dNone):"}],"source_content_type":"text/x-python","patch_set":15,"id":"fc97762d_12ecd4ca","line":65,"range":{"start_line":65,"start_character":8,"end_line":65,"end_character":14},"updated":"2025-02-25 10:41:08.000000000","message":"ditto","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"028ad8cb11f78887846d655d9d4ffa1c09619824","unresolved":false,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":" def get_counters(alarm_id, project_id, state):"},{"line_number":64,"context_line":" \"\"\"Get value of a counter.\"\"\""},{"line_number":65,"context_line":" return 0"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":" @staticmethod"},{"line_number":68,"context_line":" def clear_expired_alarm_history_data(ttl, max_count\u003dNone):"}],"source_content_type":"text/x-python","patch_set":15,"id":"37b22cb0_1d896a9a","line":65,"range":{"start_line":65,"start_character":8,"end_line":65,"end_character":14},"in_reply_to":"fc97762d_12ecd4ca","updated":"2025-03-04 15:14:58.000000000","message":"Done","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"}],"aodh/storage/impl_sqlalchemy.py":[{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"fcd01c537313cfda13e1803e3cafb8d074e9a0bc","unresolved":true,"context_lines":[{"line_number":341,"context_line":" LOG.debug(\"Resetting counter %(name)s\", {\u0027name\u0027: name})"},{"line_number":342,"context_line":" new_value \u003d 1"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":" if counter_value \u003d\u003d 0:"},{"line_number":345,"context_line":" # We have a new uninitialized counter"},{"line_number":346,"context_line":" counter \u003d models.Counter(name\u003dname, project_id\u003dproject_id)"},{"line_number":347,"context_line":" counter.update({\u0027value\u0027: new_value})"}],"source_content_type":"text/x-python","patch_set":3,"id":"d15b0395_de04edf7","line":344,"range":{"start_line":344,"start_character":12,"end_line":344,"end_character":14},"updated":"2024-10-02 14:10:31.000000000","message":"Just for readability it would be great if you used elif here as this and the above would never be both true.","commit_id":"c6ad61152cadd57e95117d0ca5e9c6c8eda3c85c"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"bede010080878f4ecb5fa1790d0ce6bf405f44c3","unresolved":false,"context_lines":[{"line_number":341,"context_line":" LOG.debug(\"Resetting counter %(name)s\", {\u0027name\u0027: name})"},{"line_number":342,"context_line":" new_value \u003d 1"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":" if counter_value \u003d\u003d 0:"},{"line_number":345,"context_line":" # We have a new uninitialized counter"},{"line_number":346,"context_line":" counter \u003d models.Counter(name\u003dname, project_id\u003dproject_id)"},{"line_number":347,"context_line":" counter.update({\u0027value\u0027: new_value})"}],"source_content_type":"text/x-python","patch_set":3,"id":"d012b16e_74c40de1","line":344,"range":{"start_line":344,"start_character":12,"end_line":344,"end_character":14},"in_reply_to":"d15b0395_de04edf7","updated":"2024-10-02 19:42:37.000000000","message":"Done","commit_id":"c6ad61152cadd57e95117d0ca5e9c6c8eda3c85c"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"db074cb5c6a2967402d5e020b03cb0ecd15b3431","unresolved":true,"context_lines":[{"line_number":343,"context_line":""},{"line_number":344,"context_line":" elif counter_value \u003d\u003d 0:"},{"line_number":345,"context_line":" # We have a new uninitialized counter"},{"line_number":346,"context_line":" counter \u003d models.Counter(name\u003dname, project_id\u003dproject_id)"},{"line_number":347,"context_line":" counter.update({\u0027value\u0027: new_value})"},{"line_number":348,"context_line":" session.add(counter)"},{"line_number":349,"context_line":" else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"614920cd_2eb2252e","line":346,"range":{"start_line":346,"start_character":26,"end_line":346,"end_character":74},"updated":"2024-11-20 06:02:37.000000000","message":"So a counter record is created here, but when is it deleted ? It looks like the current implementation lacks the trigger to purge old triggers and the table may grow infinitely.","commit_id":"a1f31c6750120f40da92bf9335511fd068a63289"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"4b10f84334a93549a779f8f8629cf31aa1d8bb2b","unresolved":false,"context_lines":[{"line_number":343,"context_line":""},{"line_number":344,"context_line":" elif counter_value \u003d\u003d 0:"},{"line_number":345,"context_line":" # We have a new uninitialized counter"},{"line_number":346,"context_line":" counter \u003d models.Counter(name\u003dname, project_id\u003dproject_id)"},{"line_number":347,"context_line":" counter.update({\u0027value\u0027: new_value})"},{"line_number":348,"context_line":" session.add(counter)"},{"line_number":349,"context_line":" else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"9276e548_ea644875","line":346,"range":{"start_line":346,"start_character":26,"end_line":346,"end_character":74},"in_reply_to":"05debf31_c3ee4fe6","updated":"2025-03-06 14:38:39.000000000","message":"I think we no longer need this as the counters are purged when the associated alarms are deleted. We might need to consider the mechanism to purge alarms for deleted projects but that\u0027s not a new problem brought by this change.","commit_id":"a1f31c6750120f40da92bf9335511fd068a63289"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"430e4ed87f89f57bf7ea84f81487fbb2eae895bf","unresolved":true,"context_lines":[{"line_number":343,"context_line":""},{"line_number":344,"context_line":" elif counter_value \u003d\u003d 0:"},{"line_number":345,"context_line":" # We have a new uninitialized counter"},{"line_number":346,"context_line":" counter \u003d models.Counter(name\u003dname, project_id\u003dproject_id)"},{"line_number":347,"context_line":" counter.update({\u0027value\u0027: new_value})"},{"line_number":348,"context_line":" session.add(counter)"},{"line_number":349,"context_line":" else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"05debf31_c3ee4fe6","line":346,"range":{"start_line":346,"start_character":26,"end_line":346,"end_character":74},"in_reply_to":"5ed74f47_c4d81cef","updated":"2024-11-20 14:46:27.000000000","message":"Adding the functionality to aodh-expire is a bit tricky because it requires connection to keystone. Probably the first option is to provide a tool (in aodh-dbsync ?) to purge the counters with specific project id but I\u0027m ok to leave it for follow-up","commit_id":"a1f31c6750120f40da92bf9335511fd068a63289"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"43a4d200fe787e342702d7eca1d610dde4ef1e1c","unresolved":true,"context_lines":[{"line_number":343,"context_line":""},{"line_number":344,"context_line":" elif counter_value \u003d\u003d 0:"},{"line_number":345,"context_line":" # We have a new uninitialized counter"},{"line_number":346,"context_line":" counter \u003d models.Counter(name\u003dname, project_id\u003dproject_id)"},{"line_number":347,"context_line":" counter.update({\u0027value\u0027: new_value})"},{"line_number":348,"context_line":" session.add(counter)"},{"line_number":349,"context_line":" else:"}],"source_content_type":"text/x-python","patch_set":9,"id":"5ed74f47_c4d81cef","line":346,"range":{"start_line":346,"start_character":26,"end_line":346,"end_character":74},"in_reply_to":"614920cd_2eb2252e","updated":"2024-11-20 13:59:15.000000000","message":"hmm. This is true. I expect there to be 3 counters per project, I don\u0027t think it makes much sense to delete them while the project exists and the enable_metrics is true. I guess we could delete the counters when enable_metrics gets set to false. I could also modify the aodh-expirer to delete counters from non-existing projects when it runs.","commit_id":"a1f31c6750120f40da92bf9335511fd068a63289"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"8d1c18cb31799e1742623345bdd842480da8c140","unresolved":true,"context_lines":[{"line_number":324,"context_line":" session.query(models.AlarmChange).filter("},{"line_number":325,"context_line":" models.AlarmChange.alarm_id \u003d\u003d alarm_id,"},{"line_number":326,"context_line":" ).delete()"},{"line_number":327,"context_line":" if self.conf.enable_evaluation_results_metrics:"},{"line_number":328,"context_line":" session.query(models.Counter).filter("},{"line_number":329,"context_line":" models.Counter.alarm_id \u003d\u003d alarm_id,"},{"line_number":330,"context_line":" ).delete()"}],"source_content_type":"text/x-python","patch_set":15,"id":"45d596e8_aad10e25","line":327,"range":{"start_line":327,"start_character":12,"end_line":327,"end_character":59},"updated":"2025-03-03 10:21:34.000000000","message":"I\u0027d suggest purging the counters regardless of this option, so that alarms can be deleted even after user once enable the feature and then disable it.","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"028ad8cb11f78887846d655d9d4ffa1c09619824","unresolved":false,"context_lines":[{"line_number":324,"context_line":" session.query(models.AlarmChange).filter("},{"line_number":325,"context_line":" models.AlarmChange.alarm_id \u003d\u003d alarm_id,"},{"line_number":326,"context_line":" ).delete()"},{"line_number":327,"context_line":" if self.conf.enable_evaluation_results_metrics:"},{"line_number":328,"context_line":" session.query(models.Counter).filter("},{"line_number":329,"context_line":" models.Counter.alarm_id \u003d\u003d alarm_id,"},{"line_number":330,"context_line":" ).delete()"}],"source_content_type":"text/x-python","patch_set":15,"id":"5fe00a74_d07cfbac","line":327,"range":{"start_line":327,"start_character":12,"end_line":327,"end_character":59},"in_reply_to":"45d596e8_aad10e25","updated":"2025-03-04 15:14:58.000000000","message":"Done","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"}],"aodh/storage/models.py":[{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"fcd01c537313cfda13e1803e3cafb8d074e9a0bc","unresolved":true,"context_lines":[{"line_number":164,"context_line":""},{"line_number":165,"context_line":"class Counter(base.Model):"},{"line_number":166,"context_line":" def __init__(self, name):"},{"line_number":167,"context_line":" base.Model.__init__(self, name\u003dname, value\u003d0)"}],"source_content_type":"text/x-python","patch_set":3,"id":"e061e36c_485c0631","line":167,"updated":"2024-10-02 14:10:31.000000000","message":"\u0027project_id\u0027 missing?","commit_id":"c6ad61152cadd57e95117d0ca5e9c6c8eda3c85c"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"bede010080878f4ecb5fa1790d0ce6bf405f44c3","unresolved":false,"context_lines":[{"line_number":164,"context_line":""},{"line_number":165,"context_line":"class Counter(base.Model):"},{"line_number":166,"context_line":" def __init__(self, name):"},{"line_number":167,"context_line":" base.Model.__init__(self, name\u003dname, value\u003d0)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3c0bcb30_67542b89","line":167,"in_reply_to":"e061e36c_485c0631","updated":"2024-10-02 19:42:37.000000000","message":"Done","commit_id":"c6ad61152cadd57e95117d0ca5e9c6c8eda3c85c"}],"aodh/storage/sqlalchemy/alembic/alembic.ini":[{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"112f430bc1e1283c72d9ea900e160d17c109c11d","unresolved":true,"context_lines":[{"line_number":1,"context_line":"[alembic]"},{"line_number":2,"context_line":"script_location \u003d aodh.storage.sqlalchemy:alembic"},{"line_number":3,"context_line":"sqlalchemy.url \u003d sqlite:///aodh.db"},{"line_number":4,"context_line":""},{"line_number":5,"context_line":"[loggers]"},{"line_number":6,"context_line":"keys \u003d root,sqlalchemy,alembic"}],"source_content_type":"text/x-properties","patch_set":13,"id":"e076b296_10d2d996","line":3,"range":{"start_line":3,"start_character":17,"end_line":3,"end_character":34},"updated":"2025-01-23 09:32:43.000000000","message":"What all does this addition actually affect?","commit_id":"780bcca10b126725c1863de8c060fa02881a00b1"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"4b10f84334a93549a779f8f8629cf31aa1d8bb2b","unresolved":false,"context_lines":[{"line_number":1,"context_line":"[alembic]"},{"line_number":2,"context_line":"script_location \u003d aodh.storage.sqlalchemy:alembic"},{"line_number":3,"context_line":"sqlalchemy.url \u003d sqlite:///aodh.db"},{"line_number":4,"context_line":""},{"line_number":5,"context_line":"[loggers]"},{"line_number":6,"context_line":"keys \u003d root,sqlalchemy,alembic"}],"source_content_type":"text/x-properties","patch_set":13,"id":"eb91518c_6abc971c","line":3,"range":{"start_line":3,"start_character":17,"end_line":3,"end_character":34},"in_reply_to":"641d0be4_9808c6ae","updated":"2025-03-06 14:38:39.000000000","message":"Done","commit_id":"780bcca10b126725c1863de8c060fa02881a00b1"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"076f86a69a2e2b7a5b5a28a43ba73c8d79c0a567","unresolved":true,"context_lines":[{"line_number":1,"context_line":"[alembic]"},{"line_number":2,"context_line":"script_location \u003d aodh.storage.sqlalchemy:alembic"},{"line_number":3,"context_line":"sqlalchemy.url \u003d sqlite:///aodh.db"},{"line_number":4,"context_line":""},{"line_number":5,"context_line":"[loggers]"},{"line_number":6,"context_line":"keys \u003d root,sqlalchemy,alembic"}],"source_content_type":"text/x-properties","patch_set":13,"id":"641d0be4_9808c6ae","line":3,"range":{"start_line":3,"start_character":17,"end_line":3,"end_character":34},"in_reply_to":"e076b296_10d2d996","updated":"2025-02-25 10:25:05.000000000","message":"This is used when we generate migration script by alembic.","commit_id":"780bcca10b126725c1863de8c060fa02881a00b1"}],"aodh/storage/sqlalchemy/alembic/env.py":[{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"112f430bc1e1283c72d9ea900e160d17c109c11d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":13,"id":"14ecc612_0d1ea2aa","updated":"2025-01-23 09:32:43.000000000","message":"I\u0027m trying ot figure out how these changes are relevant in the scope of this patch. Any insight?","commit_id":"780bcca10b126725c1863de8c060fa02881a00b1"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"a83b8daf0b6abe1c431ed587064c450df84af3b7","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":13,"id":"652391f0_ce2a6b3e","in_reply_to":"14ecc612_0d1ea2aa","updated":"2025-01-23 10:30:31.000000000","message":"These actually weren\u0027t done by me, but they were uploaded by @kajinamit@oss.nttdata.com . I don\u0027t know enough about sqlalchemy or alembic to comment on this, but it turned out to be quite helpful when generating the db upgrade scripts.","commit_id":"780bcca10b126725c1863de8c060fa02881a00b1"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"d883844eac6b640f41ab34ca13597434937710e2","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":13,"id":"d33117fd_71ec9a9e","in_reply_to":"2c670928_050b3203","updated":"2025-02-24 13:29:03.000000000","message":"@jokke@usr.fi or @kajinamit@oss.nttdata.com any comment?","commit_id":"780bcca10b126725c1863de8c060fa02881a00b1"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"b89738597caca19361dcb1e7ed6e94d61653e95d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":13,"id":"ddc31eec_1be4d8f0","in_reply_to":"652391f0_ce2a6b3e","updated":"2025-01-23 11:18:06.000000000","message":"Ok, maybe he will chime in. I\u0027m not the one normally asking to split changes that are logical unit of work, but there is no comments on the files nor anything in the commit message explaining the impact, scope or reasoning for those alembic changes making it quite difficult to review.","commit_id":"780bcca10b126725c1863de8c060fa02881a00b1"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"283effcd2e0b6ae891647bc946e9b5d2ed68a2a1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"d64bd185_f7b91070","in_reply_to":"894c9777_0eb9ebb4","updated":"2025-03-06 14:37:24.000000000","message":"Done","commit_id":"780bcca10b126725c1863de8c060fa02881a00b1"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"076f86a69a2e2b7a5b5a28a43ba73c8d79c0a567","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":13,"id":"894c9777_0eb9ebb4","in_reply_to":"d33117fd_71ec9a9e","updated":"2025-02-25 10:25:05.000000000","message":"These changes were required so that we could generate the migration script by alembic. These do not affect runtime behavior (thus we haven\u0027t caught the problem before)","commit_id":"780bcca10b126725c1863de8c060fa02881a00b1"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"48d18f7509dd9730204a9dd197a89696c754ba82","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":13,"id":"2c670928_050b3203","in_reply_to":"ddc31eec_1be4d8f0","updated":"2025-02-06 08:17:10.000000000","message":"This has been sitting here for quite some time now. If I split this and separate the aodh/storage/sqlalchemy/alemebic/alembic.ini and aodh/storage/sqlalchemy/alembic/env.py into a separate patch, which I move on top of the relation chain. Would it help to move this forward @jokke@usr.fi?","commit_id":"780bcca10b126725c1863de8c060fa02881a00b1"}],"aodh/storage/sqlalchemy/models.py":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"eddfa2c0346fdabbd0f2b767126cbbd1f094c0d2","unresolved":true,"context_lines":[{"line_number":143,"context_line":" limit \u003d Column(Integer)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"class Counter(Base):"},{"line_number":147,"context_line":" __tablename__ \u003d \u0027counter\u0027"},{"line_number":148,"context_line":" __table_args__ \u003d ("},{"line_number":149,"context_line":" sa.UniqueConstraint(\u0027project_id\u0027, \u0027name\u0027),"}],"source_content_type":"text/x-python","patch_set":6,"id":"0bf19f73_274e43dc","line":146,"range":{"start_line":146,"start_character":6,"end_line":146,"end_character":13},"updated":"2024-11-19 14:17:53.000000000","message":"so this introduces the new \"counter\" table, right? Then we should generate the alembic migration script to add the table.","commit_id":"974867877b5de189a123b6712780a19a92ab058d"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"68d42e31edbfb49a24d729dfede15a54746e7a53","unresolved":false,"context_lines":[{"line_number":143,"context_line":" limit \u003d Column(Integer)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"class Counter(Base):"},{"line_number":147,"context_line":" __tablename__ \u003d \u0027counter\u0027"},{"line_number":148,"context_line":" __table_args__ \u003d ("},{"line_number":149,"context_line":" sa.UniqueConstraint(\u0027project_id\u0027, \u0027name\u0027),"}],"source_content_type":"text/x-python","patch_set":6,"id":"edb29c3d_ddd473d0","line":146,"range":{"start_line":146,"start_character":6,"end_line":146,"end_character":13},"in_reply_to":"0bf19f73_274e43dc","updated":"2024-11-19 14:54:18.000000000","message":"Done","commit_id":"974867877b5de189a123b6712780a19a92ab058d"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"b07257c305392c0f4d869dfbc0d71108d3333769","unresolved":true,"context_lines":[{"line_number":152,"context_line":" )"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":" id \u003d Column(String(36), primary_key\u003dTrue, default\u003duuidutils.generate_uuid)"},{"line_number":155,"context_line":" name \u003d Column(String(128))"},{"line_number":156,"context_line":" project_id \u003d Column(String(128))"},{"line_number":157,"context_line":" value \u003d Column(Integer)"}],"source_content_type":"text/x-python","patch_set":7,"id":"8f27f444_64cc78f4","line":157,"range":{"start_line":155,"start_character":0,"end_line":157,"end_character":27},"updated":"2024-11-19 15:00:39.000000000","message":"Wondering if these should have nullable\u003dFalse ?","commit_id":"09c92726b0b69cd3d4f37610117349d5e44404fd"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"43a4d200fe787e342702d7eca1d610dde4ef1e1c","unresolved":true,"context_lines":[{"line_number":152,"context_line":" )"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":" id \u003d Column(String(36), primary_key\u003dTrue, default\u003duuidutils.generate_uuid)"},{"line_number":155,"context_line":" name \u003d Column(String(128))"},{"line_number":156,"context_line":" project_id \u003d Column(String(128))"},{"line_number":157,"context_line":" value \u003d Column(Integer)"}],"source_content_type":"text/x-python","patch_set":7,"id":"b78eadc2_cff919aa","line":157,"range":{"start_line":155,"start_character":0,"end_line":157,"end_character":27},"in_reply_to":"8f27f444_64cc78f4","updated":"2024-11-20 13:59:15.000000000","message":"Yes. This makes sense.","commit_id":"09c92726b0b69cd3d4f37610117349d5e44404fd"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"b7ad318153389c4e8d09c5fe763b9ee7cd35ad77","unresolved":false,"context_lines":[{"line_number":152,"context_line":" )"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":" id \u003d Column(String(36), primary_key\u003dTrue, default\u003duuidutils.generate_uuid)"},{"line_number":155,"context_line":" name \u003d Column(String(128))"},{"line_number":156,"context_line":" project_id \u003d Column(String(128))"},{"line_number":157,"context_line":" value \u003d Column(Integer)"}],"source_content_type":"text/x-python","patch_set":7,"id":"383e3b3f_b51aa4d9","line":157,"range":{"start_line":155,"start_character":0,"end_line":157,"end_character":27},"in_reply_to":"b78eadc2_cff919aa","updated":"2024-11-20 15:56:56.000000000","message":"Done","commit_id":"09c92726b0b69cd3d4f37610117349d5e44404fd"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"9233540c9fa8aa0d83cffaaee134f8904ba18c3e","unresolved":true,"context_lines":[{"line_number":143,"context_line":" limit \u003d Column(Integer, nullable\u003dFalse)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"class Counter(Base):"},{"line_number":147,"context_line":" __tablename__ \u003d \u0027counter\u0027"},{"line_number":148,"context_line":" __table_args__ \u003d ("},{"line_number":149,"context_line":" sa.UniqueConstraint(\u0027alarm_id\u0027, \u0027project_id\u0027, \u0027state\u0027),"}],"source_content_type":"text/x-python","patch_set":15,"id":"af17788c_21510e34","line":146,"range":{"start_line":146,"start_character":6,"end_line":146,"end_character":13},"updated":"2025-03-03 10:32:18.000000000","message":"Can we rename this to AlarmCounter so that this looks apparently specific to alarms ?","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"028ad8cb11f78887846d655d9d4ffa1c09619824","unresolved":false,"context_lines":[{"line_number":143,"context_line":" limit \u003d Column(Integer, nullable\u003dFalse)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"class Counter(Base):"},{"line_number":147,"context_line":" __tablename__ \u003d \u0027counter\u0027"},{"line_number":148,"context_line":" __table_args__ \u003d ("},{"line_number":149,"context_line":" sa.UniqueConstraint(\u0027alarm_id\u0027, \u0027project_id\u0027, \u0027state\u0027),"}],"source_content_type":"text/x-python","patch_set":15,"id":"eac089a7_ca9f1484","line":146,"range":{"start_line":146,"start_character":6,"end_line":146,"end_character":13},"in_reply_to":"af17788c_21510e34","updated":"2025-03-04 15:14:58.000000000","message":"Done","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"03d57e0b8b7b6f49573d13b5822660cc99abba42","unresolved":true,"context_lines":[{"line_number":154,"context_line":" )"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":" id \u003d Column(String(36), primary_key\u003dTrue, default\u003duuidutils.generate_uuid)"},{"line_number":157,"context_line":" alarm_id \u003d Column(String(128), nullable\u003dFalse)"},{"line_number":158,"context_line":" project_id \u003d Column(String(128), nullable\u003dFalse)"},{"line_number":159,"context_line":" state \u003d Column(String(128), nullable\u003dFalse)"},{"line_number":160,"context_line":" value \u003d Column(Integer, nullable\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":15,"id":"5379d927_1a1b3152","line":157,"range":{"start_line":157,"start_character":4,"end_line":157,"end_character":12},"updated":"2025-02-25 10:41:08.000000000","message":"You may need FK constraint. Also, because now counter is created per alarm, I think we should remove it when the alarm is deleted.","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"8cf1cc8578a8d40d85a525fd5eaa9c1112d741e9","unresolved":true,"context_lines":[{"line_number":154,"context_line":" )"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":" id \u003d Column(String(36), primary_key\u003dTrue, default\u003duuidutils.generate_uuid)"},{"line_number":157,"context_line":" alarm_id \u003d Column(String(128), nullable\u003dFalse)"},{"line_number":158,"context_line":" project_id \u003d Column(String(128), nullable\u003dFalse)"},{"line_number":159,"context_line":" state \u003d Column(String(128), nullable\u003dFalse)"},{"line_number":160,"context_line":" value \u003d Column(Integer, nullable\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":15,"id":"79778c92_da147861","line":157,"range":{"start_line":157,"start_character":4,"end_line":157,"end_character":12},"in_reply_to":"5379d927_1a1b3152","updated":"2025-03-03 10:11:08.000000000","message":"Regarding deleting the counters when alarm is deleted, it\u0027s being done here: https://review.opendev.org/c/openstack/aodh/+/925775/15/aodh/storage/impl_sqlalchemy.py#327","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"028ad8cb11f78887846d655d9d4ffa1c09619824","unresolved":false,"context_lines":[{"line_number":154,"context_line":" )"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":" id \u003d Column(String(36), primary_key\u003dTrue, default\u003duuidutils.generate_uuid)"},{"line_number":157,"context_line":" alarm_id \u003d Column(String(128), nullable\u003dFalse)"},{"line_number":158,"context_line":" project_id \u003d Column(String(128), nullable\u003dFalse)"},{"line_number":159,"context_line":" state \u003d Column(String(128), nullable\u003dFalse)"},{"line_number":160,"context_line":" value \u003d Column(Integer, nullable\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":15,"id":"53c07bae_9abde837","line":157,"range":{"start_line":157,"start_character":4,"end_line":157,"end_character":12},"in_reply_to":"63bbff64_3a8817a1","updated":"2025-03-04 15:14:58.000000000","message":"Done","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"15a2f282866b718da9f2e43481f364dfb644e4ee","unresolved":true,"context_lines":[{"line_number":154,"context_line":" )"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":" id \u003d Column(String(36), primary_key\u003dTrue, default\u003duuidutils.generate_uuid)"},{"line_number":157,"context_line":" alarm_id \u003d Column(String(128), nullable\u003dFalse)"},{"line_number":158,"context_line":" project_id \u003d Column(String(128), nullable\u003dFalse)"},{"line_number":159,"context_line":" state \u003d Column(String(128), nullable\u003dFalse)"},{"line_number":160,"context_line":" value \u003d Column(Integer, nullable\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":15,"id":"63bbff64_3a8817a1","line":157,"range":{"start_line":157,"start_character":4,"end_line":157,"end_character":12},"in_reply_to":"7015493d_ced042a7","updated":"2025-03-03 10:45:49.000000000","message":"Hmm so I noticed that the FK constraint is missing from AlarmHistory, which we may need to fix separately...","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"8d1c18cb31799e1742623345bdd842480da8c140","unresolved":true,"context_lines":[{"line_number":154,"context_line":" )"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":" id \u003d Column(String(36), primary_key\u003dTrue, default\u003duuidutils.generate_uuid)"},{"line_number":157,"context_line":" alarm_id \u003d Column(String(128), nullable\u003dFalse)"},{"line_number":158,"context_line":" project_id \u003d Column(String(128), nullable\u003dFalse)"},{"line_number":159,"context_line":" state \u003d Column(String(128), nullable\u003dFalse)"},{"line_number":160,"context_line":" value \u003d Column(Integer, nullable\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":15,"id":"7015493d_ced042a7","line":157,"range":{"start_line":157,"start_character":4,"end_line":157,"end_character":12},"in_reply_to":"79778c92_da147861","updated":"2025-03-03 10:21:34.000000000","message":"Ah, OK I overlooked it.","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"}],"releasenotes/notes/add-metrics-endpoint-and-evaluation-counter-collection-f324ebda00fa5c6c.yaml":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"03d57e0b8b7b6f49573d13b5822660cc99abba42","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":" - |"},{"line_number":4,"context_line":" Added collection of alarm evaluation counters. These show the number of times alarms were evaluated as ``alarm``, ``ok`` and ``insufficient data`` per alarm. These counters are presented by the /v2/metrics API endpoint. This feature can be enabled or disabled with the [DEFAULT].enable_evaluation_results_metrics configuration option. It\u0027s disabled by default."}],"source_content_type":"text/x-yaml","patch_set":15,"id":"5cd9261f_03719e3e","line":4,"range":{"start_line":4,"start_character":4,"end_line":4,"end_character":9},"updated":"2025-02-25 10:41:08.000000000","message":"Could you wrap lines by 79 characters for better readability in console ?","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"},{"author":{"_account_id":34975,"name":"Jaromír Wysoglad","email":"jwysogla@redhat.com","username":"jwysogla"},"change_message_id":"028ad8cb11f78887846d655d9d4ffa1c09619824","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":" - |"},{"line_number":4,"context_line":" Added collection of alarm evaluation counters. These show the number of times alarms were evaluated as ``alarm``, ``ok`` and ``insufficient data`` per alarm. These counters are presented by the /v2/metrics API endpoint. This feature can be enabled or disabled with the [DEFAULT].enable_evaluation_results_metrics configuration option. It\u0027s disabled by default."}],"source_content_type":"text/x-yaml","patch_set":15,"id":"b4c17af6_c8f8fafa","line":4,"range":{"start_line":4,"start_character":4,"end_line":4,"end_character":9},"in_reply_to":"5cd9261f_03719e3e","updated":"2025-03-04 15:14:58.000000000","message":"Done","commit_id":"4a86af34ad314708c32e4542511de41343a4eabd"}]}