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 2022/10/18 09:36:28 UTC

[GitHub] [airflow] uranusjr commented on a diff in pull request #27106: Fix warning in airflow tasks test command regarding absence of data_interval

uranusjr commented on code in PR #27106:
URL: https://github.com/apache/airflow/pull/27106#discussion_r997965953


##########
airflow/cli/commands/task_command.py:
##########
@@ -126,10 +126,12 @@ def _get_dag_run(
         dag_run = DagRun(dag.dag_id, run_id=exec_date_or_run_id, execution_date=dag_run_execution_date)
         return dag_run, True
     elif create_if_necessary == "db":
+        data_interval = dag.infer_automated_data_interval(logical_date=dag_run_execution_date)

Review Comment:
   This would not always work since infer_automated_data_interval` is only designed to provide backward compatibility for DAG runs that are created before the data interval concept was introduced, and a DAG using a custom timetable would not work correctly with this method. I think `infer_manual_data_interval` is probably a better choice; it is not technically correct (the call below created a scheduled run), but the exact value of the data interval does not actually matter much here anyway since the run is only supposed to be a stub.



##########
airflow/cli/commands/task_command.py:
##########
@@ -126,10 +126,12 @@ def _get_dag_run(
         dag_run = DagRun(dag.dag_id, run_id=exec_date_or_run_id, execution_date=dag_run_execution_date)
         return dag_run, True
     elif create_if_necessary == "db":
+        data_interval = dag.infer_automated_data_interval(logical_date=dag_run_execution_date)

Review Comment:
   This would not always work since `infer_automated_data_interval` is only designed to provide backward compatibility for DAG runs that are created before the data interval concept was introduced, and a DAG using a custom timetable would not work correctly with this method. I think `infer_manual_data_interval` is probably a better choice; it is not technically correct (the call below created a scheduled run), but the exact value of the data interval does not actually matter much here anyway since the run is only supposed to be a stub.



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