)]}'
{"mistral/workflow/direct_workflow.py":[{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"1c841936e6859edfe198006ec6fdc7eed33f88c6","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        if not t_specs_names:"},{"line_number":53,"context_line":"            return []"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        if not task_spec.get_join():"},{"line_number":56,"context_line":"            return self._get_task_executions("},{"line_number":57,"context_line":"                name\u003dt_specs_names[0],  # not a join, has just one parent"},{"line_number":58,"context_line":"                state\u003d{\u0027in\u0027: (states.SUCCESS, states.ERROR, states.CANCELLED)},"},{"line_number":59,"context_line":"                processed\u003dTrue"},{"line_number":60,"context_line":"            )"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        t_execs_candidates \u003d self._get_task_executions("},{"line_number":63,"context_line":"            name\u003d{\u0027in\u0027: t_specs_names},"},{"line_number":64,"context_line":"            state\u003d{\u0027in\u0027: (states.SUCCESS, states.ERROR, states.CANCELLED)},"},{"line_number":65,"context_line":"        )"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        t_execs \u003d []"},{"line_number":68,"context_line":"        for t_ex in t_execs_candidates:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_aaf82f49","line":65,"range":{"start_line":55,"start_character":8,"end_line":65,"end_character":9},"updated":"2019-06-17 04:00:01.000000000","message":"Not sure I understand this code. The first \"if\" clause is just a special case of the second one, no? If it\u0027s a \"join\" then t_specs_names should contain just one item and the second \"if\" will work well for it too. One difference is \"processed\u003dTrue\" but I\u0027m not sure I understand the logic here. You also use only terminal states, why? The method name doesn\u0027t tell anything about completed/processed task executions. May be we need to create another one or change the name of this method?","commit_id":"b0fb101c47d0984cdcd34cb6c48ece066c82599e"},{"author":{"_account_id":15895,"name":"Adriano Petrich","email":"apetrich@redhat.com","username":"apetrich"},"change_message_id":"9dbb711ab04bce33a1de76fdef35361274faee1f","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        if not t_specs_names:"},{"line_number":53,"context_line":"            return []"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        if not task_spec.get_join():"},{"line_number":56,"context_line":"            return self._get_task_executions("},{"line_number":57,"context_line":"                name\u003dt_specs_names[0],  # not a join, has just one parent"},{"line_number":58,"context_line":"                state\u003d{\u0027in\u0027: (states.SUCCESS, states.ERROR, states.CANCELLED)},"},{"line_number":59,"context_line":"                processed\u003dTrue"},{"line_number":60,"context_line":"            )"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        t_execs_candidates \u003d self._get_task_executions("},{"line_number":63,"context_line":"            name\u003d{\u0027in\u0027: t_specs_names},"},{"line_number":64,"context_line":"            state\u003d{\u0027in\u0027: (states.SUCCESS, states.ERROR, states.CANCELLED)},"},{"line_number":65,"context_line":"        )"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        t_execs \u003d []"},{"line_number":68,"context_line":"        for t_ex in t_execs_candidates:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_f85266de","line":65,"range":{"start_line":55,"start_character":8,"end_line":65,"end_character":9},"in_reply_to":"9fb8cfa7_96747200","updated":"2019-06-18 07:24:29.000000000","message":"I\u0027d +2 for the code, but my only issue is that this piece takes a while to understand. I\u0027d love some comments here because when we ever have to do maintenance here understanding all this back is going to be a bit hard.","commit_id":"b0fb101c47d0984cdcd34cb6c48ece066c82599e"},{"author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"change_message_id":"0dc17aa47058f639a01de30c24402f3f9d287b74","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        if not t_specs_names:"},{"line_number":53,"context_line":"            return []"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        if not task_spec.get_join():"},{"line_number":56,"context_line":"            return self._get_task_executions("},{"line_number":57,"context_line":"                name\u003dt_specs_names[0],  # not a join, has just one parent"},{"line_number":58,"context_line":"                state\u003d{\u0027in\u0027: (states.SUCCESS, states.ERROR, states.CANCELLED)},"},{"line_number":59,"context_line":"                processed\u003dTrue"},{"line_number":60,"context_line":"            )"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        t_execs_candidates \u003d self._get_task_executions("},{"line_number":63,"context_line":"            name\u003d{\u0027in\u0027: t_specs_names},"},{"line_number":64,"context_line":"            state\u003d{\u0027in\u0027: (states.SUCCESS, states.ERROR, states.CANCELLED)},"},{"line_number":65,"context_line":"        )"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        t_execs \u003d []"},{"line_number":68,"context_line":"        for t_ex in t_execs_candidates:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_96747200","line":65,"range":{"start_line":55,"start_character":8,"end_line":65,"end_character":9},"in_reply_to":"9fb8cfa7_aaf82f49","updated":"2019-06-17 08:24:47.000000000","message":"Done","commit_id":"b0fb101c47d0984cdcd34cb6c48ece066c82599e"},{"author":{"_account_id":11391,"name":"Mikhail Fedosin","email":"mfedosin@redhat.com","username":"fedosinme"},"change_message_id":"de6c73a7057ada92bd1e9018300a8ced3507f3a3","unresolved":false,"context_lines":[{"line_number":52,"context_line":"        if not t_specs_names:"},{"line_number":53,"context_line":"            return []"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        if not task_spec.get_join():"},{"line_number":56,"context_line":"            return self._get_task_executions("},{"line_number":57,"context_line":"                name\u003dt_specs_names[0],  # not a join, has just one parent"},{"line_number":58,"context_line":"                state\u003d{\u0027in\u0027: (states.SUCCESS, states.ERROR, states.CANCELLED)},"},{"line_number":59,"context_line":"                processed\u003dTrue"},{"line_number":60,"context_line":"            )"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        t_execs_candidates \u003d self._get_task_executions("},{"line_number":63,"context_line":"            name\u003d{\u0027in\u0027: t_specs_names},"},{"line_number":64,"context_line":"            state\u003d{\u0027in\u0027: (states.SUCCESS, states.ERROR, states.CANCELLED)},"},{"line_number":65,"context_line":"        )"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        t_execs \u003d []"},{"line_number":68,"context_line":"        for t_ex in t_execs_candidates:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_e7ee43ec","line":65,"range":{"start_line":55,"start_character":8,"end_line":65,"end_character":9},"in_reply_to":"9fb8cfa7_f85266de","updated":"2019-06-18 11:44:27.000000000","message":"Well, it\u0027s pretty easy... Without this patch we download all inbound task executions for a task spec and then we iterate through them, choosing only those for which _is_upstream_task_execution returns True.\n\nThey are:\n1. All completed tasks.\n2. For non-join task spec, if the parent task was processed.\n3. For join task spec, only if there is a transition to the join task. This part is calculated by _get_induced_join_state method.\n\n^ This logic can be significantly improved. ^\n\n1. Obvious optimization: if our task spec doesn\u0027t have inbound tasks - return [] immediately without making a db request.\n2. Instead of downloading all task executions and analyze them in-place, it\u0027s much wiser to add query filters:\n2.1 For both cases (join and non-join) we need only completed task executions.\n2.2 For non-join tasks we need only processed tasks.\n3. The most unobvious part: why we don\u0027t need to call _get_induced_join_state. Previously it was obligatory, because in general we couldn\u0027t determine if there is a transition between two task executions without evaluating the context. But now, when \"next_tasks\" field was introduced, we can easily do that just by looking in this list. In other words if the task spec name is in the task execution \"next_tasks\" list.","commit_id":"b0fb101c47d0984cdcd34cb6c48ece066c82599e"}]}