o-nikolas commented on code in PR #53201:
URL: https://github.com/apache/airflow/pull/53201#discussion_r2201478426


##########
airflow-core/src/airflow/models/deadline.py:
##########
@@ -64,6 +70,9 @@ class Deadline(Base):
 
     dagrun = relationship("DagRun", back_populates="deadlines")
 
+    trigger_id = Column(Integer, ForeignKey("trigger.id"), nullable=True)

Review Comment:
   I liked the pattern above of including a comment that explains what this is 
and/or how it's used.



##########
airflow-core/src/airflow/models/deadline.py:
##########
@@ -98,6 +107,33 @@ def _determine_resource() -> tuple[str, str]:
             f"{self.deadline_time} or run: {self.callback}({callback_kwargs})"
         )
 
+    def handle_miss(self, session: Session):
+        """Handle a missed deadline by creating and starting a trigger to run 
the callback."""
+        callback_func = import_string(self.callback)
+        if not asyncio.iscoroutinefunction(callback_func):
+            # TODO: For sync callbacks, use executor instead of trigger
+            logger.error("Sync callbacks not supported yet: %s", self.callback)
+            return
+
+        callback_trigger = DeadlineCallbackTrigger(
+            callback_func=self.callback,
+            callback_kwargs=self.callback_kwargs or {},
+        )
+
+        trigger_orm = Trigger.from_object(callback_trigger)
+        session.add(trigger_orm)
+        session.flush()

Review Comment:
   Should the flush come after the second add()?



##########
airflow-core/src/airflow/models/deadline.py:
##########
@@ -98,6 +107,33 @@ def _determine_resource() -> tuple[str, str]:
             f"{self.deadline_time} or run: {self.callback}({callback_kwargs})"
         )
 
+    def handle_miss(self, session: Session):
+        """Handle a missed deadline by creating and starting a trigger to run 
the callback."""
+        callback_func = import_string(self.callback)

Review Comment:
   This means the callback needs to be both present on the scheduler (where I'm 
assuming this will be called eventually) _and_ in the Triggerer. Is that a 
reasonable restriction to put on the users?



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