jason810496 commented on code in PR #63247:
URL: https://github.com/apache/airflow/pull/63247#discussion_r3013848737


##########
task-sdk/src/airflow/sdk/execution_time/supervisor.py:
##########
@@ -1034,6 +1034,9 @@ def _on_child_started(
             self.kill(signal.SIGKILL)
             raise
 
+        if ti_context.start_date is not None:
+            start_date = ti_context.start_date
+

Review Comment:
   How about moving the line 1024 `start_date = datetime.now(tz=timezone.utc)` 
down here and we could simplify the statement.
   
   Additionally, it would be nice to add comment about the context that why we 
need to set the `ti_context.start_date` in case someone remove it in the future.



##########
task-sdk/tests/task_sdk/execution_time/test_supervisor.py:
##########
@@ -514,19 +514,29 @@ def subprocess_main():
 
         assert rc == -9
 
-    def test_last_chance_exception_handling(self, capfd):

Review Comment:
   Instead of updating the existing `test_last_chance_exception_handling` test 
case, could we have an additional test case to verify the resume start date 
behavior?
   
   Since what we need to verify doesn't seem to match the 
`test_last_chance_exception_handling` case.



-- 
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]

Reply via email to