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/12/16 04:53:08 UTC
[GitHub] [airflow] ReadytoRocc opened a new pull request #19194: Refactor ti clear next method kwargs tests
ReadytoRocc opened a new pull request #19194:
URL: https://github.com/apache/airflow/pull/19194
related: #19120
Refactoring tests introduced in https://github.com/apache/airflow/pull/19183.
--
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] jedcunningham commented on pull request #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on pull request #19194:
URL: https://github.com/apache/airflow/pull/19194#issuecomment-950945805
@ReadytoRocc, I think @uranusjr's suggestion is even better fwiw.
--
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] ReadytoRocc commented on pull request #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
ReadytoRocc commented on pull request #19194:
URL: https://github.com/apache/airflow/pull/19194#issuecomment-950940880
Thanks @uranusjr. We are utilizing `@pytest.mark.parametrize` on lines `L485-L488` for the `state` var. @jedcunningham suggested refactoring these tests to use a single `run()` function that takes `state` as an input.
--
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] ReadytoRocc commented on a change in pull request #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
ReadytoRocc commented on a change in pull request #19194:
URL: https://github.com/apache/airflow/pull/19194#discussion_r735751606
##########
File path: tests/models/test_taskinstance.py
##########
@@ -537,10 +525,10 @@ def reschedule():
session.commit()
ti.task = task
- if state in [State.FAILED, State.UP_FOR_RETRY]:
+ if func.__name__ == "_failure":
Review comment:
Thanks @uranusjr, `if func is type(self)._failure:` worked. Adding it now.
--
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 #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #19194:
URL: https://github.com/apache/airflow/pull/19194#issuecomment-991384356
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] uranusjr commented on a change in pull request #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #19194:
URL: https://github.com/apache/airflow/pull/19194#discussion_r735732144
##########
File path: tests/models/test_taskinstance.py
##########
@@ -537,10 +525,10 @@ def reschedule():
session.commit()
ti.task = task
- if state in [State.FAILED, State.UP_FOR_RETRY]:
+ if func.__name__ == "_failure":
Review comment:
```suggestion
if func is self._failure:
```
(I think this should work)
--
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] potiuk merged pull request #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #19194:
URL: https://github.com/apache/airflow/pull/19194
--
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] ReadytoRocc commented on pull request #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
ReadytoRocc commented on pull request #19194:
URL: https://github.com/apache/airflow/pull/19194#issuecomment-1000542120
Thanks. @jedcunningham and @uranusjr I needed to slightly modify some of the code due to a mypy check that was not present when this PR was originally opened. Please review again before merging. Thanks!
--
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 #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #19194:
URL: https://github.com/apache/airflow/pull/19194#discussion_r779288752
##########
File path: tests/models/test_taskinstance.py
##########
@@ -483,50 +483,32 @@ def task_function(ti):
ti.state == state
@pytest.mark.parametrize(
- "state",
- [State.FAILED, State.SKIPPED, State.SUCCESS, State.UP_FOR_RESCHEDULE, State.UP_FOR_RETRY],
+ "state, exception_type, retries",
+ [
+ (State.FAILED, AirflowException, 0),
+ (State.SKIPPED, AirflowSkipException, 0),
+ (State.SUCCESS, None, 0),
+ (State.UP_FOR_RESCHEDULE, AirflowRescheduleException(timezone.utcnow()), 0),
+ (State.UP_FOR_RETRY, AirflowException, 1),
+ ],
)
- def test_task_wipes_next_fields(self, session, state, dag_maker):
+ def test_task_wipes_next_fields(self, session, dag_maker, state, exception_type, retries):
"""
- Test that ensures that tasks wipe their next_method and next_kwargs
- when they go into a state of FAILED, SKIPPED, SUCCESS, UP_FOR_RESCHEDULE, or UP_FOR_RETRY.
+ Test that ensures that tasks wipe their next_method and next_kwargs for the configured states:
+ FAILED, SKIPPED, SUCCESS, UP_FOR_RESCHEDULE, UP_FOR_RETRY.
"""
- def failure():
- raise AirflowException
-
- def skip():
- raise AirflowSkipException
-
- def success():
- return None
-
- def reschedule():
- reschedule_date = timezone.utcnow()
- raise AirflowRescheduleException(reschedule_date)
-
- _retries = 0
- _retry_delay = datetime.timedelta(seconds=0)
-
- if state == State.FAILED:
- _python_callable = failure
- elif state == State.SKIPPED:
- _python_callable = skip
- elif state == State.SUCCESS:
- _python_callable = success
- elif state == State.UP_FOR_RESCHEDULE:
- _python_callable = reschedule
- elif state in [State.FAILED, State.UP_FOR_RETRY]:
- _python_callable = failure
- _retries = 1
- _retry_delay = datetime.timedelta(seconds=2)
+ def _raise_af_exception(exception_type):
Review comment:
```suggestion
def _raise_if_exception(exception_type):
```
Probably? Or does `af` stand for Airflow?
##########
File path: tests/models/test_taskinstance.py
##########
@@ -537,10 +519,10 @@ def reschedule():
session.commit()
ti.task = task
- if state in [State.FAILED, State.UP_FOR_RETRY]:
+ if exception_type == AirflowException:
Review comment:
This does not correctly cover the `AirflowRescheduleException(timezone.utcnow())` case. It’s probably better to change `exception_type` to `exception` instead and always instantiate to avoid this subtle issue caused by `exception_type` may be either a type or an instance.
--
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] ReadytoRocc commented on pull request #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
ReadytoRocc commented on pull request #19194:
URL: https://github.com/apache/airflow/pull/19194#issuecomment-1005697139
@jedcunningham @uranusjr I needed to slightly modify some of the code due to a mypy check that was not present when this PR was originally opened. Please review again and let me know if any changes are needed before merging. Thanks!
--
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] ReadytoRocc commented on pull request #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
ReadytoRocc commented on pull request #19194:
URL: https://github.com/apache/airflow/pull/19194#issuecomment-951001209
Thanks @jedcunningham and @uranusjr. Refactored as suggested.
--
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 #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #19194:
URL: https://github.com/apache/airflow/pull/19194#discussion_r735735239
##########
File path: tests/models/test_taskinstance.py
##########
@@ -537,10 +525,10 @@ def reschedule():
session.commit()
ti.task = task
- if state in [State.FAILED, State.UP_FOR_RETRY]:
+ if func.__name__ == "_failure":
Review comment:
Another possibility would be to add yet another parameter:
```python
def run_expect_catch(ti):
with pytest.raises(AirflowException):
ti.run()
def run(ti):
ti.run()
@pytest.mark.parametrize(
"..., run_func",
[
(..., failure, run_expect_catch),
(..., success, run),
...,
],
)
```
--
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 #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #19194:
URL: https://github.com/apache/airflow/pull/19194#discussion_r735732144
##########
File path: tests/models/test_taskinstance.py
##########
@@ -537,10 +525,10 @@ def reschedule():
session.commit()
ti.task = task
- if state in [State.FAILED, State.UP_FOR_RETRY]:
+ if func.__name__ == "_failure":
Review comment:
```suggestion
if func is type(self)._failure:
```
(I think this should work. If not, we can put the function outside of the class instead.)
--
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] ReadytoRocc commented on a change in pull request #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
ReadytoRocc commented on a change in pull request #19194:
URL: https://github.com/apache/airflow/pull/19194#discussion_r779689744
##########
File path: tests/models/test_taskinstance.py
##########
@@ -537,10 +519,10 @@ def reschedule():
session.commit()
ti.task = task
- if state in [State.FAILED, State.UP_FOR_RETRY]:
+ if exception_type == AirflowException:
Review comment:
Thanks for the feedback, I will rename `exception_type` to `exception`. I will edit as follows:
```
if state in [State.FAILED, State.UP_FOR_RETRY]:
with pytest.raises(exception):
ti.run()
else:
ti.run()
```
This ensures that the `exception` is tied to whatever `state` is being checked.
The reason I did not include `AirflowRescheduleException(timezone.utcnow())` is because that exception is caught during `ti.run()`.
--
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] dstandish commented on a change in pull request #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #19194:
URL: https://github.com/apache/airflow/pull/19194#discussion_r774794859
##########
File path: tests/models/test_taskinstance.py
##########
@@ -483,50 +483,32 @@ def task_function(ti):
ti.state == state
@pytest.mark.parametrize(
- "state",
- [State.FAILED, State.SKIPPED, State.SUCCESS, State.UP_FOR_RESCHEDULE, State.UP_FOR_RETRY],
+ "state, exception_type, retries",
+ [
+ (State.FAILED, AirflowException, 0),
+ (State.SKIPPED, AirflowSkipException, 0),
+ (State.SUCCESS, None, 0),
+ (State.UP_FOR_RESCHEDULE, AirflowRescheduleException(timezone.utcnow()), 0),
+ (State.UP_FOR_RETRY, AirflowException, 1),
+ ],
)
- def test_task_wipes_next_fields(self, session, state, dag_maker):
+ def test_task_wipes_next_fields(self, session, dag_maker, state, exception_type, retries):
"""
- Test that ensures that tasks wipe their next_method and next_kwargs
- when they go into a state of FAILED, SKIPPED, SUCCESS, UP_FOR_RESCHEDULE, or UP_FOR_RETRY.
+ Test that ensures that tasks wipe their next_method and next_kwargs for the configured states:
+ FAILED, SKIPPED, SUCCESS, UP_FOR_RESCHEDULE, UP_FOR_RETRY.
"""
- def failure():
- raise AirflowException
-
- def skip():
- raise AirflowSkipException
-
- def success():
- return None
-
- def reschedule():
- reschedule_date = timezone.utcnow()
- raise AirflowRescheduleException(reschedule_date)
-
- _retries = 0
- _retry_delay = datetime.timedelta(seconds=0)
-
- if state == State.FAILED:
- _python_callable = failure
- elif state == State.SKIPPED:
- _python_callable = skip
- elif state == State.SUCCESS:
- _python_callable = success
- elif state == State.UP_FOR_RESCHEDULE:
- _python_callable = reschedule
- elif state in [State.FAILED, State.UP_FOR_RETRY]:
- _python_callable = failure
- _retries = 1
- _retry_delay = datetime.timedelta(seconds=2)
+ def _raise_af_exception(exception_type):
+ if exception_type:
+ raise exception_type
with dag_maker("test_deferred_method_clear"):
task = PythonOperator(
task_id="test_deferred_method_clear_task",
- python_callable=_python_callable,
- retries=_retries,
- retry_delay=_retry_delay,
+ python_callable=_raise_af_exception,
+ op_args=[exception_type],
Review comment:
if you remove `exception_type` as a param in `_raise_af_exception` you can also remove `op_kwargs`
--
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 #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #19194:
URL: https://github.com/apache/airflow/pull/19194#issuecomment-991384356
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] uranusjr commented on pull request #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #19194:
URL: https://github.com/apache/airflow/pull/19194#issuecomment-950553256
It's probably better to refactor using `paramtrize` instead, something like
```python
def _failure():
raise AirflowException
def _skip():
raise AirflowSkipException
# Others...
@pytest.mark.parametrize(
"state, func",
[
(State.FAILED, _failure),
(State.SKIPPED, _skip),
# Others...
],
)
def test_task_wipes_next_fields(self, session, dag_maker, state, func):
with dag_maker("test_deferred_method_clear"):
task = PythonOperator(
...,
python_callable=func,
)
...
```
--
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] closed pull request #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #19194:
URL: https://github.com/apache/airflow/pull/19194
--
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] ReadytoRocc commented on a change in pull request #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
ReadytoRocc commented on a change in pull request #19194:
URL: https://github.com/apache/airflow/pull/19194#discussion_r779654803
##########
File path: tests/models/test_taskinstance.py
##########
@@ -483,50 +483,32 @@ def task_function(ti):
ti.state == state
@pytest.mark.parametrize(
- "state",
- [State.FAILED, State.SKIPPED, State.SUCCESS, State.UP_FOR_RESCHEDULE, State.UP_FOR_RETRY],
+ "state, exception_type, retries",
+ [
+ (State.FAILED, AirflowException, 0),
+ (State.SKIPPED, AirflowSkipException, 0),
+ (State.SUCCESS, None, 0),
+ (State.UP_FOR_RESCHEDULE, AirflowRescheduleException(timezone.utcnow()), 0),
+ (State.UP_FOR_RETRY, AirflowException, 1),
+ ],
)
- def test_task_wipes_next_fields(self, session, state, dag_maker):
+ def test_task_wipes_next_fields(self, session, dag_maker, state, exception_type, retries):
"""
- Test that ensures that tasks wipe their next_method and next_kwargs
- when they go into a state of FAILED, SKIPPED, SUCCESS, UP_FOR_RESCHEDULE, or UP_FOR_RETRY.
+ Test that ensures that tasks wipe their next_method and next_kwargs for the configured states:
+ FAILED, SKIPPED, SUCCESS, UP_FOR_RESCHEDULE, UP_FOR_RETRY.
"""
- def failure():
- raise AirflowException
-
- def skip():
- raise AirflowSkipException
-
- def success():
- return None
-
- def reschedule():
- reschedule_date = timezone.utcnow()
- raise AirflowRescheduleException(reschedule_date)
-
- _retries = 0
- _retry_delay = datetime.timedelta(seconds=0)
-
- if state == State.FAILED:
- _python_callable = failure
- elif state == State.SKIPPED:
- _python_callable = skip
- elif state == State.SUCCESS:
- _python_callable = success
- elif state == State.UP_FOR_RESCHEDULE:
- _python_callable = reschedule
- elif state in [State.FAILED, State.UP_FOR_RETRY]:
- _python_callable = failure
- _retries = 1
- _retry_delay = datetime.timedelta(seconds=2)
+ def _raise_af_exception(exception_type):
+ if exception_type:
+ raise exception_type
with dag_maker("test_deferred_method_clear"):
task = PythonOperator(
task_id="test_deferred_method_clear_task",
- python_callable=_python_callable,
- retries=_retries,
- retry_delay=_retry_delay,
+ python_callable=_raise_af_exception,
+ op_args=[exception_type],
Review comment:
Thanks @dstandish, will do.
--
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] jedcunningham closed pull request #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
jedcunningham closed pull request #19194:
URL: https://github.com/apache/airflow/pull/19194
--
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] jedcunningham commented on pull request #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on pull request #19194:
URL: https://github.com/apache/airflow/pull/19194#issuecomment-995438098
@ReadytoRocc, geez sorry, didn't realize this never got merged! Can you rebase on main when you get the chance?
--
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] jedcunningham closed pull request #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
jedcunningham closed pull request #19194:
URL: https://github.com/apache/airflow/pull/19194
--
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 #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #19194:
URL: https://github.com/apache/airflow/pull/19194#issuecomment-951086792
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] dstandish commented on a change in pull request #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
dstandish commented on a change in pull request #19194:
URL: https://github.com/apache/airflow/pull/19194#discussion_r774797017
##########
File path: tests/models/test_taskinstance.py
##########
@@ -483,50 +483,32 @@ def task_function(ti):
ti.state == state
@pytest.mark.parametrize(
- "state",
- [State.FAILED, State.SKIPPED, State.SUCCESS, State.UP_FOR_RESCHEDULE, State.UP_FOR_RETRY],
+ "state, exception_type, retries",
+ [
+ (State.FAILED, AirflowException, 0),
+ (State.SKIPPED, AirflowSkipException, 0),
+ (State.SUCCESS, None, 0),
+ (State.UP_FOR_RESCHEDULE, AirflowRescheduleException(timezone.utcnow()), 0),
+ (State.UP_FOR_RETRY, AirflowException, 1),
+ ],
)
- def test_task_wipes_next_fields(self, session, state, dag_maker):
+ def test_task_wipes_next_fields(self, session, dag_maker, state, exception_type, retries):
"""
- Test that ensures that tasks wipe their next_method and next_kwargs
- when they go into a state of FAILED, SKIPPED, SUCCESS, UP_FOR_RESCHEDULE, or UP_FOR_RETRY.
+ Test that ensures that tasks wipe their next_method and next_kwargs for the configured states:
Review comment:
```suggestion
Test that ensures that tasks wipe their next_method and next_kwargs when the TI enters one of the following states:
```
--
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] ReadytoRocc commented on a change in pull request #19194: Refactor ti clear next method kwargs tests
Posted by GitBox <gi...@apache.org>.
ReadytoRocc commented on a change in pull request #19194:
URL: https://github.com/apache/airflow/pull/19194#discussion_r779560013
##########
File path: tests/models/test_taskinstance.py
##########
@@ -483,50 +483,32 @@ def task_function(ti):
ti.state == state
@pytest.mark.parametrize(
- "state",
- [State.FAILED, State.SKIPPED, State.SUCCESS, State.UP_FOR_RESCHEDULE, State.UP_FOR_RETRY],
+ "state, exception_type, retries",
+ [
+ (State.FAILED, AirflowException, 0),
+ (State.SKIPPED, AirflowSkipException, 0),
+ (State.SUCCESS, None, 0),
+ (State.UP_FOR_RESCHEDULE, AirflowRescheduleException(timezone.utcnow()), 0),
+ (State.UP_FOR_RETRY, AirflowException, 1),
+ ],
)
- def test_task_wipes_next_fields(self, session, state, dag_maker):
+ def test_task_wipes_next_fields(self, session, dag_maker, state, exception_type, retries):
"""
- Test that ensures that tasks wipe their next_method and next_kwargs
- when they go into a state of FAILED, SKIPPED, SUCCESS, UP_FOR_RESCHEDULE, or UP_FOR_RETRY.
+ Test that ensures that tasks wipe their next_method and next_kwargs for the configured states:
+ FAILED, SKIPPED, SUCCESS, UP_FOR_RESCHEDULE, UP_FOR_RETRY.
"""
- def failure():
- raise AirflowException
-
- def skip():
- raise AirflowSkipException
-
- def success():
- return None
-
- def reschedule():
- reschedule_date = timezone.utcnow()
- raise AirflowRescheduleException(reschedule_date)
-
- _retries = 0
- _retry_delay = datetime.timedelta(seconds=0)
-
- if state == State.FAILED:
- _python_callable = failure
- elif state == State.SKIPPED:
- _python_callable = skip
- elif state == State.SUCCESS:
- _python_callable = success
- elif state == State.UP_FOR_RESCHEDULE:
- _python_callable = reschedule
- elif state in [State.FAILED, State.UP_FOR_RETRY]:
- _python_callable = failure
- _retries = 1
- _retry_delay = datetime.timedelta(seconds=2)
+ def _raise_af_exception(exception_type):
Review comment:
`af` stands for Airflow. However, `_raise_if_exception` better describes the function. I will update this, thanks.
--
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