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/05/26 15:27:03 UTC
[GitHub] [airflow] dimonchik-suvorov opened a new pull request #16089: [#16087]: Backfill shouldn't interfere with scheduled run fix
dimonchik-suvorov opened a new pull request #16089:
URL: https://github.com/apache/airflow/pull/16089
closes: #16087
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #16089: [#16087]: Backfill shouldn't interfere with scheduled run fix
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #16089:
URL: https://github.com/apache/airflow/pull/16089#discussion_r649913682
##########
File path: airflow/jobs/backfill_job.py
##########
@@ -689,6 +688,9 @@ def tabulate_tis_set(set_tis: Set[TaskInstance]) -> str:
return err
+ def _get_dag_with_subdags(self):
+ return [self.dag] + self.dag.subdags
+
Review comment:
YAGNI is my general rule -- keep the change as small as sensible for now, if we need it in the future then we do it then.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #16089: [#16087]: Backfill shouldn't interfere with scheduled run fix
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #16089:
URL: https://github.com/apache/airflow/pull/16089#discussion_r662370293
##########
File path: tests/jobs/test_scheduler_job.py
##########
@@ -1846,6 +1854,10 @@ def test_dagrun_success(self):
dagrun_state=State.SUCCESS,
)
+ @unittest.skip(
+ "Hackish way to test scheduler which doesn't work anymore because the BackfillJob "
+ "doesn't interfere with 'running' DagRuns started by any other job type"
+ )
Review comment:
You can't skip this test. Instead you will have to rewrite the `evaluate_dagrun` function to not use backfil.
--
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] dimonchik-suvorov edited a comment on pull request #16089: [#16087]: Backfill shouldn't interfere with scheduled run fix
Posted by GitBox <gi...@apache.org>.
dimonchik-suvorov edited a comment on pull request #16089:
URL: https://github.com/apache/airflow/pull/16089#issuecomment-872097297
Hi @ashb, @kaxil, I've rebased from the upstream `main` branch again. PR is ready for review :)
--
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] dimonchik-suvorov commented on pull request #16089: [#16087]: Backfill shouldn't interfere with scheduled run fix
Posted by GitBox <gi...@apache.org>.
dimonchik-suvorov commented on pull request #16089:
URL: https://github.com/apache/airflow/pull/16089#issuecomment-861458970
@kaxil @XD-DENG @ashb @houqp guys, it's ready for review, I just don't know what to do with failing build... I didn't touch the flake part and I'm not sure what is `black` and why it failed.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #16089: [#16087]: Backfill shouldn't interfere with scheduled run fix
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #16089:
URL: https://github.com/apache/airflow/pull/16089#discussion_r649424052
##########
File path: airflow/jobs/backfill_job.py
##########
@@ -689,6 +688,9 @@ def tabulate_tis_set(set_tis: Set[TaskInstance]) -> str:
return err
+ def _get_dag_with_subdags(self):
+ return [self.dag] + self.dag.subdags
+
Review comment:
```suggestion
def _get_dag_with_subdags(self):
return [self.dag] + self.dag.subdags
```
Keep it the way it was -- adding a one line function that is only called in one place doesn't make readability any better.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dimonchik-suvorov commented on a change in pull request #16089: [#16087]: Backfill shouldn't interfere with scheduled run fix
Posted by GitBox <gi...@apache.org>.
dimonchik-suvorov commented on a change in pull request #16089:
URL: https://github.com/apache/airflow/pull/16089#discussion_r649869923
##########
File path: airflow/jobs/backfill_job.py
##########
@@ -689,6 +688,9 @@ def tabulate_tis_set(set_tis: Set[TaskInstance]) -> str:
return err
+ def _get_dag_with_subdags(self):
+ return [self.dag] + self.dag.subdags
+
Review comment:
It may help in future refactorings but sure, reverted :)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dimonchik-suvorov commented on pull request #16089: [#16087]: Backfill shouldn't interfere with scheduled run fix
Posted by GitBox <gi...@apache.org>.
dimonchik-suvorov commented on pull request #16089:
URL: https://github.com/apache/airflow/pull/16089#issuecomment-861482353
> @dimonchik-suvorov rebase to latest main and that should be fixed
I just did that... will try one more time
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dimonchik-suvorov commented on pull request #16089: [#16087]: Backfill shouldn't interfere with scheduled run fix
Posted by GitBox <gi...@apache.org>.
dimonchik-suvorov commented on pull request #16089:
URL: https://github.com/apache/airflow/pull/16089#issuecomment-859481113
> @dimonchik-suvorov you should be able to reproduce the CI failures locally using breeze.
I'm trying :D
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #16089: [#16087]: Backfill shouldn't interfere with scheduled run fix
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #16089:
URL: https://github.com/apache/airflow/pull/16089#issuecomment-899132205
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.
--
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] houqp commented on pull request #16089: [#16087]: Backfill shouldn't interfere with scheduled run fix
Posted by GitBox <gi...@apache.org>.
houqp commented on pull request #16089:
URL: https://github.com/apache/airflow/pull/16089#issuecomment-857044188
@dimonchik-suvorov you should be able to reproduce the CI failures locally using breeze.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #16089: [#16087]: Backfill shouldn't interfere with scheduled run fix
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #16089:
URL: https://github.com/apache/airflow/pull/16089#issuecomment-848962848
I think you need to rebase on latest master.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] closed pull request #16089: [#16087]: Backfill shouldn't interfere with scheduled run fix
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #16089:
URL: https://github.com/apache/airflow/pull/16089
--
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] ashb commented on pull request #16089: [#16087]: Backfill shouldn't interfere with scheduled run fix
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #16089:
URL: https://github.com/apache/airflow/pull/16089#issuecomment-861480825
@dimonchik-suvorov rebase to latest main and that should be fixed
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] dimonchik-suvorov commented on pull request #16089: [#16087]: Backfill shouldn't interfere with scheduled run fix
Posted by GitBox <gi...@apache.org>.
dimonchik-suvorov commented on pull request #16089:
URL: https://github.com/apache/airflow/pull/16089#issuecomment-872097297
Hi @kaxil, I've rebased from the upstream `main` branch again. PR is ready for review :)
--
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] dimonchik-suvorov commented on a change in pull request #16089: [#16087]: Backfill shouldn't interfere with scheduled run fix
Posted by GitBox <gi...@apache.org>.
dimonchik-suvorov commented on a change in pull request #16089:
URL: https://github.com/apache/airflow/pull/16089#discussion_r651016401
##########
File path: tests/jobs/test_scheduler_job.py
##########
@@ -2364,9 +2370,6 @@ def test_dagrun_root_fail_unfinished(self):
)
self.null_exec.mock_task_fail(dag_id, 'test_dagrun_fail', DEFAULT_DATE)
- with pytest.raises(AirflowException):
Review comment:
Current impl doesn't rise any exception anyway
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org