potiuk commented on a change in pull request #20435:
URL: https://github.com/apache/airflow/pull/20435#discussion_r772943562



##########
File path: airflow/models/dagrun.py
##########
@@ -939,14 +939,27 @@ def schedule_tis(self, schedulable_tis: Iterable[TI], 
session: Session = NEW_SES
         return count
 
     @provide_session
-    def get_log_filename_template(self, *, session: Session = NEW_SESSION) -> 
Optional[str]:
-        if self.log_filename_id is None:  # DagRun created before LogFilename 
introduction.
-            template = 
session.query(LogFilename.template).order_by(LogFilename.id).limit(1).scalar()
+    def get_log_filename_template(self, *, session: Session = NEW_SESSION) -> 
str:
+        if self.log_template_id is None:  # DagRun created before LogTemplate 
introduction.
+            template = 
session.query(LogTemplate.filename).order_by(LogTemplate.id).limit(1).scalar()
         else:
-            template = 
session.query(LogFilename.template).filter_by(id=self.log_filename_id).scalar()
+            template = 
session.query(LogTemplate.filename).filter_by(id=self.log_template_id).scalar()
         if template is None:
             raise AirflowException(
-                f"No log_filename entry found for ID {self.log_filename_id!r}. 
"
+                f"No log_template entry found for ID {self.log_template_id!r}. 
"
+                f"Please make sure you set up the metadatabase correctly."

Review comment:
       Should we turn it into a wanrning and fall-back to the first log 
template id ?
   
   I think exception on missing log id is a bit too much as it might result in 
"stack-trace error" (maybe that is intended though?).
   
   Also maybe the message could be somehow more specific. As the current one is 
not really actionable. I presume at the first start of airlfow there will be at 
least one entry created, so I guess the only case it can happen is when 
`log_template_id`  is set in DagRun but missing in the LogTemplate. 
   
   Maybe we should be very explicit "Your dag_run table is corrupted. The entry 
in your dag_run table with dag_run_id: <id> contains invalid log_template_id: 
<log_template_id>. You should set log_template_id for this field to one of the 
existing log_template_ids or set to NULL." . Or something like that.




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