)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3a7b828dadbc78c1f8259246ea2a74646782b61b","unresolved":true,"context_lines":[{"line_number":15,"context_line":"- Admin override"},{"line_number":16,"context_line":"- Pre-condition"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"APIImpact"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"blueprint add-skip-actions"},{"line_number":21,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"f2cc8e8b_78f38560","line":18,"updated":"2025-06-16 18:23:50.000000000","message":"By the way including this si fine\nbut the tooling that used it was discontinued about 8 years ago so it\nnot needed anymore.\n\nThis use to be used ot automatically create a ticket for the docs team before all docs were moved into the project teams\u0027 repos.","commit_id":"149bff29c2043b5edb2f6377c21e06ca895718a8"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"376bfc571f6d1cef1a61f5022ca4c972c6e4f893","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"de4044e9_099df285","updated":"2025-05-13 08:01:24.000000000","message":"Left a few small comments and a couple of questions, but overall looks good","commit_id":"e008967ba35a8c649cc4706812cfadd460fa0375"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"8024f1d9ad10b6e90f1fa00e06768cea77841b2f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"a8b4a573_e16f1ead","updated":"2025-05-13 19:05:10.000000000","message":"Still need to review the Impacts, but I already have a couple of questions about states. Thanks for proposing Alfredo.","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"a65d290f55bfbea2261243b2c2396e26d3983332","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d213b58b_48b20fb1","in_reply_to":"a8b4a573_e16f1ead","updated":"2025-05-14 08:42:16.000000000","message":"thanks, I will fix the spec with your comments once we finished the open discussions.","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"27cceddc6ff8bdfcc7d6b998ea28e9fb9a62b40b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"d8a70a94_1887000f","updated":"2025-06-03 20:41:04.000000000","message":"Added some suggestion and replied for the Sean comment on new value of existing \u0027state\u0027 field.","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"fb07139ebd91ad6c2f8cc97c50ed59b0ef762022","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"e8e3b6fd_96658fd7","updated":"2025-06-23 18:19:00.000000000","message":"Thanks Alfredo, lets get this one merged.","commit_id":"149bff29c2043b5edb2f6377c21e06ca895718a8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"853a9daba266d9d6bcadf16f992c28128b0cfdc2","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"5a4a2938_41c6783b","updated":"2025-06-16 17:46:26.000000000","message":"i have a few nits inline but nothing that could not be address in a follow up\nso i think im +2 on the overall design.\n\nim not sure if ganshyam has any other concerns but i think the proposal is good enough to move on to the implementation.","commit_id":"149bff29c2043b5edb2f6377c21e06ca895718a8"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"4be540b67e154a2af4dc8e1747ce766678b0f602","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"13bfbd76_fe4a5aec","updated":"2025-06-18 09:29:05.000000000","message":"lgtm, I think this will be a great improvement, thanks Alfredo","commit_id":"149bff29c2043b5edb2f6377c21e06ca895718a8"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"d9bcd337e7ad2a05d8523c10ee47de4596379fab","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"a0769d7a_4467053b","updated":"2025-06-17 17:30:25.000000000","message":"lgtm, the spec covers with good details the main proposal. Some other details we can discuss later in the implementation patches.\nThanks for the proposal Alfredo.","commit_id":"149bff29c2043b5edb2f6377c21e06ca895718a8"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"261151268d064e5ae60e11b855f15ceb4c15f720","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"e8c1d68b_0f123b95","updated":"2025-06-16 17:51:52.000000000","message":"thanks for updates, this looks good to me. Though i will suggest not to use patch but I will leave that decision to watcher team as watcher APIs has patch already and in future all can be moved to PUT.","commit_id":"149bff29c2043b5edb2f6377c21e06ca895718a8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3a7b828dadbc78c1f8259246ea2a74646782b61b","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"98304324_e83228a3","in_reply_to":"e8c1d68b_0f123b95","updated":"2025-06-16 18:23:50.000000000","message":"I kind of feel like for now we should stick with patch, but we might have a different microverion in the future to repalce all fo the usage of patch with put.\n\nfor now i would like to keep that out of the current specs scope.\n\nwe need to think if cleanign up the api si really worth the impact on users. \neven if it would be opt in vai microverison.","commit_id":"149bff29c2043b5edb2f6377c21e06ca895718a8"}],"specs/2025.2/approved/add-exclude-actions.rst":[{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"376bfc571f6d1cef1a61f5022ca4c972c6e4f893","unresolved":true,"context_lines":[{"line_number":22,"context_line":"Problem description"},{"line_number":23,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Currently Watcher does not provides a way to exclude actions execution when"},{"line_number":26,"context_line":"running an action plan:"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"* Administrators lack a standardized mechanism to preemptively skip"}],"source_content_type":"text/x-rst","patch_set":1,"id":"9b555d73_e4e5e19b","line":25,"updated":"2025-05-13 08:01:24.000000000","message":"```suggestion\nCurrently Watcher does not provide a way to exclude actions execution when\n```","commit_id":"e008967ba35a8c649cc4706812cfadd460fa0375"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"376bfc571f6d1cef1a61f5022ca4c972c6e4f893","unresolved":true,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"* Administrators lack a standardized mechanism to preemptively skip"},{"line_number":29,"context_line":"  actions."},{"line_number":30,"context_line":"* pre-condition failures are not clearly distinguished from execution"},{"line_number":31,"context_line":"  failures."},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"Note that Actions already include the CANCELLED state in its state machine [1]"}],"source_content_type":"text/x-rst","patch_set":1,"id":"f9953498_b4acaee1","line":30,"updated":"2025-05-13 08:01:24.000000000","message":"```suggestion\n* Pre-condition failures are not clearly distinguished from execution\n```","commit_id":"e008967ba35a8c649cc4706812cfadd460fa0375"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"376bfc571f6d1cef1a61f5022ca4c972c6e4f893","unresolved":true,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"* When an Action Plan is cancelled, all the actions included are moved to"},{"line_number":37,"context_line":"  CANCELLED state."},{"line_number":38,"context_line":"* When the watcher-applier process is started, all the actions assigned to it"},{"line_number":39,"context_line":"  in ONGOING state are moved to CANCELLED state."},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"The purpose of this spec is to document the required changes in Watcher to"}],"source_content_type":"text/x-rst","patch_set":1,"id":"d125a428_d77f40b6","line":38,"updated":"2025-05-13 08:01:24.000000000","message":"should this say when the wathcer-applier is restarted? I\u0027m not sure I understand the use case otherwise","commit_id":"e008967ba35a8c649cc4706812cfadd460fa0375"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"376bfc571f6d1cef1a61f5022ca4c972c6e4f893","unresolved":true,"context_lines":[{"line_number":47,"context_line":"* **Admin Override:** Administrators can use a patch API call to move an action"},{"line_number":48,"context_line":"  state from PENDING to CANCELLED, so the action will not be executed when"},{"line_number":49,"context_line":"  the action plan is executed."},{"line_number":50,"context_line":"* **Pre-condition detection:** When the pre_condition method of a action"},{"line_number":51,"context_line":"  returns a specific type of Exception, the action will be moved to CANCELLED"},{"line_number":52,"context_line":"  and execution and post_conditions will not be executed."},{"line_number":53,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"6c24cad7_671f6250","line":50,"updated":"2025-05-13 08:01:24.000000000","message":"```suggestion\n* **Pre-condition detection:** When the pre_condition method of an action\n```","commit_id":"e008967ba35a8c649cc4706812cfadd460fa0375"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"376bfc571f6d1cef1a61f5022ca4c972c6e4f893","unresolved":true,"context_lines":[{"line_number":58,"context_line":""},{"line_number":59,"context_line":"For the pre-condition detection use case, a new class of Exception"},{"line_number":60,"context_line":"`ActionCancelException` will be created. When pre_condition raises it, the"},{"line_number":61,"context_line":"ongoing action will be switche to CANCELLED state, and cancel_reason will be"},{"line_number":62,"context_line":"added based on the exception message."},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"For the admin override use case, a new api call Patch method will be added to"}],"source_content_type":"text/x-rst","patch_set":1,"id":"d727e739_f42028ae","line":61,"updated":"2025-05-13 08:01:24.000000000","message":"```suggestion\nongoing action will be switched to CANCELLED state, and cancel_reason will be\n```","commit_id":"e008967ba35a8c649cc4706812cfadd460fa0375"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"376bfc571f6d1cef1a61f5022ca4c972c6e4f893","unresolved":true,"context_lines":[{"line_number":94,"context_line":"* New field `cancel_reason` will be included in:"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"  * ``GET /v1/actions/detail``"},{"line_number":97,"context_line":"  * ``GET /v1/actions/``{server_id}``"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"  No changes in Return code(s)."},{"line_number":100,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"31de47ce_73dd406a","line":97,"updated":"2025-05-13 08:01:24.000000000","message":"does `server_id` have any particular meaning in this section? If it\u0027s just a placeholder, I would use `action_id`, I find it a bit confusing","commit_id":"e008967ba35a8c649cc4706812cfadd460fa0375"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"e1053b17e023b6b438efd1d7c45d9224ea59fb70","unresolved":true,"context_lines":[{"line_number":94,"context_line":"* New field `cancel_reason` will be included in:"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"  * ``GET /v1/actions/detail``"},{"line_number":97,"context_line":"  * ``GET /v1/actions/``{server_id}``"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"  No changes in Return code(s)."},{"line_number":100,"context_line":""}],"source_content_type":"text/x-rst","patch_set":1,"id":"e0b7f658_a2cbd62a","line":97,"in_reply_to":"31de47ce_73dd406a","updated":"2025-05-13 13:18:10.000000000","message":"Bad copy/paste 😊 shoudl be action_id","commit_id":"e008967ba35a8c649cc4706812cfadd460fa0375"}],"specs/2025.2/approved/add-skip-actions.rst":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"8024f1d9ad10b6e90f1fa00e06768cea77841b2f","unresolved":true,"context_lines":[{"line_number":30,"context_line":"* Pre-condition failures are not clearly distinguished from execution"},{"line_number":31,"context_line":"  failures."},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"Note that Actions already include the CANCELLED state in its state machine [1]"},{"line_number":34,"context_line":"however, it is only used for two specific cases:"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"* When an Action Plan is cancelled, all the actions included are moved to the"}],"source_content_type":"text/x-rst","patch_set":2,"id":"74edaf00_e5cc9a4c","line":33,"range":{"start_line":33,"start_character":75,"end_line":33,"end_character":78},"updated":"2025-05-13 19:05:10.000000000","message":"nit: there is a better way to provide these  links[1]\n\n[1] https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#hyperlinks","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"64068bd5aec9961318ebe1f75e02cd16b9f10da2","unresolved":false,"context_lines":[{"line_number":30,"context_line":"* Pre-condition failures are not clearly distinguished from execution"},{"line_number":31,"context_line":"  failures."},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"Note that Actions already include the CANCELLED state in its state machine [1]"},{"line_number":34,"context_line":"however, it is only used for two specific cases:"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"* When an Action Plan is cancelled, all the actions included are moved to the"}],"source_content_type":"text/x-rst","patch_set":2,"id":"b2f31791_d44978e5","line":33,"range":{"start_line":33,"start_character":75,"end_line":33,"end_character":78},"in_reply_to":"74edaf00_e5cc9a4c","updated":"2025-06-03 18:05:54.000000000","message":"Done","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"8024f1d9ad10b6e90f1fa00e06768cea77841b2f","unresolved":true,"context_lines":[{"line_number":73,"context_line":"transitions:"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"* PENDING -\u003e CANCELLED (action plan is cancelled before action is started)"},{"line_number":76,"context_line":"* PENDING -\u003e SKIPPED (action is excluded from execution before started)"},{"line_number":77,"context_line":"* PENDING -\u003e ONGOING (start action execution)"},{"line_number":78,"context_line":"* ONGOING -\u003e CANCELLED (action plan is cancelled while action is running)"},{"line_number":79,"context_line":"* ONGOING -\u003e SUCCEEDED (action finishes successfully)"}],"source_content_type":"text/x-rst","patch_set":2,"id":"56f2a2f5_3a8f641c","line":76,"range":{"start_line":76,"start_character":0,"end_line":76,"end_character":71},"updated":"2025-05-13 19:05:10.000000000","message":"nit: I think you can mention only the new ones","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"64068bd5aec9961318ebe1f75e02cd16b9f10da2","unresolved":false,"context_lines":[{"line_number":73,"context_line":"transitions:"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"* PENDING -\u003e CANCELLED (action plan is cancelled before action is started)"},{"line_number":76,"context_line":"* PENDING -\u003e SKIPPED (action is excluded from execution before started)"},{"line_number":77,"context_line":"* PENDING -\u003e ONGOING (start action execution)"},{"line_number":78,"context_line":"* ONGOING -\u003e CANCELLED (action plan is cancelled while action is running)"},{"line_number":79,"context_line":"* ONGOING -\u003e SUCCEEDED (action finishes successfully)"}],"source_content_type":"text/x-rst","patch_set":2,"id":"d4d42938_50ce962c","line":76,"range":{"start_line":76,"start_character":0,"end_line":76,"end_character":71},"in_reply_to":"56f2a2f5_3a8f641c","updated":"2025-06-03 18:05:54.000000000","message":"Done","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"8024f1d9ad10b6e90f1fa00e06768cea77841b2f","unresolved":true,"context_lines":[{"line_number":81,"context_line":""},{"line_number":82,"context_line":"Note that the transition from PENDING to SKIPPED can be triggered by the"},{"line_number":83,"context_line":"cloud administrator using an API call or automatically when watcher detects"},{"line_number":84,"context_line":"certain predefined conditions in pre_condition execution. Transitioning an"},{"line_number":85,"context_line":"action from PENDING to ONGOING should imply that pre_condition execution"},{"line_number":86,"context_line":"finishes successfully."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"Actions in SKIPPED state will not affect the final state of the action plan."},{"line_number":89,"context_line":"An action plan will only be considered to be FAILED when one or more actions"}],"source_content_type":"text/x-rst","patch_set":2,"id":"9d23d06e_441f818d","line":86,"range":{"start_line":84,"start_character":58,"end_line":86,"end_character":22},"updated":"2025-05-13 19:05:10.000000000","message":"Looking at the code now I see that it is moving to ONGOING before execute pre_condition(). Are you planning to change that in TaskFlow engine? There is no PENDING -\u003e FAILED. What happens if another exeption is raised during the pre_execute?","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"64068bd5aec9961318ebe1f75e02cd16b9f10da2","unresolved":true,"context_lines":[{"line_number":81,"context_line":""},{"line_number":82,"context_line":"Note that the transition from PENDING to SKIPPED can be triggered by the"},{"line_number":83,"context_line":"cloud administrator using an API call or automatically when watcher detects"},{"line_number":84,"context_line":"certain predefined conditions in pre_condition execution. Transitioning an"},{"line_number":85,"context_line":"action from PENDING to ONGOING should imply that pre_condition execution"},{"line_number":86,"context_line":"finishes successfully."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"Actions in SKIPPED state will not affect the final state of the action plan."},{"line_number":89,"context_line":"An action plan will only be considered to be FAILED when one or more actions"}],"source_content_type":"text/x-rst","patch_set":2,"id":"89f2196f_f6345abf","line":86,"range":{"start_line":84,"start_character":58,"end_line":86,"end_character":22},"in_reply_to":"610ebf01_30e3dd2f","updated":"2025-06-03 18:05:54.000000000","message":"i would perfer to not to have `PENDING -\u003e ONGOING -\u003e SKIPPED`\n\nand instead have PENDING -\u003e SKIPPED\n\nIf we reach ONGOING, the only state transition should be CANCELLED,  SUCCEEDED, or FAILED as it is today.\n\nThat does require that we alter the current code, but it should be equivalent.","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"27d6630c86b4551de2ecd7912491a55b6f1396e3","unresolved":true,"context_lines":[{"line_number":81,"context_line":""},{"line_number":82,"context_line":"Note that the transition from PENDING to SKIPPED can be triggered by the"},{"line_number":83,"context_line":"cloud administrator using an API call or automatically when watcher detects"},{"line_number":84,"context_line":"certain predefined conditions in pre_condition execution. Transitioning an"},{"line_number":85,"context_line":"action from PENDING to ONGOING should imply that pre_condition execution"},{"line_number":86,"context_line":"finishes successfully."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"Actions in SKIPPED state will not affect the final state of the action plan."},{"line_number":89,"context_line":"An action plan will only be considered to be FAILED when one or more actions"}],"source_content_type":"text/x-rst","patch_set":2,"id":"fd8fd267_de8faad3","line":86,"range":{"start_line":84,"start_character":58,"end_line":86,"end_character":22},"in_reply_to":"6f20842c_572dd878","updated":"2025-06-04 14:41:58.000000000","message":"I\u0027m fine with the `PENDING -\u003e SKIPPED` option.\n\nThe approach I\u0027m proposing is that pre-condition failure with a specific Exception type will lead to moving an action to SKIPPED. However, any other type of Exception will lead the action to FAILED status. In that case, the state transition will be:\n\nPENDING -\u003e FAILED\n\nSo we will also be adding this as a possible transition for the case of unexpected errors in pre-condition.","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"27cceddc6ff8bdfcc7d6b998ea28e9fb9a62b40b","unresolved":true,"context_lines":[{"line_number":81,"context_line":""},{"line_number":82,"context_line":"Note that the transition from PENDING to SKIPPED can be triggered by the"},{"line_number":83,"context_line":"cloud administrator using an API call or automatically when watcher detects"},{"line_number":84,"context_line":"certain predefined conditions in pre_condition execution. Transitioning an"},{"line_number":85,"context_line":"action from PENDING to ONGOING should imply that pre_condition execution"},{"line_number":86,"context_line":"finishes successfully."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"Actions in SKIPPED state will not affect the final state of the action plan."},{"line_number":89,"context_line":"An action plan will only be considered to be FAILED when one or more actions"}],"source_content_type":"text/x-rst","patch_set":2,"id":"6f20842c_572dd878","line":86,"range":{"start_line":84,"start_character":58,"end_line":86,"end_character":22},"in_reply_to":"89f2196f_f6345abf","updated":"2025-06-03 20:41:04.000000000","message":"agree with Sean, it is confusing to allow ongoing actions to skipped. Action should be skipped before it start/change state to ongoing.","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"a65d290f55bfbea2261243b2c2396e26d3983332","unresolved":true,"context_lines":[{"line_number":81,"context_line":""},{"line_number":82,"context_line":"Note that the transition from PENDING to SKIPPED can be triggered by the"},{"line_number":83,"context_line":"cloud administrator using an API call or automatically when watcher detects"},{"line_number":84,"context_line":"certain predefined conditions in pre_condition execution. Transitioning an"},{"line_number":85,"context_line":"action from PENDING to ONGOING should imply that pre_condition execution"},{"line_number":86,"context_line":"finishes successfully."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"Actions in SKIPPED state will not affect the final state of the action plan."},{"line_number":89,"context_line":"An action plan will only be considered to be FAILED when one or more actions"}],"source_content_type":"text/x-rst","patch_set":2,"id":"610ebf01_30e3dd2f","line":86,"range":{"start_line":84,"start_character":58,"end_line":86,"end_character":22},"in_reply_to":"9d23d06e_441f818d","updated":"2025-05-14 08:42:16.000000000","message":"Good point. That\u0027s correct, we discussed about that in the irc conversation. The plan was to change the order of moving to ONGOING after running pre-condition. As you mentioned, that would require a new transition PENDING-\u003eFAILED for other errors in pre-condition. I can add it to this spec. If there is any problem with it, we could also keep current behavior and allow following state changes for the pre-condition failure:\n\nPENDING -\u003e ONGOING -\u003e SKIPPED\n\nI think both ways can work and are doable. What is your preference?\n\n@smooney@redhat.com I\u0027d like to have your input also in this topic.","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"853a9daba266d9d6bcadf16f992c28128b0cfdc2","unresolved":false,"context_lines":[{"line_number":81,"context_line":""},{"line_number":82,"context_line":"Note that the transition from PENDING to SKIPPED can be triggered by the"},{"line_number":83,"context_line":"cloud administrator using an API call or automatically when watcher detects"},{"line_number":84,"context_line":"certain predefined conditions in pre_condition execution. Transitioning an"},{"line_number":85,"context_line":"action from PENDING to ONGOING should imply that pre_condition execution"},{"line_number":86,"context_line":"finishes successfully."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"Actions in SKIPPED state will not affect the final state of the action plan."},{"line_number":89,"context_line":"An action plan will only be considered to be FAILED when one or more actions"}],"source_content_type":"text/x-rst","patch_set":2,"id":"e52b5f08_a57d546c","line":86,"range":{"start_line":84,"start_character":58,"end_line":86,"end_character":22},"in_reply_to":"fd8fd267_de8faad3","updated":"2025-06-16 17:46:26.000000000","message":"Acknowledged","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"8024f1d9ad10b6e90f1fa00e06768cea77841b2f","unresolved":true,"context_lines":[{"line_number":90,"context_line":"are in FAILED state when execution finishes."},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"For the pre-condition detection use case, a new class of Exception"},{"line_number":93,"context_line":"`ActionSkipException` will be created. When pre_condition raises it, the"},{"line_number":94,"context_line":"ongoing action will be switched to SKIPPED state, and skip_reason will be"},{"line_number":95,"context_line":"added based on the exception message."},{"line_number":96,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"50cd1f77_4e77a16c","line":93,"range":{"start_line":93,"start_character":1,"end_line":93,"end_character":20},"updated":"2025-05-13 19:05:10.000000000","message":"I think that is fine, since pre_condition() method returns a boolean today.","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"853a9daba266d9d6bcadf16f992c28128b0cfdc2","unresolved":false,"context_lines":[{"line_number":90,"context_line":"are in FAILED state when execution finishes."},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"For the pre-condition detection use case, a new class of Exception"},{"line_number":93,"context_line":"`ActionSkipException` will be created. When pre_condition raises it, the"},{"line_number":94,"context_line":"ongoing action will be switched to SKIPPED state, and skip_reason will be"},{"line_number":95,"context_line":"added based on the exception message."},{"line_number":96,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"177517f0_3d07295c","line":93,"range":{"start_line":93,"start_character":1,"end_line":93,"end_character":20},"in_reply_to":"50cd1f77_4e77a16c","updated":"2025-06-16 17:46:26.000000000","message":"Acknowledged","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"8024f1d9ad10b6e90f1fa00e06768cea77841b2f","unresolved":true,"context_lines":[{"line_number":96,"context_line":""},{"line_number":97,"context_line":"For the admin override use case, a new api call Patch method will be added to"},{"line_number":98,"context_line":"the Actions which will allow to change the state from PENDING to SKIPPED"},{"line_number":99,"context_line":"state. Optionally, the user can also include a value for `skip_reason`, in"},{"line_number":100,"context_line":"this call."},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"Additionally, support for the changes in the API should be included in the"},{"line_number":103,"context_line":"python-watcherclient and watcher-dashboard so that the cloud admins can skip"}],"source_content_type":"text/x-rst","patch_set":2,"id":"0bf98494_7467f302","line":100,"range":{"start_line":99,"start_character":7,"end_line":100,"end_character":10},"updated":"2025-05-13 19:05:10.000000000","message":"++\nIn case the user don\u0027t provide a reason, we could add a default reason: \"skipped by user\"/\"user\u0027s requested\", etc.","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"853a9daba266d9d6bcadf16f992c28128b0cfdc2","unresolved":false,"context_lines":[{"line_number":96,"context_line":""},{"line_number":97,"context_line":"For the admin override use case, a new api call Patch method will be added to"},{"line_number":98,"context_line":"the Actions which will allow to change the state from PENDING to SKIPPED"},{"line_number":99,"context_line":"state. Optionally, the user can also include a value for `skip_reason`, in"},{"line_number":100,"context_line":"this call."},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"Additionally, support for the changes in the API should be included in the"},{"line_number":103,"context_line":"python-watcherclient and watcher-dashboard so that the cloud admins can skip"}],"source_content_type":"text/x-rst","patch_set":2,"id":"843e159c_fd084a18","line":100,"range":{"start_line":99,"start_character":7,"end_line":100,"end_character":10},"in_reply_to":"0bf98494_7467f302","updated":"2025-06-16 17:46:26.000000000","message":"Acknowledged","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"8024f1d9ad10b6e90f1fa00e06768cea77841b2f","unresolved":true,"context_lines":[{"line_number":110,"context_line":"------------"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"Instead of creating a new SKIPPED state, we may reuse the existing CANCELLED"},{"line_number":113,"context_line":"state to cover the two new use cases. After discussing this option on irc [3]"},{"line_number":114,"context_line":"we have considered that it would lead to ambiguity in the actual situation for"},{"line_number":115,"context_line":"CANCELLED actions, as multiple and pretty different situations may lead to it."},{"line_number":116,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"8cb27511_6b8772cf","line":113,"range":{"start_line":113,"start_character":37,"end_line":113,"end_character":76},"updated":"2025-05-13 19:05:10.000000000","message":"I agree that reusing it will cause more confusion. One state to represent multiple scenarios is hard to understand/debug.","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"853a9daba266d9d6bcadf16f992c28128b0cfdc2","unresolved":false,"context_lines":[{"line_number":110,"context_line":"------------"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"Instead of creating a new SKIPPED state, we may reuse the existing CANCELLED"},{"line_number":113,"context_line":"state to cover the two new use cases. After discussing this option on irc [3]"},{"line_number":114,"context_line":"we have considered that it would lead to ambiguity in the actual situation for"},{"line_number":115,"context_line":"CANCELLED actions, as multiple and pretty different situations may lead to it."},{"line_number":116,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"1dc4e172_7d8b7686","line":113,"range":{"start_line":113,"start_character":37,"end_line":113,"end_character":76},"in_reply_to":"8cb27511_6b8772cf","updated":"2025-06-16 17:46:26.000000000","message":"Acknowledged","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"8024f1d9ad10b6e90f1fa00e06768cea77841b2f","unresolved":true,"context_lines":[{"line_number":114,"context_line":"we have considered that it would lead to ambiguity in the actual situation for"},{"line_number":115,"context_line":"CANCELLED actions, as multiple and pretty different situations may lead to it."},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"With this proposal, the SKIPPED state is restricted to be used for actions"},{"line_number":118,"context_line":"which were never actually executed in executed action plans."},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"Data model impact"},{"line_number":121,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"604910c0_e09b4fdb","line":118,"range":{"start_line":117,"start_character":0,"end_line":118,"end_character":60},"updated":"2025-05-13 19:05:10.000000000","message":"Ack. I also think that it is clear that SKIPPED is only for Actions, not ActionPlans (which doesn\u0027t make sense to have)","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"853a9daba266d9d6bcadf16f992c28128b0cfdc2","unresolved":false,"context_lines":[{"line_number":114,"context_line":"we have considered that it would lead to ambiguity in the actual situation for"},{"line_number":115,"context_line":"CANCELLED actions, as multiple and pretty different situations may lead to it."},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"With this proposal, the SKIPPED state is restricted to be used for actions"},{"line_number":118,"context_line":"which were never actually executed in executed action plans."},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"Data model impact"},{"line_number":121,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":2,"id":"a59553e4_1e0f057e","line":118,"range":{"start_line":117,"start_character":0,"end_line":118,"end_character":60},"in_reply_to":"604910c0_e09b4fdb","updated":"2025-06-16 17:46:26.000000000","message":"ya if you want to skip an action plan you jsut archive it instead of accepting it.","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9a7b5aedcc0f8a89148b193f3038ff219307513a","unresolved":true,"context_lines":[{"line_number":129,"context_line":""},{"line_number":130,"context_line":"* New field `skip_reason` will be included in:"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"  * ``GET /v1/actions/detail``"},{"line_number":133,"context_line":"  * ``GET /v1/actions/``{action_id}``"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"  No changes in Return code(s)."},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"  Example of json addition in ``GET /v1/actions/``{action_id}`` response:"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"  .. code-block::"},{"line_number":140,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"b00b38dd_589aff1c","line":137,"range":{"start_line":132,"start_character":1,"end_line":137,"end_character":73},"updated":"2025-05-13 17:24:16.000000000","message":"im not sure this is correct.\n\ni have not read the spec yet just skiming it but i was expecting this to be more like\n\n ```/v1/action_plans/{plan_id}/actions/{action id or index}```\n \n \n\nwe dont really want to allow skiping actions via the rest api once an action plan is accpeted.\n\nso i dont think we would want a top level api to enabel this on any action.","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"27d6630c86b4551de2ecd7912491a55b6f1396e3","unresolved":false,"context_lines":[{"line_number":129,"context_line":""},{"line_number":130,"context_line":"* New field `skip_reason` will be included in:"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"  * ``GET /v1/actions/detail``"},{"line_number":133,"context_line":"  * ``GET /v1/actions/``{action_id}``"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"  No changes in Return code(s)."},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"  Example of json addition in ``GET /v1/actions/``{action_id}`` response:"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"  .. code-block::"},{"line_number":140,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"87e499c0_ac72796b","line":137,"range":{"start_line":132,"start_character":1,"end_line":137,"end_character":73},"in_reply_to":"33216094_0a20987d","updated":"2025-06-04 14:41:58.000000000","message":"ack","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"64068bd5aec9961318ebe1f75e02cd16b9f10da2","unresolved":false,"context_lines":[{"line_number":129,"context_line":""},{"line_number":130,"context_line":"* New field `skip_reason` will be included in:"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"  * ``GET /v1/actions/detail``"},{"line_number":133,"context_line":"  * ``GET /v1/actions/``{action_id}``"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"  No changes in Return code(s)."},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"  Example of json addition in ``GET /v1/actions/``{action_id}`` response:"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"  .. code-block::"},{"line_number":140,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"33216094_0a20987d","line":137,"range":{"start_line":132,"start_character":1,"end_line":137,"end_character":73},"in_reply_to":"7119aa5f_cc7f23b8","updated":"2025-06-03 18:05:54.000000000","message":"i disucssed this a littel with gmann.\n\ncurrenlty actions are unfortully exposed as top level resouce.\n\nlogicly the action are internal private state of the action plan and should have been sub resouce of the action plan.\n\nsince we are currently locked into the design of having actions be a top level resocue we shoudl proceed with \"PATCH /v1/actions/{action_id}\" for consitency.","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"a65d290f55bfbea2261243b2c2396e26d3983332","unresolved":true,"context_lines":[{"line_number":129,"context_line":""},{"line_number":130,"context_line":"* New field `skip_reason` will be included in:"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"  * ``GET /v1/actions/detail``"},{"line_number":133,"context_line":"  * ``GET /v1/actions/``{action_id}``"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"  No changes in Return code(s)."},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"  Example of json addition in ``GET /v1/actions/``{action_id}`` response:"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"  .. code-block::"},{"line_number":140,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"7119aa5f_cc7f23b8","line":137,"range":{"start_line":132,"start_character":1,"end_line":137,"end_character":73},"in_reply_to":"b00b38dd_589aff1c","updated":"2025-05-14 08:42:16.000000000","message":"Some considerations which I find relevant for this:\n\n- Currently, in the API actions are not part of the ActionPlan (in the sense that there is no an Actions field in the AP which contains all the Action definitions).\n- Actions are related to APs via foreign_key action_plan_uuid in actions. i.e. the way the api list the actions in an AP is using `GET /v1/actions/?action_plan_uuid\u003d\u003cuid\u003e` , there is no something like \u0027/v1/action_plans/{plan_id}/actions/` or `GET  /v1/action_plans/{plan_id}` and use some \u0027actions\u0027 field.\n- An action is created in the database and api as soon as the action plan is created in PENDING state, and since then is available in top level /actions (with PENDING state).\n\nAccording to ^ , my understanding of the implementation of \"we dont really want to allow skiping actions via the rest api once an action plan is accpeted\" would be to allow state transitions to SKIPPED only when the parent AP is in PENDING state using an API call `PATCH /v1/actions/{action_id}` to modify the state field (this is a similar approach to the current implementation of AP cancellation, i.e.\n\nIMO implementing something like `/v1/action_plans/{plan_id}/actions/{action id or index}` is a more complex change from API PoV and imo there is no big advantadge over my proposal. WDYT?\n\nBTW, in the current state of the spec, my proposal was to allow only a transition to SKIPPED only for actions in PENDING from the API. However, I will modify it to allow only a transition to SKIPPED for actions in ActionPlans in PENDING. It\u0027s slightly different and closer to what we want (once an AP is started, actions can not be skipped even for actions which didn\u0027t started yet).","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"64068bd5aec9961318ebe1f75e02cd16b9f10da2","unresolved":true,"context_lines":[{"line_number":275,"context_line":"References"},{"line_number":276,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"[1] https://github.com/openstack/watcher/blob/f38ab70ba46756b2c3ae74b1a2fafdb39ac58cc7/watcher/api/controllers/v1/action.py#L38-L51"},{"line_number":279,"context_line":"[2] https://github.com/openstack/watcher/blob/59607f616a0a7c8e38f488922ec3c27dffe692e7/watcher/applier/sync.py#L54-L75"},{"line_number":280,"context_line":"[3] https://meetings.opendev.org/irclogs/%23openstack-watcher/%23openstack-watcher.2025-05-13.log.html#openstack-watcher.2025-05-13.log.html#t2025-05-13T12:53:54"},{"line_number":281,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"aec4e14b_aa0c4872","line":278,"updated":"2025-06-03 18:05:54.000000000","message":"this is not wrong by the way.\nwe often put the actull links in teh refence section.\n\ni.e. instead of doing an inline likn we will put the links in a bullet list in the\nrefence section and also link form them in the main spec.","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"853a9daba266d9d6bcadf16f992c28128b0cfdc2","unresolved":false,"context_lines":[{"line_number":275,"context_line":"References"},{"line_number":276,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":277,"context_line":""},{"line_number":278,"context_line":"[1] https://github.com/openstack/watcher/blob/f38ab70ba46756b2c3ae74b1a2fafdb39ac58cc7/watcher/api/controllers/v1/action.py#L38-L51"},{"line_number":279,"context_line":"[2] https://github.com/openstack/watcher/blob/59607f616a0a7c8e38f488922ec3c27dffe692e7/watcher/applier/sync.py#L54-L75"},{"line_number":280,"context_line":"[3] https://meetings.opendev.org/irclogs/%23openstack-watcher/%23openstack-watcher.2025-05-13.log.html#openstack-watcher.2025-05-13.log.html#t2025-05-13T12:53:54"},{"line_number":281,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"251fb113_d28bdcf4","line":278,"in_reply_to":"aec4e14b_aa0c4872","updated":"2025-06-16 17:46:26.000000000","message":"Acknowledged","commit_id":"858f9570414c4fcda4c343213f597db110a12a24"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"27cceddc6ff8bdfcc7d6b998ea28e9fb9a62b40b","unresolved":true,"context_lines":[{"line_number":81,"context_line":"action from PENDING to ONGOING should imply that pre_condition execution"},{"line_number":82,"context_line":"finishes successfully."},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"Actions in SKIPPED state will not affect the final state of the action plan."},{"line_number":85,"context_line":"An action plan will only be considered to be FAILED when one or more actions"},{"line_number":86,"context_line":"are in FAILED state when execution finishes."},{"line_number":87,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"507c3f86_f605c844","line":84,"range":{"start_line":84,"start_character":0,"end_line":84,"end_character":76},"updated":"2025-06-03 20:41:04.000000000","message":"success-with-skip is not equal to success so IMO, action plan should give clear message via final state or some message to convey that \"Action plan is success but with some skipped actions\".","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"761987c9f1ac35f21c63f9b2223d3fb18b993bcd","unresolved":true,"context_lines":[{"line_number":81,"context_line":"action from PENDING to ONGOING should imply that pre_condition execution"},{"line_number":82,"context_line":"finishes successfully."},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"Actions in SKIPPED state will not affect the final state of the action plan."},{"line_number":85,"context_line":"An action plan will only be considered to be FAILED when one or more actions"},{"line_number":86,"context_line":"are in FAILED state when execution finishes."},{"line_number":87,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"859ebfd2_ce7bdb64","line":84,"range":{"start_line":84,"start_character":0,"end_line":84,"end_character":76},"in_reply_to":"475c264f_b3664a31","updated":"2025-06-12 16:59:07.000000000","message":"It does not need to be a different state but adding the skipped actions in some field say \u0027skipped_actions\u0027 which can None if no action skipped otherwise skipped actions info","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"a4ac7543b740df124b0aea3af50bad0800e9e5f4","unresolved":true,"context_lines":[{"line_number":81,"context_line":"action from PENDING to ONGOING should imply that pre_condition execution"},{"line_number":82,"context_line":"finishes successfully."},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"Actions in SKIPPED state will not affect the final state of the action plan."},{"line_number":85,"context_line":"An action plan will only be considered to be FAILED when one or more actions"},{"line_number":86,"context_line":"are in FAILED state when execution finishes."},{"line_number":87,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"475c264f_b3664a31","line":84,"range":{"start_line":84,"start_character":0,"end_line":84,"end_character":76},"in_reply_to":"507c3f86_f605c844","updated":"2025-06-04 14:50:33.000000000","message":"I don\u0027t know what\u0027s the best way to manage this. Currently the watcher actionplan api does not provide any field or mechanismo to add that message\". I understand the potential options are:\n\n- Add a new action plan state SUCCEEDED_WITH_SKIPPED \n- Add a new text field `reason` (or `notes` or whatever) to the action plan that we may populate.\n- Add a new field with_skipped to actionplans that we may turn to true or false when an action plan is executed.\n\nwdyt?","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"853a9daba266d9d6bcadf16f992c28128b0cfdc2","unresolved":true,"context_lines":[{"line_number":81,"context_line":"action from PENDING to ONGOING should imply that pre_condition execution"},{"line_number":82,"context_line":"finishes successfully."},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"Actions in SKIPPED state will not affect the final state of the action plan."},{"line_number":85,"context_line":"An action plan will only be considered to be FAILED when one or more actions"},{"line_number":86,"context_line":"are in FAILED state when execution finishes."},{"line_number":87,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"636fb553_7d941102","line":84,"range":{"start_line":84,"start_character":0,"end_line":84,"end_character":76},"in_reply_to":"859ebfd2_ce7bdb64","updated":"2025-06-16 17:46:26.000000000","message":"so i think we have more or less agreed that the action plan should go to SUCCEEDED and the new status_message field will say something like \"Succeed with n skipped actions\"\n\nwe can bikeshed on the exact message in the implementation review.\n\nin the future the status_message can be extened to convay information in other states as well.\n\nfor now i would suggest that we simple leave it empty for a standard SUCCEEDED state wehre all actions were aplied succssfully and only populate a message if we have something useful to say.","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"27cceddc6ff8bdfcc7d6b998ea28e9fb9a62b40b","unresolved":true,"context_lines":[{"line_number":90,"context_line":"ongoing action will be switched to SKIPPED state, and skip_reason will be"},{"line_number":91,"context_line":"added based on the exception message."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"For the admin override use case, a new api call Patch method will be added to"},{"line_number":94,"context_line":"the Actions which will allow to change the state from PENDING to SKIPPED"},{"line_number":95,"context_line":"state only when the parent action plan is also in the PENDING state."},{"line_number":96,"context_line":"Optionally, the user can also include a value for `skip_reason`, in"}],"source_content_type":"text/x-rst","patch_set":3,"id":"be6de953_42fcec62","line":93,"range":{"start_line":93,"start_character":48,"end_line":93,"end_character":54},"updated":"2025-06-03 20:41:04.000000000","message":"As per API guidelines, patch is not preferred instead PUT method should be added to update the resource/its states","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"761987c9f1ac35f21c63f9b2223d3fb18b993bcd","unresolved":true,"context_lines":[{"line_number":90,"context_line":"ongoing action will be switched to SKIPPED state, and skip_reason will be"},{"line_number":91,"context_line":"added based on the exception message."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"For the admin override use case, a new api call Patch method will be added to"},{"line_number":94,"context_line":"the Actions which will allow to change the state from PENDING to SKIPPED"},{"line_number":95,"context_line":"state only when the parent action plan is also in the PENDING state."},{"line_number":96,"context_line":"Optionally, the user can also include a value for `skip_reason`, in"}],"source_content_type":"text/x-rst","patch_set":3,"id":"e7dbf2eb_9ed33a16","line":93,"range":{"start_line":93,"start_character":48,"end_line":93,"end_character":54},"in_reply_to":"a37d2d2a_56eb5ee5","updated":"2025-06-12 16:59:07.000000000","message":"yeah consistency is important but if you start using PUT for new which give chance to convert existing patch into put in new microvesion. \n\nBut anyways I will leave the decision to wather team as consistency is good point (if no future plan to convert patch into put).","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"27d6630c86b4551de2ecd7912491a55b6f1396e3","unresolved":true,"context_lines":[{"line_number":90,"context_line":"ongoing action will be switched to SKIPPED state, and skip_reason will be"},{"line_number":91,"context_line":"added based on the exception message."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"For the admin override use case, a new api call Patch method will be added to"},{"line_number":94,"context_line":"the Actions which will allow to change the state from PENDING to SKIPPED"},{"line_number":95,"context_line":"state only when the parent action plan is also in the PENDING state."},{"line_number":96,"context_line":"Optionally, the user can also include a value for `skip_reason`, in"}],"source_content_type":"text/x-rst","patch_set":3,"id":"a37d2d2a_56eb5ee5","line":93,"range":{"start_line":93,"start_character":48,"end_line":93,"end_character":54},"in_reply_to":"be6de953_42fcec62","updated":"2025-06-04 14:41:58.000000000","message":"yes, that is not the preferred but it is how the rest of the api has been created in the past for similar purposes in different API elements. I\u0027m proposing that for consistency with it.","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"853a9daba266d9d6bcadf16f992c28128b0cfdc2","unresolved":false,"context_lines":[{"line_number":90,"context_line":"ongoing action will be switched to SKIPPED state, and skip_reason will be"},{"line_number":91,"context_line":"added based on the exception message."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"For the admin override use case, a new api call Patch method will be added to"},{"line_number":94,"context_line":"the Actions which will allow to change the state from PENDING to SKIPPED"},{"line_number":95,"context_line":"state only when the parent action plan is also in the PENDING state."},{"line_number":96,"context_line":"Optionally, the user can also include a value for `skip_reason`, in"}],"source_content_type":"text/x-rst","patch_set":3,"id":"e135d15d_88f3c7d5","line":93,"range":{"start_line":93,"start_character":48,"end_line":93,"end_character":54},"in_reply_to":"e7dbf2eb_9ed33a16","updated":"2025-06-16 17:46:26.000000000","message":"Acknowledged","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"27cceddc6ff8bdfcc7d6b998ea28e9fb9a62b40b","unresolved":true,"context_lines":[{"line_number":93,"context_line":"For the admin override use case, a new api call Patch method will be added to"},{"line_number":94,"context_line":"the Actions which will allow to change the state from PENDING to SKIPPED"},{"line_number":95,"context_line":"state only when the parent action plan is also in the PENDING state."},{"line_number":96,"context_line":"Optionally, the user can also include a value for `skip_reason`, in"},{"line_number":97,"context_line":"this call."},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"Additionally, support for the changes in the API should be included in the"},{"line_number":100,"context_line":"python-watcherclient and watcher-dashboard so that the cloud admins can skip"}],"source_content_type":"text/x-rst","patch_set":3,"id":"d8fdbe66_f0d78635","line":97,"range":{"start_line":96,"start_character":0,"end_line":97,"end_character":10},"updated":"2025-06-03 20:41:04.000000000","message":"++","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"853a9daba266d9d6bcadf16f992c28128b0cfdc2","unresolved":false,"context_lines":[{"line_number":93,"context_line":"For the admin override use case, a new api call Patch method will be added to"},{"line_number":94,"context_line":"the Actions which will allow to change the state from PENDING to SKIPPED"},{"line_number":95,"context_line":"state only when the parent action plan is also in the PENDING state."},{"line_number":96,"context_line":"Optionally, the user can also include a value for `skip_reason`, in"},{"line_number":97,"context_line":"this call."},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"Additionally, support for the changes in the API should be included in the"},{"line_number":100,"context_line":"python-watcherclient and watcher-dashboard so that the cloud admins can skip"}],"source_content_type":"text/x-rst","patch_set":3,"id":"1903cc9d_5aacfca3","line":97,"range":{"start_line":96,"start_character":0,"end_line":97,"end_character":10},"in_reply_to":"d8fdbe66_f0d78635","updated":"2025-06-16 17:46:26.000000000","message":"Acknowledged","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"64068bd5aec9961318ebe1f75e02cd16b9f10da2","unresolved":true,"context_lines":[{"line_number":101,"context_line":"an action from PENDING state and see the `skip_reason` value for SKIPPED"},{"line_number":102,"context_line":"actions."},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"Microversion will be increased for this change."},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"Alternatives"},{"line_number":107,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"354d836d_631424e3","line":104,"updated":"2025-06-03 18:05:54.000000000","message":"so this is correct but this spec is missing a disucssion of how the SKIPPED state will be reported when using the older microversion\n\nfor backward compatibility we cannot return the SKIPPED status you do an action show using an older microverion.\n\nWe are going to have to decide how to handle that.\n\nim inclined to say it should be reported as FAILED or SUCCEEDED\n\nWe could to align this to how precondition failures are handled today when precondition fails in ONGOING. i.e. we would align skip to whatever we set the acction state too if you have a migration operation and the vm is delete after the action plane is created.\n\nim inclined to say we woudl report it as SUCCEEDED, but i am not sure.\n\nThe state field is incorrectly defined as a string in the object today.\nIt really should be an enum field.\n\nhttps://github.com/openstack/watcher/blob/59757249bbbbb45bead28bfb2899242aea97dfc1/watcher/objects/action.py#L26-L533\n\n\nWatcher is not properly implementing object versioning as it does not have support for downleveling objects...\n\n\nas a resutl im not sure how strict we should be with the api response.\n\nCurrently looking at the audit api, there is a hide_fields_in_newer_versions function\n\nhttps://github.com/openstack/watcher/blob/59757249bbbbb45bead28bfb2899242aea97dfc1/watcher/api/controllers/v1/audit.py#L68-L79\n\nthere is also a noop for the actions api\nhttps://github.com/openstack/watcher/blob/master/watcher/api/controllers/v1/action.py#L77-L84\n\n\nwe may neeed a hide_fields_in_older_versions\n\nto backlevel the response here\n\nhttps://github.com/openstack/watcher/blob/master/watcher/api/controllers/v1/action.py#L187\n\nPerhaps this si a place where Gmann can provide some guidance.\ngiven we currently do not have a scheme and becuase the state field is only a string, perhaps its currently ok to allow SKIPPED in older api response if we consider it to be unversioned, but normally that would not be allowed as older clients should not need to handle this vlaue when doing audit list or show if they use the older microveriosn.\n\nif we take the api filed litrally then its just a string and therefor can chave any value and the client shoudl not restrict to only the valid values.\nhowever watcher api ref specifes which states are valid https://docs.openstack.org/api-ref/resource-optimization/#actions\n\nso it would not be incorect for them to only accpt the valid set.","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"853a9daba266d9d6bcadf16f992c28128b0cfdc2","unresolved":true,"context_lines":[{"line_number":101,"context_line":"an action from PENDING state and see the `skip_reason` value for SKIPPED"},{"line_number":102,"context_line":"actions."},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"Microversion will be increased for this change."},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"Alternatives"},{"line_number":107,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"6783aa06_04ff72c0","line":104,"in_reply_to":"06168863_c141e9c0","updated":"2025-06-16 17:46:26.000000000","message":"Thanks for confirming that I\u0027m overthinking this and that we only need to modify the response if the type of the field changes or when we add/remvoe one.\n\nso in old microversion or new action show will contain SKIPPED\nbut for the old micoversion we should not return status_message in the reponce.\n\nso we will have to provide some level of ai backlevleing but it ammoutn to just not returning the new field when called with the old micorversion.\nthat alines with what i expect.","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"27d6630c86b4551de2ecd7912491a55b6f1396e3","unresolved":true,"context_lines":[{"line_number":101,"context_line":"an action from PENDING state and see the `skip_reason` value for SKIPPED"},{"line_number":102,"context_line":"actions."},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"Microversion will be increased for this change."},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"Alternatives"},{"line_number":107,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"4aa88964_3d0a2a96","line":104,"in_reply_to":"06168863_c141e9c0","updated":"2025-06-04 14:41:58.000000000","message":"Thanks for the info. To summarize:\n- The state SKIPPED will be returned by the API independently of the microversion.\n- New Patch call will be supported only in the new microversion.\n\nwhat about the new field `reason`? I understand it will also be part of the new microversion and will be only returned with calls using it.","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e0244db96d5b18a7b3d59374227f8102e7ffb3c7","unresolved":true,"context_lines":[{"line_number":101,"context_line":"an action from PENDING state and see the `skip_reason` value for SKIPPED"},{"line_number":102,"context_line":"actions."},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"Microversion will be increased for this change."},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"Alternatives"},{"line_number":107,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"49f5da34_8d6af1dd","line":104,"in_reply_to":"354d836d_631424e3","updated":"2025-06-03 18:37:01.000000000","message":"looking at nova i dont see any example of the value of a filed changing based on the microversion where we map a new value to a prviosuly allowed value.\n\nthe only exctpiotn to that is the comptue service id\n\nin 2.53+ its a uuid\nin 2.52 and below its the integer database primary key.\n\nhttps://docs.openstack.org/nova/latest/reference/api-microversion-history.html#maximum-in-pike\n\n\nwe do sometimes include or exclude them based on micoverion, but we do not adapt the values of vm_state or task_state as far as i can see.\n\nim hesitating becasue im not sure we have exetened vm_state ro task_sate since we started using microverison in kilo. so that might just be a side effect of that.\n\n\nthe other api we could draw insperation form is the console chagne for spice-direct\n\n\nhttps://docs.openstack.org/nova/latest/reference/api-microversion-history.html#microversion-2-99\n\nin 2.99 the allowd values fo the type enum\n\nhttps://github.com/openstack/tempest/blob/master/tempest/lib/api_schema/response/compute/v2_99/servers.py#L63-L65\n\nwas extened to add \u0027spice-direct\u0027\n\nin older micoversion i.e. 2.3\n\nhttps://github.com/openstack/tempest/blob/master/tempest/lib/api_schema/response/compute/v2_6/servers.py#L39-L63\n\nnova retrun the enum values that were supproted then.\n\n\ni think we are going to have to thread it as unversioned\nand instead just add a note to the api ref that SKIPPED was added in the 2025.2 release\n\nif we look at how tempest validates vm_state and task_state it treat it as a nullable sting for task state as that is admin only and a sting for vm_state\n\nhttps://github.com/openstack/tempest/blob/7cb807a324c9f2bac47030c590ed488229bc113b/tempest/lib/api_schema/response/compute/v2_16/servers.py#L78-L79\n\nthat refect the fact taht we can extend those over tiem without considring it a micoversion bump.\n\nas such that means the new micovertion we woudl be addign for this spec is only for the new PATCH endpoin not the extenion of the states to include SKIPPED.","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"27cceddc6ff8bdfcc7d6b998ea28e9fb9a62b40b","unresolved":true,"context_lines":[{"line_number":101,"context_line":"an action from PENDING state and see the `skip_reason` value for SKIPPED"},{"line_number":102,"context_line":"actions."},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"Microversion will be increased for this change."},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"Alternatives"},{"line_number":107,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"06168863_c141e9c0","line":104,"in_reply_to":"49f5da34_8d6af1dd","updated":"2025-06-03 20:41:04.000000000","message":"Yes, adding the new state value \u0027Skipped\u0027 can be return for older microversion if present and it does not need to be mapped with the microversion. And that is because of:\n- response field \u0027state\u0027 is not changed\n- Type of response field is not changed\n- Only value returning is changed which does not require microversion bump.\n\nSo adding new value of same type in existing field is safe to add/return for all microversion.\n\nIf you see Nova example Sean provided, we only bumped the microversion when either field or type of field\u0027s value is changed.","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"761987c9f1ac35f21c63f9b2223d3fb18b993bcd","unresolved":true,"context_lines":[{"line_number":101,"context_line":"an action from PENDING state and see the `skip_reason` value for SKIPPED"},{"line_number":102,"context_line":"actions."},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"Microversion will be increased for this change."},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"Alternatives"},{"line_number":107,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"4740d1cc_b414e29d","line":104,"in_reply_to":"4aa88964_3d0a2a96","updated":"2025-06-12 16:59:07.000000000","message":"Any new field addition or existing field removal needs to be microversioned.","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"64068bd5aec9961318ebe1f75e02cd16b9f10da2","unresolved":true,"context_lines":[{"line_number":118,"context_line":"Data model impact"},{"line_number":119,"context_line":"-----------------"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"* Add a new column `skip_reason` of type `String(255)` in Actions table."},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"REST API impact"},{"line_number":124,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"579905f2_1e9c5a1b","line":121,"updated":"2025-06-03 18:05:54.000000000","message":"normally addign a new state would modify the filed and reuslt in a change to the object version. \n\nthat would require backleveling supprot like this\n\nhttps://github.com/openstack/nova/blob/master/nova/objects/image_meta.py#L209-L212\n\nBut watcher has not implemented object versioning corectly.\n\nthis is ment to catch an attept to backlevel an object to the supported version of a removte agent when making an RPC call if they do not support the version.\n\ncurrently watcher is not passing object via the RPC bus only resouce uuids or primitave types like string.\n\nthis is the only reasons that its not breaking horribly today.\n\nthe proiblem is that the object are also used when loadign dobject form the db.\n\nthat currently menas that its not possibel to run diffent version fo watcher-api watcher-decsion-enging adn watcher-applier today.\n\nthey all have to agree on both the db model and object version exactly meaning they have to be exactly the same version to work safely since there is no versioning supprot at all.\n\n\nfixing this is out of scope of this change but there is very little benifit to suing OVOs today given how they are being used.","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"64068bd5aec9961318ebe1f75e02cd16b9f10da2","unresolved":true,"context_lines":[{"line_number":118,"context_line":"Data model impact"},{"line_number":119,"context_line":"-----------------"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"* Add a new column `skip_reason` of type `String(255)` in Actions table."},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"REST API impact"},{"line_number":124,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"af848a38_a157eb55","line":121,"updated":"2025-06-03 18:05:54.000000000","message":"regarding the field name.\n\nwe could name it reason.\n\nan action should only ever be in one state.\nif we add a generic reason field in the db we can use it for skip today\nbut we can use it for CANCELED or FAILED in the future\n\nthat will allow us ot report errors properly if an action fails without needing a new field.\n\nif we agree with that usecase i would isntead use a Text field or consider increaseing the size to String(4096).\n\nhttps://docs.sqlalchemy.org/en/20/core/type_basics.html#sqlalchemy.types.TEXT\n\nits easy to make that change now but non trivial to do later.","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"853a9daba266d9d6bcadf16f992c28128b0cfdc2","unresolved":false,"context_lines":[{"line_number":118,"context_line":"Data model impact"},{"line_number":119,"context_line":"-----------------"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"* Add a new column `skip_reason` of type `String(255)` in Actions table."},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"REST API impact"},{"line_number":124,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"772d6763_a39da804","line":121,"in_reply_to":"271574c2_bd47baac","updated":"2025-06-16 17:46:26.000000000","message":"Acknowledged","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"27d6630c86b4551de2ecd7912491a55b6f1396e3","unresolved":true,"context_lines":[{"line_number":118,"context_line":"Data model impact"},{"line_number":119,"context_line":"-----------------"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"* Add a new column `skip_reason` of type `String(255)` in Actions table."},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"REST API impact"},{"line_number":124,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"271574c2_bd47baac","line":121,"in_reply_to":"579905f2_1e9c5a1b","updated":"2025-06-04 14:41:58.000000000","message":"My approach for microversioning the reason field was just hiding it in previous version calls, similar to https://github.com/openstack/watcher/blob/59757249bbbbb45bead28bfb2899242aea97dfc1/watcher/api/controllers/v1/audit.py#L68-L79 , is that fine?","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"853a9daba266d9d6bcadf16f992c28128b0cfdc2","unresolved":false,"context_lines":[{"line_number":118,"context_line":"Data model impact"},{"line_number":119,"context_line":"-----------------"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"* Add a new column `skip_reason` of type `String(255)` in Actions table."},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"REST API impact"},{"line_number":124,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"0e879f9b_61167b1b","line":121,"in_reply_to":"899f0af2_f323a105","updated":"2025-06-16 17:46:26.000000000","message":"Done","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"27d6630c86b4551de2ecd7912491a55b6f1396e3","unresolved":true,"context_lines":[{"line_number":118,"context_line":"Data model impact"},{"line_number":119,"context_line":"-----------------"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"* Add a new column `skip_reason` of type `String(255)` in Actions table."},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"REST API impact"},{"line_number":124,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"899f0af2_f323a105","line":121,"in_reply_to":"af848a38_a157eb55","updated":"2025-06-04 14:41:58.000000000","message":"+1 , I like the idea a lot. We can do it it also for audits, but that\u0027s out of the scope of this spec.","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"64068bd5aec9961318ebe1f75e02cd16b9f10da2","unresolved":true,"context_lines":[{"line_number":167,"context_line":"              \"path\": \"/skip_reason\""},{"line_number":168,"context_line":"          },"},{"line_number":169,"context_line":"      ]"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"  Trying to patch an unexisting Action will return a 404 error."},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"  Request to skip an action using Patch API method in any state different"}],"source_content_type":"text/x-rst","patch_set":3,"id":"758900f3_038bf73c","line":170,"updated":"2025-06-03 18:05:54.000000000","message":"as much as i dislike this api approch its consitent with the other apis","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"853a9daba266d9d6bcadf16f992c28128b0cfdc2","unresolved":false,"context_lines":[{"line_number":167,"context_line":"              \"path\": \"/skip_reason\""},{"line_number":168,"context_line":"          },"},{"line_number":169,"context_line":"      ]"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"  Trying to patch an unexisting Action will return a 404 error."},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"  Request to skip an action using Patch API method in any state different"}],"source_content_type":"text/x-rst","patch_set":3,"id":"e54f33f0_8a256a15","line":170,"in_reply_to":"758900f3_038bf73c","updated":"2025-06-16 17:46:26.000000000","message":"Acknowledged","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"64068bd5aec9961318ebe1f75e02cd16b9f10da2","unresolved":true,"context_lines":[{"line_number":171,"context_line":"  Trying to patch an unexisting Action will return a 404 error."},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"  Request to skip an action using Patch API method in any state different"},{"line_number":174,"context_line":"  that PENDING will return a 400 error."},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"  The API Patch call will allow to modify the `skip_reason` field for an"},{"line_number":177,"context_line":"  action which is in SKIPPED state."}],"source_content_type":"text/x-rst","patch_set":3,"id":"c3c54dc1_e34ab41e","line":174,"updated":"2025-06-03 18:05:54.000000000","message":"This should be a 409 conflict not a 400\n\n400 implies the syntax of the request is incorrect\n\nthat is not the case 409 confict is more correct because it imples the current status of the resouce that is being modified is not valid for the given request.\n\n```\nThe HTTP 409 Conflict client error response status code indicates a request conflict with the current state of the target resource.\n```\n\nhttps://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status/409","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"853a9daba266d9d6bcadf16f992c28128b0cfdc2","unresolved":false,"context_lines":[{"line_number":171,"context_line":"  Trying to patch an unexisting Action will return a 404 error."},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"  Request to skip an action using Patch API method in any state different"},{"line_number":174,"context_line":"  that PENDING will return a 400 error."},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"  The API Patch call will allow to modify the `skip_reason` field for an"},{"line_number":177,"context_line":"  action which is in SKIPPED state."}],"source_content_type":"text/x-rst","patch_set":3,"id":"3f4422ca_b52c94bb","line":174,"in_reply_to":"aba0be27_15b683a2","updated":"2025-06-16 17:46:26.000000000","message":"Done","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"27d6630c86b4551de2ecd7912491a55b6f1396e3","unresolved":true,"context_lines":[{"line_number":171,"context_line":"  Trying to patch an unexisting Action will return a 404 error."},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"  Request to skip an action using Patch API method in any state different"},{"line_number":174,"context_line":"  that PENDING will return a 400 error."},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"  The API Patch call will allow to modify the `skip_reason` field for an"},{"line_number":177,"context_line":"  action which is in SKIPPED state."}],"source_content_type":"text/x-rst","patch_set":3,"id":"aba0be27_15b683a2","line":174,"in_reply_to":"c3c54dc1_e34ab41e","updated":"2025-06-04 14:41:58.000000000","message":"ack","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"64068bd5aec9961318ebe1f75e02cd16b9f10da2","unresolved":true,"context_lines":[{"line_number":185,"context_line":"Notifications impact"},{"line_number":186,"context_line":"--------------------"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"No notification impact."},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"Other end user impact"},{"line_number":191,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"250514d7_6fa1cf45","line":188,"updated":"2025-06-03 18:05:54.000000000","message":"technially the notication object version should change and the payload will now suprpot a new state.\n\nWatcher is again not doing this properly today.\n\n\nif you look at how we did this in nova when we added a new value to the hw_vif_model we bumped both the actual object and the notificaiton object.\n\n\nhttps://github.com/openstack/nova/commit/638efe3cd5a2af9aeef5bd26e9b55bde354b9767#diff-666bba5f0a334a95776789827358036bee258476a1421adb016677cce4c167d6\n\nwe normlaly just note the notification object will be updated with teh new value\n\nhttps://specs.openstack.org/openstack/nova-specs/specs/train/implemented/libvirt-pmu-configuration.html#notifications-impact","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"853a9daba266d9d6bcadf16f992c28128b0cfdc2","unresolved":false,"context_lines":[{"line_number":185,"context_line":"Notifications impact"},{"line_number":186,"context_line":"--------------------"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"No notification impact."},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"Other end user impact"},{"line_number":191,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"e04fc74d_8b594b9d","line":188,"in_reply_to":"250514d7_6fa1cf45","updated":"2025-06-16 17:46:26.000000000","message":"Done","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"64068bd5aec9961318ebe1f75e02cd16b9f10da2","unresolved":true,"context_lines":[{"line_number":202,"context_line":""},{"line_number":203,"context_line":"  .. code-block::"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    openstack optimize action skip --reason \u003creason\u003e \u003caction id\u003e"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"Similar functionalities will be implemented in the watcher-dashboard package"},{"line_number":208,"context_line":"to perform the same actions and get similar information from the horizon"}],"source_content_type":"text/x-rst","patch_set":3,"id":"66d9a315_0df10027","line":205,"range":{"start_line":205,"start_character":3,"end_line":205,"end_character":64},"updated":"2025-06-03 18:05:54.000000000","message":"hum we could but that would not be following the standard convetion\n\nthat said wtacher already deviates form the normal convetion\n\n\nidiomaticlly this should be\n\nopenstack action set --state\u003d\u0027Skipped\" --reasoun \"\" \u003caction uuid\u003e\n\n\nif we had `openstack optimize action skip --reason \u003creason\u003e \u003caction id\u003e`\n\nthat would generally imply we are calling a sub endpoint on action\n\ni.e. /action/\u003cid\u003e/\u003coperation\u003e\nwhere operation is cancel or skip\n\nlooking at the the current watcher client plugin this is a common problem for all ressouces. i.e. watcher is not using set/unset for update.\n\n\nwith that in mind we likely shoudl follow watcher convetion for now\n\n`openstack optimize action update --state skip --reason \u003creason\u003e \u003caction id\u003e`\n\nwe are not currently rendering the osc version fo our plugin docs.\nbut looking at the standaone version that would follwo the exsitign pattern for action_plans\n\nhttps://docs.openstack.org/python-watcherclient/2024.2/cli/details.html#watcher-actionplan-update\n\nthis is also consitent with audit update\n\nhttps://docs.openstack.org/python-watcherclient/2024.2/cli/details.html#watcher-audit-update","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"853a9daba266d9d6bcadf16f992c28128b0cfdc2","unresolved":false,"context_lines":[{"line_number":202,"context_line":""},{"line_number":203,"context_line":"  .. code-block::"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    openstack optimize action skip --reason \u003creason\u003e \u003caction id\u003e"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"Similar functionalities will be implemented in the watcher-dashboard package"},{"line_number":208,"context_line":"to perform the same actions and get similar information from the horizon"}],"source_content_type":"text/x-rst","patch_set":3,"id":"b0ffe105_45d32ab9","line":205,"range":{"start_line":205,"start_character":3,"end_line":205,"end_character":64},"in_reply_to":"5a5d5c02_84f5abb2","updated":"2025-06-16 17:46:26.000000000","message":"Done","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"27d6630c86b4551de2ecd7912491a55b6f1396e3","unresolved":true,"context_lines":[{"line_number":202,"context_line":""},{"line_number":203,"context_line":"  .. code-block::"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    openstack optimize action skip --reason \u003creason\u003e \u003caction id\u003e"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"Similar functionalities will be implemented in the watcher-dashboard package"},{"line_number":208,"context_line":"to perform the same actions and get similar information from the horizon"}],"source_content_type":"text/x-rst","patch_set":3,"id":"5a5d5c02_84f5abb2","line":205,"range":{"start_line":205,"start_character":3,"end_line":205,"end_character":64},"in_reply_to":"66d9a315_0df10027","updated":"2025-06-04 14:41:58.000000000","message":"LGTM I took the idea of `openstack optimize action skip` of the existing `openstack optimize actionplan cancel` but update is more consistent as it is also included in audit, audittemplate and actionplan.","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"64068bd5aec9961318ebe1f75e02cd16b9f10da2","unresolved":true,"context_lines":[{"line_number":242,"context_line":"  to SKIPPED state if pre_condition raises it."},{"line_number":243,"context_line":"* Add a new patch method to the actions api to skip pending actions."},{"line_number":244,"context_line":"* Add support in watcherclient."},{"line_number":245,"context_line":"* Add support in watcher-dashboard."},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"Dependencies"},{"line_number":248,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":3,"id":"f4fc07c4_b4d0a43a","line":245,"updated":"2025-06-03 18:05:54.000000000","message":"ack\n\nwe dont need to go into much detail but you should list which views will support skipign the action in the proposed changes section.\n\ni assume the action plan view will be updated to allow skiping indivgual actions.\n\nwe coudl also supprot this directly on the action view but i think doing it for the action_plans is more intuitive.\n\ni.e., we should be able to skip the actions from the related actions panel in the action plan detail view.\n\ndoing it form the actions view is also ok bu tyou will need to manually filter by the action_plan id to be able to reason about it and that feel less user friendly.","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"853a9daba266d9d6bcadf16f992c28128b0cfdc2","unresolved":false,"context_lines":[{"line_number":242,"context_line":"  to SKIPPED state if pre_condition raises it."},{"line_number":243,"context_line":"* Add a new patch method to the actions api to skip pending actions."},{"line_number":244,"context_line":"* Add support in watcherclient."},{"line_number":245,"context_line":"* Add support in watcher-dashboard."},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"Dependencies"},{"line_number":248,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":3,"id":"9e5ccb46_ca39e44b","line":245,"in_reply_to":"f4fc07c4_b4d0a43a","updated":"2025-06-16 17:46:26.000000000","message":"Done","commit_id":"42f761c50873904476380de116227d441849b8f2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"853a9daba266d9d6bcadf16f992c28128b0cfdc2","unresolved":true,"context_lines":[{"line_number":75,"context_line":"After this change, additionally, the state machine for Actions will allow"},{"line_number":76,"context_line":"following state transition:"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"* PENDING -\u003e SKIPPED (action is excluded from execution before started)"},{"line_number":79,"context_line":"* PENDING -\u003e FAILED (action failed in pre-condition with unexpected error)"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"Note that the transition from PENDING to SKIPPED can be triggered by the"}],"source_content_type":"text/x-rst","patch_set":5,"id":"c6413e2f_d89da8c8","line":78,"range":{"start_line":78,"start_character":0,"end_line":78,"end_character":71},"updated":"2025-06-16 17:46:26.000000000","message":"nit: This could be a little clearer to sate that it was excluded because of an expected a pre condition failure i.e. the vm to be moved was deleted.\n```suggestion\n* PENDING -\u003e SKIPPED (action is excluded from by its pre-conditions)\n```\n\nyou have covered that in the note below so this is not deservince of a -1 its just a nit","commit_id":"149bff29c2043b5edb2f6377c21e06ca895718a8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"853a9daba266d9d6bcadf16f992c28128b0cfdc2","unresolved":true,"context_lines":[{"line_number":78,"context_line":"* PENDING -\u003e SKIPPED (action is excluded from execution before started)"},{"line_number":79,"context_line":"* PENDING -\u003e FAILED (action failed in pre-condition with unexpected error)"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"Note that the transition from PENDING to SKIPPED can be triggered by the"},{"line_number":82,"context_line":"cloud administrator using an API call or automatically when watcher detects"},{"line_number":83,"context_line":"certain predefined conditions in pre_condition execution. Transitioning an"},{"line_number":84,"context_line":"action from PENDING to ONGOING should imply that pre_condition execution"}],"source_content_type":"text/x-rst","patch_set":5,"id":"c09071fd_c1e63c96","line":81,"updated":"2025-06-16 17:46:26.000000000","message":"it would be better to use an actual sphix/rst Note here\n\n```suggestion\n.. Note::\n\n    The transition from PENDING to SKIPPED can be triggered by the\n```","commit_id":"149bff29c2043b5edb2f6377c21e06ca895718a8"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"d9bcd337e7ad2a05d8523c10ee47de4596379fab","unresolved":true,"context_lines":[{"line_number":78,"context_line":"* PENDING -\u003e SKIPPED (action is excluded from execution before started)"},{"line_number":79,"context_line":"* PENDING -\u003e FAILED (action failed in pre-condition with unexpected error)"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"Note that the transition from PENDING to SKIPPED can be triggered by the"},{"line_number":82,"context_line":"cloud administrator using an API call or automatically when watcher detects"},{"line_number":83,"context_line":"certain predefined conditions in pre_condition execution. Transitioning an"},{"line_number":84,"context_line":"action from PENDING to ONGOING should imply that pre_condition execution"}],"source_content_type":"text/x-rst","patch_set":5,"id":"a0b5165c_3e4c1390","line":81,"in_reply_to":"c09071fd_c1e63c96","updated":"2025-06-17 17:30:25.000000000","message":"+1","commit_id":"149bff29c2043b5edb2f6377c21e06ca895718a8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"853a9daba266d9d6bcadf16f992c28128b0cfdc2","unresolved":true,"context_lines":[{"line_number":92,"context_line":"user with details about the actual result of the execution, a new field will"},{"line_number":93,"context_line":"be added to the action plan `status_message` which will be updated when one"},{"line_number":94,"context_line":"or more actions are skipped with relevant information about the skipped"},{"line_number":95,"context_line":"actions."},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"For the pre-condition detection use case, a new class of Exception"},{"line_number":98,"context_line":"`ActionSkipException` will be created. When pre_condition raises it, the"}],"source_content_type":"text/x-rst","patch_set":5,"id":"7c5c1902_cf7be330","line":95,"updated":"2025-06-16 17:46:26.000000000","message":"so its slightly out of scope but as a follow up to this work it woudl be nice to also store the reason why they failed in the status_message too.\n\nso for now lets focus on using it for the skipped status but as a strech goal once the inital feature is done we shoudl also populate the message for failed states too.","commit_id":"149bff29c2043b5edb2f6377c21e06ca895718a8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"853a9daba266d9d6bcadf16f992c28128b0cfdc2","unresolved":true,"context_lines":[{"line_number":97,"context_line":"For the pre-condition detection use case, a new class of Exception"},{"line_number":98,"context_line":"`ActionSkipException` will be created. When pre_condition raises it, the"},{"line_number":99,"context_line":"ongoing action will be switched to SKIPPED state, and `status_message` will be"},{"line_number":100,"context_line":"added based on the exception message."},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"For the admin override use case, a new api call Patch method will be added to"},{"line_number":103,"context_line":"the Actions which will allow to change the state from PENDING to SKIPPED"}],"source_content_type":"text/x-rst","patch_set":5,"id":"9e29d804_92861c9b","line":100,"updated":"2025-06-16 17:46:26.000000000","message":"based on is probably the correct apprroch.\n\nwatcher is an admin only service but we may decied to sanatize the excption message depending on what it contains.\n\nfor nova we provide the full excptions to admins but\nonly the excaption name to non admins so maybe its fine to provide the full excption now and we can revisit this if we want to actully support non admin users in the future.","commit_id":"149bff29c2043b5edb2f6377c21e06ca895718a8"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"d9bcd337e7ad2a05d8523c10ee47de4596379fab","unresolved":true,"context_lines":[{"line_number":240,"context_line":"              \"path\": \"/state\""},{"line_number":241,"context_line":"          },"},{"line_number":242,"context_line":"          {"},{"line_number":243,"context_line":"              \"op\": \"add\","},{"line_number":244,"context_line":"              \"value\": \"Exclude migration of intance foo\","},{"line_number":245,"context_line":"              \"path\": \"/status_message\""},{"line_number":246,"context_line":"          },"}],"source_content_type":"text/x-rst","patch_set":5,"id":"749ca193_74aba01e","line":243,"range":{"start_line":243,"start_character":21,"end_line":243,"end_character":24},"updated":"2025-06-17 17:30:25.000000000","message":"isn\u0027t a \u0027replace\u0027 too? with the initial value as an empty string. I\u0027m not an expert, but \u0027add\u0027 sounds like you would append messages. Maybe any of them works here\nthe \u0027replace\u0027 op will be needed when updating only the status message, which you mention below.","commit_id":"149bff29c2043b5edb2f6377c21e06ca895718a8"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"155b403e451c86db157271baa9b62d3b98a9eb6a","unresolved":true,"context_lines":[{"line_number":240,"context_line":"              \"path\": \"/state\""},{"line_number":241,"context_line":"          },"},{"line_number":242,"context_line":"          {"},{"line_number":243,"context_line":"              \"op\": \"add\","},{"line_number":244,"context_line":"              \"value\": \"Exclude migration of intance foo\","},{"line_number":245,"context_line":"              \"path\": \"/status_message\""},{"line_number":246,"context_line":"          },"}],"source_content_type":"text/x-rst","patch_set":5,"id":"e2776c92_abc5c433","line":243,"range":{"start_line":243,"start_character":21,"end_line":243,"end_character":24},"in_reply_to":"749ca193_74aba01e","updated":"2025-06-17 18:32:59.000000000","message":"it depend on the inital value but i would expect ti to be null i.e. pyhton None in which case add is correct.\n\nrepalce might work but it kidn of depend on if we mark the field as nullable or not.\n\nalso whtere it would be add or repalce may depend on the exact behavior with the empty string.\n\nadd is really for adding a new filed where as repalce is for settign the value of an existing filed on the object. \n\nwe may need to test this and then document the correct op after the fact.","commit_id":"149bff29c2043b5edb2f6377c21e06ca895718a8"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"fb07139ebd91ad6c2f8cc97c50ed59b0ef762022","unresolved":false,"context_lines":[{"line_number":240,"context_line":"              \"path\": \"/state\""},{"line_number":241,"context_line":"          },"},{"line_number":242,"context_line":"          {"},{"line_number":243,"context_line":"              \"op\": \"add\","},{"line_number":244,"context_line":"              \"value\": \"Exclude migration of intance foo\","},{"line_number":245,"context_line":"              \"path\": \"/status_message\""},{"line_number":246,"context_line":"          },"}],"source_content_type":"text/x-rst","patch_set":5,"id":"47a2142c_e88a21b2","line":243,"range":{"start_line":243,"start_character":21,"end_line":243,"end_character":24},"in_reply_to":"c5752edb_74b4608c","updated":"2025-06-23 18:19:00.000000000","message":"ack, we can defer this to PoC/implementation phase. No need to hold the spec for that.","commit_id":"149bff29c2043b5edb2f6377c21e06ca895718a8"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"f8d06159514f81ca881e92ff826a0aeda3696193","unresolved":true,"context_lines":[{"line_number":240,"context_line":"              \"path\": \"/state\""},{"line_number":241,"context_line":"          },"},{"line_number":242,"context_line":"          {"},{"line_number":243,"context_line":"              \"op\": \"add\","},{"line_number":244,"context_line":"              \"value\": \"Exclude migration of intance foo\","},{"line_number":245,"context_line":"              \"path\": \"/status_message\""},{"line_number":246,"context_line":"          },"}],"source_content_type":"text/x-rst","patch_set":5,"id":"c5752edb_74b4608c","line":243,"range":{"start_line":243,"start_character":21,"end_line":243,"end_character":24},"in_reply_to":"e2776c92_abc5c433","updated":"2025-06-18 07:08:40.000000000","message":"Yes, that\u0027s the case i was thinking, the file will be initially empty so we need to add the first time.","commit_id":"149bff29c2043b5edb2f6377c21e06ca895718a8"}]}