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/07/20 23:16:56 UTC
[GitHub] [airflow] ephraimbuddy opened a new pull request #17118: Use `dag_maker` fixture in test_backfill_job.py
ephraimbuddy opened a new pull request #17118:
URL: https://github.com/apache/airflow/pull/17118
This change uses the dag_maker fixture in tests
---
**^ 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] uranusjr commented on a change in pull request #17118: Use `dag_maker` fixture in test_backfill_job.py
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #17118:
URL: https://github.com/apache/airflow/pull/17118#discussion_r673911401
##########
File path: tests/jobs/test_backfill_job.py
##########
@@ -290,13 +306,13 @@ def test_backfill_conf(self):
dr = DagRun.find(dag_id='test_backfill_conf')
- assert conf_ == dr[0].conf
+ assert conf_ == dr[1].conf
Review comment:
Would it be easier to pass additional arguments to `find` to get only that second run? (e.g. `run_id`)
--
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] uranusjr commented on a change in pull request #17118: Use `dag_maker` fixture in test_backfill_job.py
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #17118:
URL: https://github.com/apache/airflow/pull/17118#discussion_r673914099
##########
File path: tests/jobs/test_backfill_job.py
##########
@@ -919,9 +895,9 @@ def test_backfill_max_limit_check_within_limit(self):
assert 2 == len(dagruns)
assert all(run.state == State.SUCCESS for run in dagruns)
- def test_backfill_max_limit_check(self):
+ def test_backfill_max_limit_check(self, get_dag_test_max_active_limits):
dag_id = 'test_backfill_max_limit_check'
- run_id = 'test_dagrun'
+ run_id = 'test'
Review comment:
Is this change needed? (just curious)
--
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] uranusjr commented on a change in pull request #17118: Use `dag_maker` fixture in test_backfill_job.py
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #17118:
URL: https://github.com/apache/airflow/pull/17118#discussion_r673567264
##########
File path: tests/jobs/test_backfill_job.py
##########
@@ -71,14 +71,14 @@ def set_instance_attrs(self, dag_bag):
self.parser = cli_parser.get_parser()
self.dagbag = dag_bag
- def _get_dummy_dag(self, dag_id, pool=Pool.DEFAULT_POOL_NAME, task_concurrency=None):
- dag = DAG(dag_id=dag_id, start_date=DEFAULT_DATE, schedule_interval='@daily')
+ def _get_dummy_dag(
+ self, dag_maker, dag_id, pool=Pool.DEFAULT_POOL_NAME, task_concurrency=None, task_id='op', **kwargs
+ ):
- with dag:
- DummyOperator(task_id='op', pool=pool, task_concurrency=task_concurrency, dag=dag)
+ with dag_maker(dag_id=dag_id, schedule_interval='@daily', **kwargs) as dag:
+ DummyOperator(task_id=task_id, pool=pool, task_concurrency=task_concurrency)
- dag.clear()
- return dag
+ return dag, dag_maker.dag_run
Review comment:
It might be possible to further simply the code by also turning this into a fixture.
```python
@pytest.fixture(scope="the_same_as_dag_maker")
def get_dummy_dag_and_run(dag_maker):
def _get_dummy_dag_and_run(
dag_id,
pool=Pool.DEFAULT_POOL_NAME,
task_concurrency=None,
task_id='op',
**kwargs,
):
with dag_maker(dag_id=dag_id, schedule_interval='@daily', **kwargs) as dag:
DummyOperator(task_id=task_id, pool=pool, task_concurrency=task_concurrency)
return dag, dag_maker.dag_run
return _get_dummy_dag_and_run
```
And use it like
```python
def test_unfinished_dag_runs_set_to_failed(self, get_dummy_dag_and_run):
dag, dag_run = get_dummy_dag_and_run(dag_id='dummy_dag')
--
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 closed pull request #17118: Use `dag_maker` fixture in test_backfill_job.py
Posted by GitBox <gi...@apache.org>.
ephraimbuddy closed pull request #17118:
URL: https://github.com/apache/airflow/pull/17118
--
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 #17118: Use `dag_maker` fixture in test_backfill_job.py
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #17118:
URL: https://github.com/apache/airflow/pull/17118#discussion_r673890194
##########
File path: tests/jobs/test_backfill_job.py
##########
@@ -290,13 +306,13 @@ def test_backfill_conf(self):
dr = DagRun.find(dag_id='test_backfill_conf')
- assert conf_ == dr[0].conf
+ assert conf_ == dr[1].conf
Review comment:
Previously, the get_dummy_dag doesn't create dagrun. Now it creates dag_run, so when the BackFillJob runs, it creates extra two dagruns which now have the conf. Escaping the first dagrun created by get_dummy_dag_and_run fixture, we now find the conf of the 2nd dagrun=>dr[1]
--
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 #17118: Use `dag_maker` fixture in test_backfill_job.py
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #17118:
URL: https://github.com/apache/airflow/pull/17118#discussion_r674327559
##########
File path: tests/jobs/test_backfill_job.py
##########
@@ -919,9 +895,9 @@ def test_backfill_max_limit_check_within_limit(self):
assert 2 == len(dagruns)
assert all(run.state == State.SUCCESS for run in dagruns)
- def test_backfill_max_limit_check(self):
+ def test_backfill_max_limit_check(self, get_dag_test_max_active_limits):
dag_id = 'test_backfill_max_limit_check'
- run_id = 'test_dagrun'
+ run_id = 'test'
Review comment:
Checking why I did this...
--
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 merged pull request #17118: Use `dag_maker` fixture in test_backfill_job.py
Posted by GitBox <gi...@apache.org>.
ephraimbuddy merged pull request #17118:
URL: https://github.com/apache/airflow/pull/17118
--
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] github-actions[bot] commented on pull request #17118: Use `dag_maker` fixture in test_backfill_job.py
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17118:
URL: https://github.com/apache/airflow/pull/17118#issuecomment-884140176
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.
--
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