ephraimbuddy commented on code in PR #56660:
URL: https://github.com/apache/airflow/pull/56660#discussion_r2432379738


##########
airflow-core/src/airflow/serialization/serialized_objects.py:
##########
@@ -2318,7 +2318,7 @@ def _create_orm_dagrun(
             select(DagModel.bundle_version).where(DagModel.dag_id == 
dag.dag_id),
         )
     dag_version = DagVersion.get_latest_version(dag.dag_id, session=session)
-    if not dag_version:
+    if not dag_version and triggering_user_name != "dag_test":

Review Comment:
   > First question: why do we need a orm dag object anyway? (I think this is 
since that is where the scheduling logic is?)
   The ORM object here is used to ensure the dag version is linked to the 
created dag run and task instances. Since we don't need that in test, we can 
use the serdag in the test funtion itself:
   
https://github.com/apache/airflow/blob/947f4b980137d75530a60edf8af91e38074da2ae/task-sdk/src/airflow/sdk/definitions/dag.py#L1200-L1210
   
   > Second question: since we know we aren't going to store anything here in 
the db: do we need to call this function, or would a specialised one that just 
creates the orm objects in memory fir the purposes of dag.test be better suited?
   
   I think the serdag used in the dag.test method is enough. I can prevent 
calling the dag_version if triggering_user_name is dag_test. The dag_version is 
only used to ensure dagrun and created TI is linked to a version but tests can 
skip it
   



##########
airflow-core/src/airflow/serialization/serialized_objects.py:
##########
@@ -2318,7 +2318,7 @@ def _create_orm_dagrun(
             select(DagModel.bundle_version).where(DagModel.dag_id == 
dag.dag_id),
         )
     dag_version = DagVersion.get_latest_version(dag.dag_id, session=session)
-    if not dag_version:
+    if not dag_version and triggering_user_name != "dag_test":

Review Comment:
   > First question: why do we need a orm dag object anyway? (I think this is 
since that is where the scheduling logic is?)
   
   
   The ORM object here is used to ensure the dag version is linked to the 
created dag run and task instances. Since we don't need that in test, we can 
use the serdag in the test funtion itself:
   
https://github.com/apache/airflow/blob/947f4b980137d75530a60edf8af91e38074da2ae/task-sdk/src/airflow/sdk/definitions/dag.py#L1200-L1210
   
   > Second question: since we know we aren't going to store anything here in 
the db: do we need to call this function, or would a specialised one that just 
creates the orm objects in memory fir the purposes of dag.test be better suited?
   
   I think the serdag used in the dag.test method is enough. I can prevent 
calling the dag_version if triggering_user_name is dag_test. The dag_version is 
only used to ensure dagrun and created TI is linked to a version but tests can 
skip it
   



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