amoghrajesh commented on code in PR #45106:
URL: https://github.com/apache/airflow/pull/45106#discussion_r1898461440
##########
task_sdk/src/airflow/sdk/api/client.py:
##########
@@ -128,7 +128,11 @@ def finish(self, id: uuid.UUID, state: TerminalTIState,
when: datetime):
"""Tell the API server that this TI has reached a terminal state."""
# TODO: handle the naming better. finish sounds wrong as "even"
deferred is essentially finishing.
body = TITerminalStatePayload(end_date=when,
state=TerminalTIState(state))
+ self.client.patch(f"task-instances/{id}/state",
content=body.model_dump_json())
+ def fail(self, id: uuid.UUID, when: datetime, should_retry: bool):
+ """Tell the API server that this TI has to fail, with or without
retries."""
+ body = TITerminalStatePayload(end_date=when,
state=TerminalTIState.FAILED, should_retry=should_retry)
Review Comment:
I briefly thought about it. Thought that a good balance would be to have a
separate set of "parameters" for failed state in case we need to extend it
later. What do you think?
I think its alright to adjust this inside `finish`, but I fear that it may
grow over time leading to a "cello taped" if else comparison within `fail`. On
the contrary, would make readability better to keep it all inside `finish`,
would also make more sense to do that if we have no more "fail" specific data
we need to send between SDK and Execution api. WDYT? @kaxil
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
