You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/08/19 08:58:13 UTC

[GitHub] [airflow] uranusjr commented on a change in pull request #17122: AIP-39: Add logical_date to OpenAPI DAGRun schema

uranusjr commented on a change in pull request #17122:
URL: https://github.com/apache/airflow/pull/17122#discussion_r691921974



##########
File path: airflow/models/dag.py
##########
@@ -2097,19 +2097,25 @@ def create_dagrun(
         :param data_interval: Data interval of the DagRun
         :type data_interval: tuple[datetime, datetime] | None
         """
-        if run_id and not run_type:
+        if run_id:  # Infer run_type from run_id if needed.
             if not isinstance(run_id, str):
                 raise ValueError(f"`run_id` expected to be a str is {type(run_id)}")
-            run_type: DagRunType = DagRunType.from_run_id(run_id)
-        elif run_type and execution_date:
+            if not run_type:
+                run_type = DagRunType.from_run_id(run_id)
+        elif run_type and execution_date:  # Generate run_id from run_type and execution_date.
             if not isinstance(run_type, DagRunType):
                 raise ValueError(f"`run_type` expected to be a DagRunType is {type(run_type)}")
             run_id = DagRun.generate_run_id(run_type, execution_date)
-        elif not run_id:
+        else:
             raise AirflowException(
                 "Creating DagRun needs either `run_id` or both `run_type` and `execution_date`"
             )

Review comment:
       This is actually fixing an existing but. Previously, when `run_id`, `run_type`, and `execution_date` are *all* provided, the explicit `run_id` value would be ignored and overwritten by an auto-generated value. This fixes the logic and respect the explicit `run_id` value if it’s provided.

##########
File path: airflow/models/dag.py
##########
@@ -2097,19 +2097,25 @@ def create_dagrun(
         :param data_interval: Data interval of the DagRun
         :type data_interval: tuple[datetime, datetime] | None
         """
-        if run_id and not run_type:
+        if run_id:  # Infer run_type from run_id if needed.
             if not isinstance(run_id, str):
                 raise ValueError(f"`run_id` expected to be a str is {type(run_id)}")
-            run_type: DagRunType = DagRunType.from_run_id(run_id)
-        elif run_type and execution_date:
+            if not run_type:
+                run_type = DagRunType.from_run_id(run_id)
+        elif run_type and execution_date:  # Generate run_id from run_type and execution_date.
             if not isinstance(run_type, DagRunType):
                 raise ValueError(f"`run_type` expected to be a DagRunType is {type(run_type)}")
             run_id = DagRun.generate_run_id(run_type, execution_date)
-        elif not run_id:
+        else:
             raise AirflowException(
                 "Creating DagRun needs either `run_id` or both `run_type` and `execution_date`"
             )

Review comment:
       This is actually fixing an existing bug. Previously, when `run_id`, `run_type`, and `execution_date` are *all* provided, the explicit `run_id` value would be ignored and overwritten by an auto-generated value. This fixes the logic and respect the explicit `run_id` value if it’s provided.




-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org