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/09 12:20:41 UTC
[GitHub] [airflow] ephraimbuddy opened a new pull request #17506: Use `dag_maker` fixture in test_processor.py
ephraimbuddy opened a new pull request #17506:
URL: https://github.com/apache/airflow/pull/17506
This change applies `dag_maker` fixture in test_process.py
---
**^ 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] kaxil commented on a change in pull request #17506: Use `dag_maker` fixture in test_processor.py
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #17506:
URL: https://github.com/apache/airflow/pull/17506#discussion_r685951621
##########
File path: tests/conftest.py
##########
@@ -484,3 +489,41 @@ def __call__(self, dag_id='test_dag', session=None, **kwargs):
return self
return DagFactory()
+
+
+@pytest.fixture
+def get_dummy_dag(dag_maker):
Review comment:
```suggestion
def create_dummy_dag(dag_maker):
```
Probably? It can stay as it is too -- no strong opinion
--
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 #17506: Use `dag_maker` fixture in test_processor.py
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #17506:
URL: https://github.com/apache/airflow/pull/17506#discussion_r686536617
##########
File path: tests/dag_processing/test_processor.py
##########
@@ -97,34 +96,12 @@ def teardown_method(self) -> None:
self.scheduler_job = None
self.clean_db()
- def create_test_dag(self, start_date=DEFAULT_DATE, end_date=DEFAULT_DATE + timedelta(hours=1), **kwargs):
- dag = DAG(
- dag_id='test_scheduler_reschedule',
- start_date=start_date,
- # Make sure it only creates a single DAG Run
- end_date=end_date,
- )
- dag.clear()
- dag.is_subdag = False
- with create_session() as session:
- orm_dag = DagModel(dag_id=dag.dag_id, is_paused=False)
- session.merge(orm_dag)
- session.commit()
- return dag
-
- @classmethod
- def setup_class(cls):
- # Ensure the DAGs we are looking at from the DB are up-to-date
- non_serialized_dagbag = DagBag(read_dags_from_db=False, include_examples=False)
- non_serialized_dagbag.sync_to_db()
- cls.dagbag = DagBag(read_dags_from_db=True)
Review comment:
Yes. Not used.
--
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 #17506: Use `dag_maker` fixture in test_processor.py
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #17506:
URL: https://github.com/apache/airflow/pull/17506#discussion_r685194037
##########
File path: tests/conftest.py
##########
@@ -473,7 +473,11 @@ def __call__(self, dag_id='test_dag', session=None, **kwargs):
self.kwargs = kwargs
self.session = session
self.start_date = self.kwargs.get('start_date', None)
+ default_args = kwargs.get('default_args', None)
+ if not self.start_date and 'start_date' in default_args:
+ self.start_date = default_args.get('start_date')
Review comment:
The default_args here is the one used by DAG to pass to tasks. It can be in the kwargs as `default_args`.
See: https://github.com/apache/airflow/blob/37cc115d30dc17cf474a6eff04aad056cb6adef4/airflow/models/dag.py#L165
--
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 #17506: Use `dag_maker` fixture in test_processor.py
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #17506:
URL: https://github.com/apache/airflow/pull/17506#discussion_r686001654
##########
File path: tests/conftest.py
##########
@@ -484,3 +489,41 @@ def __call__(self, dag_id='test_dag', session=None, **kwargs):
return self
return DagFactory()
+
+
+@pytest.fixture
+def get_dummy_dag(dag_maker):
Review comment:
Cool.
--
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 #17506: Use `dag_maker` fixture in test_processor.py
Posted by GitBox <gi...@apache.org>.
ephraimbuddy merged pull request #17506:
URL: https://github.com/apache/airflow/pull/17506
--
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 #17506: Use `dag_maker` fixture in test_processor.py
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #17506:
URL: https://github.com/apache/airflow/pull/17506#discussion_r686489172
##########
File path: tests/dag_processing/test_processor.py
##########
@@ -97,34 +96,12 @@ def teardown_method(self) -> None:
self.scheduler_job = None
self.clean_db()
- def create_test_dag(self, start_date=DEFAULT_DATE, end_date=DEFAULT_DATE + timedelta(hours=1), **kwargs):
- dag = DAG(
- dag_id='test_scheduler_reschedule',
- start_date=start_date,
- # Make sure it only creates a single DAG Run
- end_date=end_date,
- )
- dag.clear()
- dag.is_subdag = False
- with create_session() as session:
- orm_dag = DagModel(dag_id=dag.dag_id, is_paused=False)
- session.merge(orm_dag)
- session.commit()
- return dag
-
- @classmethod
- def setup_class(cls):
- # Ensure the DAGs we are looking at from the DB are up-to-date
- non_serialized_dagbag = DagBag(read_dags_from_db=False, include_examples=False)
- non_serialized_dagbag.sync_to_db()
- cls.dagbag = DagBag(read_dags_from_db=True)
Review comment:
Hmm. So this dagbag is not actually used anywhere?
--
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] kaxil commented on a change in pull request #17506: Use `dag_maker` fixture in test_processor.py
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #17506:
URL: https://github.com/apache/airflow/pull/17506#discussion_r685952006
##########
File path: tests/conftest.py
##########
@@ -484,3 +489,41 @@ def __call__(self, dag_id='test_dag', session=None, **kwargs):
return self
return DagFactory()
+
+
+@pytest.fixture
+def get_dummy_dag(dag_maker):
Review comment:
Can you add some description/docstring on all fixtures too so a new author/contributor knows when to use what etc
--
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 #17506: Use `dag_maker` fixture in test_processor.py
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17506:
URL: https://github.com/apache/airflow/pull/17506#issuecomment-896359095
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
[GitHub] [airflow] uranusjr commented on a change in pull request #17506: Use `dag_maker` fixture in test_processor.py
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #17506:
URL: https://github.com/apache/airflow/pull/17506#discussion_r685153836
##########
File path: tests/conftest.py
##########
@@ -473,7 +473,11 @@ def __call__(self, dag_id='test_dag', session=None, **kwargs):
self.kwargs = kwargs
self.session = session
self.start_date = self.kwargs.get('start_date', None)
+ default_args = kwargs.get('default_args', None)
+ if not self.start_date and 'start_date' in default_args:
+ self.start_date = default_args.get('start_date')
Review comment:
Would it work if we do this?
```python
def __call__(self, dag_id='test_dag', default_args=None, session=None, **kwargs):
if default_args is not None:
kwargs = {**default_args, **kwargs}
self.kwargs = kwargs
self.session = session
self.start_date = self.kwargs.get('start_date', 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] ephraimbuddy commented on a change in pull request #17506: Use `dag_maker` fixture in test_processor.py
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #17506:
URL: https://github.com/apache/airflow/pull/17506#discussion_r686172336
##########
File path: tests/conftest.py
##########
@@ -484,3 +489,41 @@ def __call__(self, dag_id='test_dag', session=None, **kwargs):
return self
return DagFactory()
+
+
+@pytest.fixture
+def get_dummy_dag(dag_maker):
Review comment:
Description added
--
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 #17506: Use `dag_maker` fixture in test_processor.py
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #17506:
URL: https://github.com/apache/airflow/pull/17506#discussion_r685152406
##########
File path: tests/conftest.py
##########
@@ -473,7 +473,11 @@ def __call__(self, dag_id='test_dag', session=None, **kwargs):
self.kwargs = kwargs
self.session = session
self.start_date = self.kwargs.get('start_date', None)
+ default_args = kwargs.get('default_args', None)
+ if not self.start_date and 'start_date' in default_args:
+ self.start_date = default_args.get('start_date')
Review comment:
Gosh this `default_args` thing is really awkward 🤯
--
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 #17506: Use `dag_maker` fixture in test_processor.py
Posted by GitBox <gi...@apache.org>.
ephraimbuddy closed pull request #17506:
URL: https://github.com/apache/airflow/pull/17506
--
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