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/02/19 15:13:08 UTC
[GitHub] [airflow] avkirilishin opened a new pull request #21684: Add dependency to the running_deps
avkirilishin opened a new pull request #21684:
URL: https://github.com/apache/airflow/pull/21684
closes: #20461
Problem: Airflow is trying to schedule tasks prior to DAG's start_date.
Solution: Added ExecDateAfterStartDateDep to the dependencies that need to be met for a given task instance to be set to 'RUNNING' state.
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
--
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
[GitHub] [airflow] avkirilishin commented on a change in pull request #21684: Add dependency to the running_deps
Posted by GitBox <gi...@apache.org>.
avkirilishin commented on a change in pull request #21684:
URL: https://github.com/apache/airflow/pull/21684#discussion_r813116831
##########
File path: tests/models/test_dag.py
##########
@@ -2029,14 +2029,14 @@ def test_count_number_queries(self, tasks_count):
class TestDagDecorator(unittest.TestCase):
+ DEFAULT_DATE = timezone.datetime(2016, 1, 1)
DEFAULT_ARGS = {
"owner": "test",
"depends_on_past": True,
- "start_date": timezone.utcnow(),
+ "start_date": DEFAULT_DATE,
Review comment:
It affects `test_dag_param_dagrun_parameterized` and `test_dag_param_resolves` where `execution_date` is `self.DEFAULT_DATE` (2016-01-01), but `execution_date` can not be less than DAG's start_date.
--
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
[GitHub] [airflow] ephraimbuddy commented on a change in pull request #21684: Add dependency to the running_deps
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #21684:
URL: https://github.com/apache/airflow/pull/21684#discussion_r810829508
##########
File path: airflow/ti_deps/deps/exec_date_after_start_date_dep.py
##########
@@ -28,15 +28,19 @@ class ExecDateAfterStartDateDep(BaseTIDep):
@provide_session
def _get_dep_statuses(self, ti, session, dep_context):
- if ti.task.start_date and ti.execution_date < ti.task.start_date:
+ if ti.task.start_date and ti.get_dagrun(session=session).execution_date < ti.task.start_date:
Review comment:
Any reason why we are changing this? I think the ti.execution_date will be available at this point.
##########
File path: tests/models/test_dag.py
##########
@@ -2029,14 +2029,14 @@ def test_count_number_queries(self, tasks_count):
class TestDagDecorator(unittest.TestCase):
+ DEFAULT_DATE = timezone.datetime(2016, 1, 1)
DEFAULT_ARGS = {
"owner": "test",
"depends_on_past": True,
- "start_date": timezone.utcnow(),
+ "start_date": DEFAULT_DATE,
Review comment:
Unrelated?
--
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
[GitHub] [airflow] avkirilishin commented on a change in pull request #21684: Add dependency to the running_deps
Posted by GitBox <gi...@apache.org>.
avkirilishin commented on a change in pull request #21684:
URL: https://github.com/apache/airflow/pull/21684#discussion_r813102311
##########
File path: airflow/ti_deps/deps/exec_date_after_start_date_dep.py
##########
@@ -28,15 +28,19 @@ class ExecDateAfterStartDateDep(BaseTIDep):
@provide_session
def _get_dep_statuses(self, ti, session, dep_context):
- if ti.task.start_date and ti.execution_date < ti.task.start_date:
+ if ti.task.start_date and ti.get_dagrun(session=session).execution_date < ti.task.start_date:
Review comment:
After your commit https://github.com/apache/airflow/commit/80ae70c3d0f6109e974d50726da5a44e58d1b674 DagRun must be loaded first (via get_dagrun) or TaskInstance's execution_date would be None
--
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
[GitHub] [airflow] avkirilishin commented on pull request #21684: Add dependency to the running_deps
Posted by GitBox <gi...@apache.org>.
avkirilishin commented on pull request #21684:
URL: https://github.com/apache/airflow/pull/21684#issuecomment-1048797182
> I don't think this is the right fix. If I understand correctly, the problem is that the scheduler is creating dagruns for those past dates and not that the task instances are being scheduled. The ExecDateAfterStartDateDep currently in place will prevent tasks from getting into queued state if the start date is before the execution date
I explained it here: https://github.com/apache/airflow/issues/20461#issuecomment-1046051990
> I think there are two different problems:
>
> The problem is related to the different logic of the scheduler before and after the update. Maybe there are no tasks in the running dags or something else. Can you show the rows for this dag run in dag, dag_run and task_instance?
>
> _**I agree with you that it is not the right behavior for the scheduler to continue scheduling runs that are earlier than the actual start date of the DAG or Task. It can happen, for example, after turning the dag off and back on. So I made a PR to fix it: https://github.com/apache/airflow/pull/21684**_
--
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