)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"3dc030b076c6b1ec3c1773ea12af5f100977dc58","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"925960d5_d96ec83d","updated":"2026-07-21 10:07:31.000000000","message":"it looks very reasonable overall, but do have some comments.","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"34e5a60d427373fee3ad16bad5a81c078d975c0e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"1a375a94_5135d524","updated":"2026-07-23 10:25:27.000000000","message":"lgtm","commit_id":"1cc635f27f2ed223b7481f4ccfc3a723ea172328"}],"freezer_api/common/check.py":[{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"3dc030b076c6b1ec3c1773ea12af5f100977dc58","unresolved":true,"context_lines":[{"line_number":19,"context_line":"    # Check whether the client can execute every action of the job."},{"line_number":20,"context_line":"    # Takes the list of ``freezer_action`` definitions (not the full job"},{"line_number":21,"context_line":"    # document) so it works the same for create, update and replace handlers."},{"line_number":22,"context_line":"    capabilities \u003d [\"action\", \"mode\", \"storage\", \"engine\"]"},{"line_number":23,"context_line":"    client \u003d client[\"client\"]"},{"line_number":24,"context_line":"    for freezer_action in freezer_actions:"},{"line_number":25,"context_line":"        if not freezer_action:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7adf9880_b756adcf","line":22,"range":{"start_line":22,"start_character":0,"end_line":22,"end_character":58},"updated":"2026-07-21 10:07:31.000000000","message":"Might be worth moving that to common.json_schemas in a follow-up as we seems to store constants like this there","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"3427b56a44f2861083a0bdacb0b56631c5bbd71a","unresolved":false,"context_lines":[{"line_number":19,"context_line":"    # Check whether the client can execute every action of the job."},{"line_number":20,"context_line":"    # Takes the list of ``freezer_action`` definitions (not the full job"},{"line_number":21,"context_line":"    # document) so it works the same for create, update and replace handlers."},{"line_number":22,"context_line":"    capabilities \u003d [\"action\", \"mode\", \"storage\", \"engine\"]"},{"line_number":23,"context_line":"    client \u003d client[\"client\"]"},{"line_number":24,"context_line":"    for freezer_action in freezer_actions:"},{"line_number":25,"context_line":"        if not freezer_action:"}],"source_content_type":"text/x-python","patch_set":3,"id":"081ded83_4472aaf1","line":22,"range":{"start_line":22,"start_character":0,"end_line":22,"end_character":58},"in_reply_to":"2740d9df_23545f70","updated":"2026-07-22 07:24:49.000000000","message":"Acknowledged","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":39044,"name":"Alicja Filipek","display_name":"alaf01","email":"alicja.filipek@cleura.com","username":"alaf01"},"change_message_id":"272c44e02e67820631192e1080af620642063fd5","unresolved":true,"context_lines":[{"line_number":19,"context_line":"    # Check whether the client can execute every action of the job."},{"line_number":20,"context_line":"    # Takes the list of ``freezer_action`` definitions (not the full job"},{"line_number":21,"context_line":"    # document) so it works the same for create, update and replace handlers."},{"line_number":22,"context_line":"    capabilities \u003d [\"action\", \"mode\", \"storage\", \"engine\"]"},{"line_number":23,"context_line":"    client \u003d client[\"client\"]"},{"line_number":24,"context_line":"    for freezer_action in freezer_actions:"},{"line_number":25,"context_line":"        if not freezer_action:"}],"source_content_type":"text/x-python","patch_set":3,"id":"2740d9df_23545f70","line":22,"range":{"start_line":22,"start_character":0,"end_line":22,"end_character":58},"in_reply_to":"7adf9880_b756adcf","updated":"2026-07-21 18:10:16.000000000","message":"yes, we can. Let\u0027s do it in follow up, as this patch is already big enough.","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"}],"freezer_api/db/sqlalchemy/api.py":[{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"3dc030b076c6b1ec3c1773ea12af5f100977dc58","unresolved":true,"context_lines":[{"line_number":617,"context_line":"def delete_action(user_id: str, action_id: str,"},{"line_number":618,"context_line":"                  project_id: str | None \u003d None) -\u003e str:"},{"line_number":619,"context_line":"    # An action may still be referenced by jobs. Soft-delete those JobAction"},{"line_number":620,"context_line":"    # rows in the same transaction so the jobs stop listing a deleted action"},{"line_number":621,"context_line":"    # (the Job.job_actions relationship filters out deleted rows) instead of"},{"line_number":622,"context_line":"    # keeping a dangling reference."},{"line_number":623,"context_line":"    with session_for_write() as session:"}],"source_content_type":"text/x-python","patch_set":3,"id":"119c5a80_030a5d34","line":620,"range":{"start_line":620,"start_character":11,"end_line":620,"end_character":34},"updated":"2026-07-21 10:07:31.000000000","message":"are you sure this is gonna be the same transaction? As I\u0027d assume that `delete_tuple()` is creating the new one regardless?","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":39044,"name":"Alicja Filipek","display_name":"alaf01","email":"alicja.filipek@cleura.com","username":"alaf01"},"change_message_id":"272c44e02e67820631192e1080af620642063fd5","unresolved":true,"context_lines":[{"line_number":617,"context_line":"def delete_action(user_id: str, action_id: str,"},{"line_number":618,"context_line":"                  project_id: str | None \u003d None) -\u003e str:"},{"line_number":619,"context_line":"    # An action may still be referenced by jobs. Soft-delete those JobAction"},{"line_number":620,"context_line":"    # rows in the same transaction so the jobs stop listing a deleted action"},{"line_number":621,"context_line":"    # (the Job.job_actions relationship filters out deleted rows) instead of"},{"line_number":622,"context_line":"    # keeping a dangling reference."},{"line_number":623,"context_line":"    with session_for_write() as session:"}],"source_content_type":"text/x-python","patch_set":3,"id":"82b2d2d0_9733fd4c","line":620,"range":{"start_line":620,"start_character":11,"end_line":620,"end_character":34},"in_reply_to":"119c5a80_030a5d34","updated":"2026-07-21 18:10:16.000000000","message":"Yes, should be the same transaction. In with session_for_write() _get_main_context is used and it reuse the same threading.","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"3427b56a44f2861083a0bdacb0b56631c5bbd71a","unresolved":false,"context_lines":[{"line_number":617,"context_line":"def delete_action(user_id: str, action_id: str,"},{"line_number":618,"context_line":"                  project_id: str | None \u003d None) -\u003e str:"},{"line_number":619,"context_line":"    # An action may still be referenced by jobs. Soft-delete those JobAction"},{"line_number":620,"context_line":"    # rows in the same transaction so the jobs stop listing a deleted action"},{"line_number":621,"context_line":"    # (the Job.job_actions relationship filters out deleted rows) instead of"},{"line_number":622,"context_line":"    # keeping a dangling reference."},{"line_number":623,"context_line":"    with session_for_write() as session:"}],"source_content_type":"text/x-python","patch_set":3,"id":"58d1cf0d_9fef7685","line":620,"range":{"start_line":620,"start_character":11,"end_line":620,"end_character":34},"in_reply_to":"82b2d2d0_9733fd4c","updated":"2026-07-22 07:24:49.000000000","message":"Acknowledged","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"3dc030b076c6b1ec3c1773ea12af5f100977dc58","unresolved":true,"context_lines":[{"line_number":723,"context_line":"def get_action(action_id: str, project_id: str | None \u003d None) -\u003e dict:"},{"line_number":724,"context_line":"    actions \u003d get_tuple(tablename\u003dmodels.Action,"},{"line_number":725,"context_line":"                        tuple_id\u003daction_id, project_id\u003dproject_id)"},{"line_number":726,"context_line":"    if 1 \u003d\u003d len(actions):"},{"line_number":727,"context_line":"        return convert_action_to_dict(actions[0])"},{"line_number":728,"context_line":"    return {}"},{"line_number":729,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"d3ff3265_4d96437b","line":726,"range":{"start_line":726,"start_character":7,"end_line":726,"end_character":24},"updated":"2026-07-21 10:07:31.000000000","message":"given this is still ended up as a changed line, can we do `len(actions) \u003d\u003d 1`, as it hurts to see it like that.","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"3427b56a44f2861083a0bdacb0b56631c5bbd71a","unresolved":false,"context_lines":[{"line_number":723,"context_line":"def get_action(action_id: str, project_id: str | None \u003d None) -\u003e dict:"},{"line_number":724,"context_line":"    actions \u003d get_tuple(tablename\u003dmodels.Action,"},{"line_number":725,"context_line":"                        tuple_id\u003daction_id, project_id\u003dproject_id)"},{"line_number":726,"context_line":"    if 1 \u003d\u003d len(actions):"},{"line_number":727,"context_line":"        return convert_action_to_dict(actions[0])"},{"line_number":728,"context_line":"    return {}"},{"line_number":729,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9206f615_379f0d27","line":726,"range":{"start_line":726,"start_character":7,"end_line":726,"end_character":24},"in_reply_to":"0b1e6da5_c36e1a35","updated":"2026-07-22 07:24:49.000000000","message":"Done","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":39044,"name":"Alicja Filipek","display_name":"alaf01","email":"alicja.filipek@cleura.com","username":"alaf01"},"change_message_id":"272c44e02e67820631192e1080af620642063fd5","unresolved":true,"context_lines":[{"line_number":723,"context_line":"def get_action(action_id: str, project_id: str | None \u003d None) -\u003e dict:"},{"line_number":724,"context_line":"    actions \u003d get_tuple(tablename\u003dmodels.Action,"},{"line_number":725,"context_line":"                        tuple_id\u003daction_id, project_id\u003dproject_id)"},{"line_number":726,"context_line":"    if 1 \u003d\u003d len(actions):"},{"line_number":727,"context_line":"        return convert_action_to_dict(actions[0])"},{"line_number":728,"context_line":"    return {}"},{"line_number":729,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"0b1e6da5_c36e1a35","line":726,"range":{"start_line":726,"start_character":7,"end_line":726,"end_character":24},"in_reply_to":"d3ff3265_4d96437b","updated":"2026-07-21 18:10:16.000000000","message":"Yes. I did not want to change anything that not requires it, but yes, definietly.","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"3dc030b076c6b1ec3c1773ea12af5f100977dc58","unresolved":true,"context_lines":[{"line_number":859,"context_line":"    check_client_capabilities(freezer_actions, client)"},{"line_number":860,"context_line":""},{"line_number":861,"context_line":""},{"line_number":862,"context_line":"class ResolvedAction(NamedTuple):"},{"line_number":863,"context_line":"    action_id: str"},{"line_number":864,"context_line":"    freezer_action: dict"},{"line_number":865,"context_line":""},{"line_number":866,"context_line":""},{"line_number":867,"context_line":"def _resolve_actions_from_job_actions("}],"source_content_type":"text/x-python","patch_set":3,"id":"ab21f397_bf2be477","line":864,"range":{"start_line":862,"start_character":0,"end_line":864,"end_character":24},"updated":"2026-07-21 10:07:31.000000000","message":"Hm, I am thinking of why we\u0027re introducing a new type in here?\n\nIt\u0027s just looks a bit random in here. And I don\u0027t see any arbitrary types for other resources from brief look at it.\nIs it mainly for annotation?\n\nShould we maybe start defining types in some \"common\" place then, if we wanna gradually introduce them for everything, and import from there?\n\nThough, I am on the fence if this is a good idea or not, as I can recall some discussions about - don\u0027t invent types, use built-in ones, unless you absolutely have to. But maybe it was about exception types, not resource types...","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":39044,"name":"Alicja Filipek","display_name":"alaf01","email":"alicja.filipek@cleura.com","username":"alaf01"},"change_message_id":"4161e9e5bd86b31ae9af9b19feb5a8c5807f8964","unresolved":true,"context_lines":[{"line_number":859,"context_line":"    check_client_capabilities(freezer_actions, client)"},{"line_number":860,"context_line":""},{"line_number":861,"context_line":""},{"line_number":862,"context_line":"class ResolvedAction(NamedTuple):"},{"line_number":863,"context_line":"    action_id: str"},{"line_number":864,"context_line":"    freezer_action: dict"},{"line_number":865,"context_line":""},{"line_number":866,"context_line":""},{"line_number":867,"context_line":"def _resolve_actions_from_job_actions("}],"source_content_type":"text/x-python","patch_set":3,"id":"d0d1160e_9b3379d1","line":864,"range":{"start_line":862,"start_character":0,"end_line":864,"end_character":24},"in_reply_to":"79821b29_45f47fdb","updated":"2026-07-22 10:49:43.000000000","message":"In Nova similar solution (but a bit older) is used. Named tuples look like this:\n\n```\nGuestNumaConfig \u003d collections.namedtuple(\n    \u0027GuestNumaConfig\u0027, [\u0027cpuset\u0027, \u0027cputune\u0027, \u0027numaconfig\u0027, \u0027numatune\u0027])\n```\n\nThey are defined in the place, where they are used. The class is defined in line, and for me it looks a bit weird, as a variable with capital letters (even though it is a class).\n\nFunctionality is the same, but NamedTuple from typing is a more modern way of creating data classes. \n\nI would keep it near the code where it is used, and move to some common file once we need it in some other place.\n\nAlso one benefit above the personal preferences: we are iterating and accessing the class data. If this were a plain tuple, it would be easy to lose track of the argument order and mix them up.","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":39044,"name":"Alicja Filipek","display_name":"alaf01","email":"alicja.filipek@cleura.com","username":"alaf01"},"change_message_id":"272c44e02e67820631192e1080af620642063fd5","unresolved":true,"context_lines":[{"line_number":859,"context_line":"    check_client_capabilities(freezer_actions, client)"},{"line_number":860,"context_line":""},{"line_number":861,"context_line":""},{"line_number":862,"context_line":"class ResolvedAction(NamedTuple):"},{"line_number":863,"context_line":"    action_id: str"},{"line_number":864,"context_line":"    freezer_action: dict"},{"line_number":865,"context_line":""},{"line_number":866,"context_line":""},{"line_number":867,"context_line":"def _resolve_actions_from_job_actions("}],"source_content_type":"text/x-python","patch_set":3,"id":"79821b29_45f47fdb","line":864,"range":{"start_line":862,"start_character":0,"end_line":864,"end_character":24},"in_reply_to":"ab21f397_bf2be477","updated":"2026-07-21 18:10:16.000000000","message":"IMHO types are what makes python readible. I added it here, as we perform some kind of operations that are not simple db query, and it it easy to get lost. You are right that we do not have this pattern yet, but I belive long-term it makes code easier to read and more maintainable.","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"340a0bd35835150a45498aeae1c1b652dbbde54e","unresolved":false,"context_lines":[{"line_number":859,"context_line":"    check_client_capabilities(freezer_actions, client)"},{"line_number":860,"context_line":""},{"line_number":861,"context_line":""},{"line_number":862,"context_line":"class ResolvedAction(NamedTuple):"},{"line_number":863,"context_line":"    action_id: str"},{"line_number":864,"context_line":"    freezer_action: dict"},{"line_number":865,"context_line":""},{"line_number":866,"context_line":""},{"line_number":867,"context_line":"def _resolve_actions_from_job_actions("}],"source_content_type":"text/x-python","patch_set":3,"id":"28305c00_fff2bc76","line":864,"range":{"start_line":862,"start_character":0,"end_line":864,"end_character":24},"in_reply_to":"d0d1160e_9b3379d1","updated":"2026-07-23 10:09:16.000000000","message":"Ack, lets leave it as is then.","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"3dc030b076c6b1ec3c1773ea12af5f100977dc58","unresolved":true,"context_lines":[{"line_number":1125,"context_line":"    # Single write transaction: created actions, the job update and the"},{"line_number":1126,"context_line":"    # replaced JobAction rows commit or roll back together."},{"line_number":1127,"context_line":"    with session_for_write():"},{"line_number":1128,"context_line":"        resolved_actions \u003d None"},{"line_number":1129,"context_line":"        if \u0027job_actions\u0027 in valid_patch:"},{"line_number":1130,"context_line":"            client_id \u003d valid_patch.get("},{"line_number":1131,"context_line":"                \u0027client_id\u0027,"},{"line_number":1132,"context_line":"                get_job(project_id\u003dproject_id, job_id\u003djob_id"}],"source_content_type":"text/x-python","patch_set":3,"id":"4d638a40_94a790fb","line":1129,"range":{"start_line":1128,"start_character":0,"end_line":1129,"end_character":40},"updated":"2026-07-21 10:07:31.000000000","message":"make sense to do that condition before starting transaction?","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":39044,"name":"Alicja Filipek","display_name":"alaf01","email":"alicja.filipek@cleura.com","username":"alaf01"},"change_message_id":"272c44e02e67820631192e1080af620642063fd5","unresolved":true,"context_lines":[{"line_number":1125,"context_line":"    # Single write transaction: created actions, the job update and the"},{"line_number":1126,"context_line":"    # replaced JobAction rows commit or roll back together."},{"line_number":1127,"context_line":"    with session_for_write():"},{"line_number":1128,"context_line":"        resolved_actions \u003d None"},{"line_number":1129,"context_line":"        if \u0027job_actions\u0027 in valid_patch:"},{"line_number":1130,"context_line":"            client_id \u003d valid_patch.get("},{"line_number":1131,"context_line":"                \u0027client_id\u0027,"},{"line_number":1132,"context_line":"                get_job(project_id\u003dproject_id, job_id\u003djob_id"}],"source_content_type":"text/x-python","patch_set":3,"id":"940cb47b_4fde3bc6","line":1129,"range":{"start_line":1128,"start_character":0,"end_line":1129,"end_character":40},"in_reply_to":"4d638a40_94a790fb","updated":"2026-07-21 18:10:16.000000000","message":"I\u0027d keep it in transaction for atomicity reason. Resolving actions might create new ones and if the job transacton fails, the added actions should be reverted as well.","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"3427b56a44f2861083a0bdacb0b56631c5bbd71a","unresolved":false,"context_lines":[{"line_number":1125,"context_line":"    # Single write transaction: created actions, the job update and the"},{"line_number":1126,"context_line":"    # replaced JobAction rows commit or roll back together."},{"line_number":1127,"context_line":"    with session_for_write():"},{"line_number":1128,"context_line":"        resolved_actions \u003d None"},{"line_number":1129,"context_line":"        if \u0027job_actions\u0027 in valid_patch:"},{"line_number":1130,"context_line":"            client_id \u003d valid_patch.get("},{"line_number":1131,"context_line":"                \u0027client_id\u0027,"},{"line_number":1132,"context_line":"                get_job(project_id\u003dproject_id, job_id\u003djob_id"}],"source_content_type":"text/x-python","patch_set":3,"id":"508b91bc_9d54afed","line":1129,"range":{"start_line":1128,"start_character":0,"end_line":1129,"end_character":40},"in_reply_to":"940cb47b_4fde3bc6","updated":"2026-07-22 07:24:49.000000000","message":"Ah, my bad, you\u0027re right.","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"3dc030b076c6b1ec3c1773ea12af5f100977dc58","unresolved":true,"context_lines":[{"line_number":1145,"context_line":"        if resolved_actions is not None:"},{"line_number":1146,"context_line":"            _replace_job_actions(job_id, resolved_actions)"},{"line_number":1147,"context_line":""},{"line_number":1148,"context_line":"    return 0"},{"line_number":1149,"context_line":""},{"line_number":1150,"context_line":""},{"line_number":1151,"context_line":"def replace_job(user_id: str, job_id: str, doc: dict,"}],"source_content_type":"text/x-python","patch_set":3,"id":"418d8f53_f0aa4d6c","line":1148,"updated":"2026-07-21 10:07:31.000000000","message":"This is damn weird return I must admit... Does it make sense to do it at all?","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"3427b56a44f2861083a0bdacb0b56631c5bbd71a","unresolved":false,"context_lines":[{"line_number":1145,"context_line":"        if resolved_actions is not None:"},{"line_number":1146,"context_line":"            _replace_job_actions(job_id, resolved_actions)"},{"line_number":1147,"context_line":""},{"line_number":1148,"context_line":"    return 0"},{"line_number":1149,"context_line":""},{"line_number":1150,"context_line":""},{"line_number":1151,"context_line":"def replace_job(user_id: str, job_id: str, doc: dict,"}],"source_content_type":"text/x-python","patch_set":3,"id":"8a0f15ae_e1c22e5f","line":1148,"in_reply_to":"1f0f9665_c27097fb","updated":"2026-07-22 07:24:49.000000000","message":"Acknowledged","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":39044,"name":"Alicja Filipek","display_name":"alaf01","email":"alicja.filipek@cleura.com","username":"alaf01"},"change_message_id":"272c44e02e67820631192e1080af620642063fd5","unresolved":true,"context_lines":[{"line_number":1145,"context_line":"        if resolved_actions is not None:"},{"line_number":1146,"context_line":"            _replace_job_actions(job_id, resolved_actions)"},{"line_number":1147,"context_line":""},{"line_number":1148,"context_line":"    return 0"},{"line_number":1149,"context_line":""},{"line_number":1150,"context_line":""},{"line_number":1151,"context_line":"def replace_job(user_id: str, job_id: str, doc: dict,"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f0f9665_c27097fb","line":1148,"in_reply_to":"418d8f53_f0aa4d6c","updated":"2026-07-21 18:10:16.000000000","message":"This is some residual leftover, in jobs.py JobsResource it is return as a version on_patch. It is obviously not supported, but I would not touch it here.","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"}],"freezer_api/db/sqlalchemy/migrations/versions/a1b2c3d4e5f6_normalize_job_actions.py":[{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"c722ef627d8b2f6e09fd1b7edce4f1cff9bba71b","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"\"\"\""},{"line_number":28,"context_line":"import uuid"},{"line_number":29,"context_line":"from typing import Sequence"},{"line_number":30,"context_line":"from alembic import op"},{"line_number":31,"context_line":"from oslo_log import log"},{"line_number":32,"context_line":"from oslo_serialization import jsonutils as json"}],"source_content_type":"text/x-python","patch_set":2,"id":"e0e0d4b2_e93e2fa7","line":29,"updated":"2026-07-07 11:28:29.000000000","message":"pep8: H306: imports not in alphabetical order (uuid, typing.sequence)","commit_id":"79b09c62a3a012a53bbbe3e0a994d6b0022a1239"},{"author":{"_account_id":22348,"name":"Zuul","username":"zuul","tags":["SERVICE_USER"]},"tag":"autogenerated:zuul:check","change_message_id":"c722ef627d8b2f6e09fd1b7edce4f1cff9bba71b","unresolved":false,"context_lines":[{"line_number":27,"context_line":"\"\"\""},{"line_number":28,"context_line":"import uuid"},{"line_number":29,"context_line":"from typing import Sequence"},{"line_number":30,"context_line":"from alembic import op"},{"line_number":31,"context_line":"from oslo_log import log"},{"line_number":32,"context_line":"from oslo_serialization import jsonutils as json"},{"line_number":33,"context_line":"import sqlalchemy as sa"}],"source_content_type":"text/x-python","patch_set":2,"id":"6f970253_3b75483c","line":30,"updated":"2026-07-07 11:28:29.000000000","message":"pep8: H306: imports not in alphabetical order (typing.sequence, alembic.op)","commit_id":"79b09c62a3a012a53bbbe3e0a994d6b0022a1239"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"3dc030b076c6b1ec3c1773ea12af5f100977dc58","unresolved":true,"context_lines":[{"line_number":42,"context_line":"depends_on: str | Sequence[str] | None \u003d None"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"def _loads(value, on_empty):"},{"line_number":46,"context_line":"    if not value:"},{"line_number":47,"context_line":"        return on_empty"},{"line_number":48,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"52bd6f5e_a539aaf2","line":45,"range":{"start_line":45,"start_character":18,"end_line":45,"end_character":26},"updated":"2026-07-21 10:07:31.000000000","message":"this is kinda weird-looking. Why not to return `None` and just do `loads() or {}` like you did anyway on L180?","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":39044,"name":"Alicja Filipek","display_name":"alaf01","email":"alicja.filipek@cleura.com","username":"alaf01"},"change_message_id":"272c44e02e67820631192e1080af620642063fd5","unresolved":true,"context_lines":[{"line_number":42,"context_line":"depends_on: str | Sequence[str] | None \u003d None"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"def _loads(value, on_empty):"},{"line_number":46,"context_line":"    if not value:"},{"line_number":47,"context_line":"        return on_empty"},{"line_number":48,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"fd9f4e38_b73d2825","line":45,"range":{"start_line":45,"start_character":18,"end_line":45,"end_character":26},"in_reply_to":"52bd6f5e_a539aaf2","updated":"2026-07-21 18:10:16.000000000","message":"true, I just wanted to reuse function for the differernt data types, but \"loads() or {}/[]\" would work as well","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":39044,"name":"Alicja Filipek","display_name":"alaf01","email":"alicja.filipek@cleura.com","username":"alaf01"},"change_message_id":"4161e9e5bd86b31ae9af9b19feb5a8c5807f8964","unresolved":true,"context_lines":[{"line_number":42,"context_line":"depends_on: str | Sequence[str] | None \u003d None"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"def _loads(value, on_empty):"},{"line_number":46,"context_line":"    if not value:"},{"line_number":47,"context_line":"        return on_empty"},{"line_number":48,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"f53c7417_9b9fc1b2","line":45,"range":{"start_line":45,"start_character":18,"end_line":45,"end_character":26},"in_reply_to":"e6bd0b9f_45b341e1","updated":"2026-07-22 10:49:43.000000000","message":"Sure. I think that solution without \u0027on_empty\u0027 looks cleaner and is easier to read, while the functionality stays as it is.","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"34e5a60d427373fee3ad16bad5a81c078d975c0e","unresolved":false,"context_lines":[{"line_number":42,"context_line":"depends_on: str | Sequence[str] | None \u003d None"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"def _loads(value, on_empty):"},{"line_number":46,"context_line":"    if not value:"},{"line_number":47,"context_line":"        return on_empty"},{"line_number":48,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"01fc2cc7_8ef47be7","line":45,"range":{"start_line":45,"start_character":18,"end_line":45,"end_character":26},"in_reply_to":"f53c7417_9b9fc1b2","updated":"2026-07-23 10:25:27.000000000","message":"Done","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"a9911f7b290c31ac9f538874b651d983efe23980","unresolved":true,"context_lines":[{"line_number":42,"context_line":"depends_on: str | Sequence[str] | None \u003d None"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"def _loads(value, on_empty):"},{"line_number":46,"context_line":"    if not value:"},{"line_number":47,"context_line":"        return on_empty"},{"line_number":48,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"e6bd0b9f_45b341e1","line":45,"range":{"start_line":45,"start_character":18,"end_line":45,"end_character":26},"in_reply_to":"fd9f4e38_b73d2825","updated":"2026-07-22 07:50:02.000000000","message":"I was just thinking that it won\u0027t be re-used outside of the migration, and in one of 2 usages you already have `_loads(...) or {}`, so it kinda in between.\n\nyou can drop the `or` from L180 as well though, I don\u0027t mind leaving args here as is.","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"3dc030b076c6b1ec3c1773ea12af5f100977dc58","unresolved":true,"context_lines":[{"line_number":101,"context_line":"        \u0027updated_at\u0027: job_row.updated_at,"},{"line_number":102,"context_line":"    }"},{"line_number":103,"context_line":"    for key in freezer_action_keys:"},{"line_number":104,"context_line":"        if key in freezer_action:"},{"line_number":105,"context_line":"            values[key] \u003d freezer_action.get(key)"},{"line_number":106,"context_line":"    # ``action`` is NOT NULL; make sure it is always populated."},{"line_number":107,"context_line":"    values[\u0027action\u0027] \u003d freezer_action.get(\u0027action\u0027)"},{"line_number":108,"context_line":"    return values"}],"source_content_type":"text/x-python","patch_set":3,"id":"2f152324_9cfd9cd6","line":105,"range":{"start_line":104,"start_character":0,"end_line":105,"end_character":49},"updated":"2026-07-21 10:07:31.000000000","message":"I have hard times reading it, as getting confused a bit\n\n- Do we really care if key is in action? As maybe we wanna define it as None anyway?\n- If we do want to avoid None in values, why is `.get()` still needed?","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":39044,"name":"Alicja Filipek","display_name":"alaf01","email":"alicja.filipek@cleura.com","username":"alaf01"},"change_message_id":"272c44e02e67820631192e1080af620642063fd5","unresolved":true,"context_lines":[{"line_number":101,"context_line":"        \u0027updated_at\u0027: job_row.updated_at,"},{"line_number":102,"context_line":"    }"},{"line_number":103,"context_line":"    for key in freezer_action_keys:"},{"line_number":104,"context_line":"        if key in freezer_action:"},{"line_number":105,"context_line":"            values[key] \u003d freezer_action.get(key)"},{"line_number":106,"context_line":"    # ``action`` is NOT NULL; make sure it is always populated."},{"line_number":107,"context_line":"    values[\u0027action\u0027] \u003d freezer_action.get(\u0027action\u0027)"},{"line_number":108,"context_line":"    return values"}],"source_content_type":"text/x-python","patch_set":3,"id":"3bf4487d_a07cf783","line":105,"range":{"start_line":104,"start_character":0,"end_line":105,"end_character":49},"in_reply_to":"2f152324_9cfd9cd6","updated":"2026-07-21 18:10:16.000000000","message":"I guess you are right, we can assign None in case the key is not in freezer_action. We allow None, I just wanted to copy exactly what we had in json.","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"a9911f7b290c31ac9f538874b651d983efe23980","unresolved":false,"context_lines":[{"line_number":101,"context_line":"        \u0027updated_at\u0027: job_row.updated_at,"},{"line_number":102,"context_line":"    }"},{"line_number":103,"context_line":"    for key in freezer_action_keys:"},{"line_number":104,"context_line":"        if key in freezer_action:"},{"line_number":105,"context_line":"            values[key] \u003d freezer_action.get(key)"},{"line_number":106,"context_line":"    # ``action`` is NOT NULL; make sure it is always populated."},{"line_number":107,"context_line":"    values[\u0027action\u0027] \u003d freezer_action.get(\u0027action\u0027)"},{"line_number":108,"context_line":"    return values"}],"source_content_type":"text/x-python","patch_set":3,"id":"b34dd1f2_f6207f9b","line":105,"range":{"start_line":104,"start_character":0,"end_line":105,"end_character":49},"in_reply_to":"3bf4487d_a07cf783","updated":"2026-07-22 07:50:02.000000000","message":"Acknowledged","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"3dc030b076c6b1ec3c1773ea12af5f100977dc58","unresolved":true,"context_lines":[{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    for job_row in job_rows:"},{"line_number":130,"context_line":"        job_actions_entries \u003d _loads(job_row.job_actions, [])"},{"line_number":131,"context_line":"        if not isinstance(job_actions_entries, list):"},{"line_number":132,"context_line":"            LOG.warning(\u0027Skipping job %s: job_actions is of type %s\u0027,"},{"line_number":133,"context_line":"                        job_row.id, type(job_actions_entries))"},{"line_number":134,"context_line":"            continue"},{"line_number":135,"context_line":"        for position, job_action_entry in enumerate(job_actions_entries):"},{"line_number":136,"context_line":"            if not isinstance(job_action_entry, dict):"}],"source_content_type":"text/x-python","patch_set":3,"id":"faba39c4_fccdfb6d","line":133,"range":{"start_line":131,"start_character":0,"end_line":133,"end_character":62},"updated":"2026-07-21 10:07:31.000000000","message":"how is this possible given the current `_loads()` logic?\n\nAnd also - would not you write up warning in there? So you\u0027re gonna have 2 log messages for the same entry.","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"a9911f7b290c31ac9f538874b651d983efe23980","unresolved":false,"context_lines":[{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    for job_row in job_rows:"},{"line_number":130,"context_line":"        job_actions_entries \u003d _loads(job_row.job_actions, [])"},{"line_number":131,"context_line":"        if not isinstance(job_actions_entries, list):"},{"line_number":132,"context_line":"            LOG.warning(\u0027Skipping job %s: job_actions is of type %s\u0027,"},{"line_number":133,"context_line":"                        job_row.id, type(job_actions_entries))"},{"line_number":134,"context_line":"            continue"},{"line_number":135,"context_line":"        for position, job_action_entry in enumerate(job_actions_entries):"},{"line_number":136,"context_line":"            if not isinstance(job_action_entry, dict):"}],"source_content_type":"text/x-python","patch_set":3,"id":"02c59611_4ff1eb1b","line":133,"range":{"start_line":131,"start_character":0,"end_line":133,"end_character":62},"in_reply_to":"479b268a_7a3bd1bc","updated":"2026-07-22 07:50:02.000000000","message":"aha, ok, gotcha. so it\u0027s basically to verify what we already store in db and avoid migration failures.\nfair enough.","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":39044,"name":"Alicja Filipek","display_name":"alaf01","email":"alicja.filipek@cleura.com","username":"alaf01"},"change_message_id":"272c44e02e67820631192e1080af620642063fd5","unresolved":true,"context_lines":[{"line_number":128,"context_line":""},{"line_number":129,"context_line":"    for job_row in job_rows:"},{"line_number":130,"context_line":"        job_actions_entries \u003d _loads(job_row.job_actions, [])"},{"line_number":131,"context_line":"        if not isinstance(job_actions_entries, list):"},{"line_number":132,"context_line":"            LOG.warning(\u0027Skipping job %s: job_actions is of type %s\u0027,"},{"line_number":133,"context_line":"                        job_row.id, type(job_actions_entries))"},{"line_number":134,"context_line":"            continue"},{"line_number":135,"context_line":"        for position, job_action_entry in enumerate(job_actions_entries):"},{"line_number":136,"context_line":"            if not isinstance(job_action_entry, dict):"}],"source_content_type":"text/x-python","patch_set":3,"id":"479b268a_7a3bd1bc","line":133,"range":{"start_line":131,"start_character":0,"end_line":133,"end_character":62},"in_reply_to":"faba39c4_fccdfb6d","updated":"2026-07-21 18:10:16.000000000","message":"_loads() can return whatever json.loads() return. If the object is not a list (so we have corrupted job_actions json data), this line catches it and invalidates this job_actions. Theoreticaly I could move it to _loads function, but because it covers also dicts (or even any data type) I would keep it in here.\n\nI added a log warning, but I don\u0027t want to raise, as it can break the migration. Why would I have 2 log messages?","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"3dc030b076c6b1ec3c1773ea12af5f100977dc58","unresolved":true,"context_lines":[{"line_number":133,"context_line":"                        job_row.id, type(job_actions_entries))"},{"line_number":134,"context_line":"            continue"},{"line_number":135,"context_line":"        for position, job_action_entry in enumerate(job_actions_entries):"},{"line_number":136,"context_line":"            if not isinstance(job_action_entry, dict):"},{"line_number":137,"context_line":"                LOG.warning(\u0027Skipping job_action %s of job %s: not a \u0027"},{"line_number":138,"context_line":"                            \u0027dictionary\u0027, position, job_row.id)"},{"line_number":139,"context_line":"                continue"},{"line_number":140,"context_line":"            action_id \u003d job_action_entry.get(\u0027action_id\u0027)"},{"line_number":141,"context_line":"            if not action_id or action_id not in known_actions:"},{"line_number":142,"context_line":"                # The referenced action is missing (inline action that was"}],"source_content_type":"text/x-python","patch_set":3,"id":"c091b8fe_32ac0670","line":139,"range":{"start_line":136,"start_character":0,"end_line":139,"end_character":24},"updated":"2026-07-21 10:07:31.000000000","message":"given you have previous check on the list - how it\u0027s possible that enumerate would produce non-dict from the guaranteed list?","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":39044,"name":"Alicja Filipek","display_name":"alaf01","email":"alicja.filipek@cleura.com","username":"alaf01"},"change_message_id":"272c44e02e67820631192e1080af620642063fd5","unresolved":true,"context_lines":[{"line_number":133,"context_line":"                        job_row.id, type(job_actions_entries))"},{"line_number":134,"context_line":"            continue"},{"line_number":135,"context_line":"        for position, job_action_entry in enumerate(job_actions_entries):"},{"line_number":136,"context_line":"            if not isinstance(job_action_entry, dict):"},{"line_number":137,"context_line":"                LOG.warning(\u0027Skipping job_action %s of job %s: not a \u0027"},{"line_number":138,"context_line":"                            \u0027dictionary\u0027, position, job_row.id)"},{"line_number":139,"context_line":"                continue"},{"line_number":140,"context_line":"            action_id \u003d job_action_entry.get(\u0027action_id\u0027)"},{"line_number":141,"context_line":"            if not action_id or action_id not in known_actions:"},{"line_number":142,"context_line":"                # The referenced action is missing (inline action that was"}],"source_content_type":"text/x-python","patch_set":3,"id":"c6907b99_b0d12691","line":139,"range":{"start_line":136,"start_character":0,"end_line":139,"end_character":24},"in_reply_to":"c091b8fe_32ac0670","updated":"2026-07-21 18:10:16.000000000","message":"We have job_actions_entries, which is a guaranted list of, as we expect job_action dicts. But I would not trust whatever we have there. Json.loads() just changes the job_action json to python data type. If the data is corrupted it might give us something else than a dict, so I added thos check","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"},{"author":{"_account_id":28619,"name":"Dmitriy Rabotyagov","email":"noonedeadpunk@gmail.com","username":"noonedeadpunk"},"change_message_id":"a9911f7b290c31ac9f538874b651d983efe23980","unresolved":false,"context_lines":[{"line_number":133,"context_line":"                        job_row.id, type(job_actions_entries))"},{"line_number":134,"context_line":"            continue"},{"line_number":135,"context_line":"        for position, job_action_entry in enumerate(job_actions_entries):"},{"line_number":136,"context_line":"            if not isinstance(job_action_entry, dict):"},{"line_number":137,"context_line":"                LOG.warning(\u0027Skipping job_action %s of job %s: not a \u0027"},{"line_number":138,"context_line":"                            \u0027dictionary\u0027, position, job_row.id)"},{"line_number":139,"context_line":"                continue"},{"line_number":140,"context_line":"            action_id \u003d job_action_entry.get(\u0027action_id\u0027)"},{"line_number":141,"context_line":"            if not action_id or action_id not in known_actions:"},{"line_number":142,"context_line":"                # The referenced action is missing (inline action that was"}],"source_content_type":"text/x-python","patch_set":3,"id":"f6fdf70a_a76031eb","line":139,"range":{"start_line":136,"start_character":0,"end_line":139,"end_character":24},"in_reply_to":"c6907b99_b0d12691","updated":"2026-07-22 07:50:02.000000000","message":"Acknowledged","commit_id":"267e9d66db7769bd48d2a42539a079a12929a296"}]}
