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/12/04 16:36:26 UTC
[GitHub] [airflow] surabathini opened a new pull request, #28097: logging poke info when external dag is not none and task_id and task_ids are none
surabathini opened a new pull request, #28097:
URL: https://github.com/apache/airflow/pull/28097
When using ExternalTaskSensor to check for an external_dag_id and external_task_ids=None, we are missing the logging info.
This PR is to fix the missing logging info for "Poking for tasks None in 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] surabathini commented on a diff in pull request #28097: logging poke info when external dag is not none and task_id and task_ids are none
Posted by GitBox <gi...@apache.org>.
surabathini commented on code in PR #28097:
URL: https://github.com/apache/airflow/pull/28097#discussion_r1039355812
##########
airflow/sensors/external_task.py:
##########
@@ -199,18 +199,17 @@ def poke(self, context, session=None):
dttm_filter = self._get_dttm_filter(context)
serialized_dttm_filter = ",".join(dt.isoformat() for dt in dttm_filter)
- if self.external_task_ids:
+ if self.external_task_group_id:
self.log.info(
- "Poking for tasks %s in dag %s on %s ... ",
- self.external_task_ids,
+ "Poking for task_group '%s' in dag '%s' on %s ... ",
+ self.external_task_group_id,
self.external_dag_id,
serialized_dttm_filter,
)
-
- if self.external_task_group_id:
+ else:
self.log.info(
- "Poking for task_group '%s' in dag '%s' on %s ... ",
- self.external_task_group_id,
+ "Poking for tasks %s in dag %s on %s ... ",
Review Comment:
Thanks for the review, will work on the changes.
--
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 diff in pull request #28097: logging poke info when external dag is not none and task_id and task_ids are none
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #28097:
URL: https://github.com/apache/airflow/pull/28097#discussion_r1063093904
##########
airflow/sensors/external_task.py:
##########
@@ -143,18 +143,27 @@ def __init__(
if external_task_id is not None and external_task_ids is not None:
raise ValueError(
"Only one of `external_task_id` or `external_task_ids` may "
- "be provided to ExternalTaskSensor; not both."
+ "be provided to ExternalTaskSensor; "
+ "Use external_task_id or external_task_ids or external_task_group_id."
Review Comment:
```suggestion
"be provided to ExternalTaskSensor; "
"use external_task_id or external_task_ids or external_task_group_id."
```
Same for other messages below. Or you could use a period instead.
##########
airflow/sensors/external_task.py:
##########
@@ -217,6 +226,13 @@ def poke(self, context, session=None):
serialized_dttm_filter,
)
+ if self.external_dag_id and not self.external_task_group_id and not self.external_task_ids:
+ self.log.info(
+ "Poking for dag '%s' on %s ... ",
Review Comment:
```suggestion
"Poking for DAG '%s' on %s ... ",
```
I wonder if this message could be spammy? This would appear again and again until the poke is successful.
--
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 diff in pull request #28097: logging poke info when external dag is not none and task_id and task_ids are none
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #28097:
URL: https://github.com/apache/airflow/pull/28097#discussion_r1064364596
##########
airflow/sensors/external_task.py:
##########
@@ -217,6 +226,13 @@ def poke(self, context, session=None):
serialized_dttm_filter,
)
+ if self.external_dag_id and not self.external_task_group_id and not self.external_task_ids:
+ self.log.info(
+ "Poking for dag '%s' on %s ... ",
Review Comment:
OK. I don’t feel too strong about this, and we can always revisit if someone complains.
--
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] boring-cyborg[bot] commented on pull request #28097: logging poke info when external dag is not none and task_id and task_ids are none
Posted by boring-cyborg.
boring-cyborg[bot] commented on PR #28097:
URL: https://github.com/apache/airflow/pull/28097#issuecomment-1398765025
Awesome work, congrats on your first merged pull request!
--
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 diff in pull request #28097: logging poke info when external dag is not none and task_id and task_ids are none
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #28097:
URL: https://github.com/apache/airflow/pull/28097#discussion_r1060221310
##########
airflow/sensors/external_task.py:
##########
@@ -140,21 +140,30 @@ def __init__(
f"`{self.allowed_states}` and failed states `{self.failed_states}`"
)
- if external_task_id is not None and external_task_ids is not None:
+ if external_task_id and external_task_ids:
raise ValueError(
"Only one of `external_task_id` or `external_task_ids` may "
- "be provided to ExternalTaskSensor; not both."
+ "be provided to ExternalTaskSensor; "
+ "Use external_task_id or external_task_ids or external_task_group_id."
)
- if external_task_id is not None:
- external_task_ids = [external_task_id]
+ if external_task_group_id and external_task_id:
+ raise ValueError(
+ "Only one of `external_task_group_id` or `external_task_id` may "
+ "be provided to ExternalTaskSensor; "
+ "Use external_task_id or external_task_ids or external_task_group_id."
+ )
if external_task_group_id and external_task_ids:
raise ValueError(
- "Values for `external_task_group_id` and `external_task_id` or `external_task_ids` "
- "can't be set at the same time"
+ "Only one of `external_task_group_id` or `external_task_ids` may "
+ "be provided to ExternalTaskSensor; "
+ "Use external_task_id or external_task_ids or external_task_group_id."
)
Review Comment:
The question is mainly for the `is not None` changes.
--
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 commented on a diff in pull request #28097: logging poke info when external dag is not none and task_id and task_ids are none
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #28097:
URL: https://github.com/apache/airflow/pull/28097#discussion_r1039105739
##########
airflow/sensors/external_task.py:
##########
@@ -199,18 +199,17 @@ def poke(self, context, session=None):
dttm_filter = self._get_dttm_filter(context)
serialized_dttm_filter = ",".join(dt.isoformat() for dt in dttm_filter)
- if self.external_task_ids:
+ if self.external_task_group_id:
self.log.info(
- "Poking for tasks %s in dag %s on %s ... ",
- self.external_task_ids,
+ "Poking for task_group '%s' in dag '%s' on %s ... ",
+ self.external_task_group_id,
self.external_dag_id,
serialized_dttm_filter,
)
-
- if self.external_task_group_id:
+ else:
self.log.info(
- "Poking for task_group '%s' in dag '%s' on %s ... ",
- self.external_task_group_id,
+ "Poking for tasks %s in dag %s on %s ... ",
Review Comment:
I thin, we should change the log and it should be "Poking for DAG" if task_id is None according to documentation of the Sensor. We should likely also fali the sensor when both task_id and group_id are specified.
--
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] surabathini commented on a diff in pull request #28097: logging poke info when external dag is not none and task_id and task_ids are none
Posted by GitBox <gi...@apache.org>.
surabathini commented on code in PR #28097:
URL: https://github.com/apache/airflow/pull/28097#discussion_r1060969692
##########
airflow/sensors/external_task.py:
##########
@@ -140,21 +140,30 @@ def __init__(
f"`{self.allowed_states}` and failed states `{self.failed_states}`"
)
- if external_task_id is not None and external_task_ids is not None:
+ if external_task_id and external_task_ids:
raise ValueError(
"Only one of `external_task_id` or `external_task_ids` may "
- "be provided to ExternalTaskSensor; not both."
+ "be provided to ExternalTaskSensor; "
+ "Use external_task_id or external_task_ids or external_task_group_id."
)
- if external_task_id is not None:
- external_task_ids = [external_task_id]
+ if external_task_group_id and external_task_id:
+ raise ValueError(
+ "Only one of `external_task_group_id` or `external_task_id` may "
+ "be provided to ExternalTaskSensor; "
+ "Use external_task_id or external_task_ids or external_task_group_id."
+ )
if external_task_group_id and external_task_ids:
raise ValueError(
- "Values for `external_task_group_id` and `external_task_id` or `external_task_ids` "
- "can't be set at the same time"
+ "Only one of `external_task_group_id` or `external_task_ids` may "
+ "be provided to ExternalTaskSensor; "
+ "Use external_task_id or external_task_ids or external_task_group_id."
)
Review Comment:
No specific reason other than keeping all the conditions look same in this function. some are with out is not none and some are with is not none.
Are there any potential issues you foresee with this change?
--
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] surabathini commented on a diff in pull request #28097: logging poke info when external dag is not none and task_id and task_ids are none
Posted by GitBox <gi...@apache.org>.
surabathini commented on code in PR #28097:
URL: https://github.com/apache/airflow/pull/28097#discussion_r1064618191
##########
tests/sensors/test_external_task_sensor.py:
##########
@@ -351,6 +382,21 @@ def test_external_dag_sensor(self):
)
op.run(start_date=DEFAULT_DATE, end_date=DEFAULT_DATE, ignore_ti_state=True)
+ def test_external_dag_sensor_log(self, caplog):
+ other_dag = DAG("other_dag", default_args=self.args, end_date=DEFAULT_DATE, schedule="@once")
+ other_dag.create_dagrun(
+ run_id="test", start_date=DEFAULT_DATE, execution_date=DEFAULT_DATE, state=State.SUCCESS
+ )
+ op = ExternalTaskSensor(
+ task_id="test_external_dag_sensor_check",
+ external_dag_id="other_dag",
+ dag=self.dag,
+ )
+ with caplog.at_level(logging.INFO, logger=op.log.name):
+ caplog.clear()
Review Comment:
removed the same and the test working as expected.
--
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] surabathini commented on a diff in pull request #28097: logging poke info when external dag is not none and task_id and task_ids are none
Posted by GitBox <gi...@apache.org>.
surabathini commented on code in PR #28097:
URL: https://github.com/apache/airflow/pull/28097#discussion_r1063225098
##########
airflow/sensors/external_task.py:
##########
@@ -217,6 +226,13 @@ def poke(self, context, session=None):
serialized_dttm_filter,
)
+ if self.external_dag_id and not self.external_task_group_id and not self.external_task_ids:
+ self.log.info(
+ "Poking for dag '%s' on %s ... ",
Review Comment:
I see that same as other poking messages when we give a task_id/task_ids/task_groups. In this case we are poking for the dag status.
--
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 #28097: logging poke info when external dag is not none and task_id and task_ids are none
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #28097:
URL: https://github.com/apache/airflow/pull/28097
--
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] surabathini commented on a diff in pull request #28097: logging poke info when external dag is not none and task_id and task_ids are none
Posted by GitBox <gi...@apache.org>.
surabathini commented on code in PR #28097:
URL: https://github.com/apache/airflow/pull/28097#discussion_r1039389992
##########
airflow/sensors/external_task.py:
##########
@@ -199,18 +199,17 @@ def poke(self, context, session=None):
dttm_filter = self._get_dttm_filter(context)
serialized_dttm_filter = ",".join(dt.isoformat() for dt in dttm_filter)
- if self.external_task_ids:
+ if self.external_task_group_id:
self.log.info(
- "Poking for tasks %s in dag %s on %s ... ",
- self.external_task_ids,
+ "Poking for task_group '%s' in dag '%s' on %s ... ",
+ self.external_task_group_id,
self.external_dag_id,
serialized_dttm_filter,
)
-
- if self.external_task_group_id:
+ else:
self.log.info(
- "Poking for task_group '%s' in dag '%s' on %s ... ",
- self.external_task_group_id,
+ "Poking for tasks %s in dag %s on %s ... ",
Review Comment:
Updated the log messages to reflect the usage
--
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] surabathini commented on a diff in pull request #28097: logging poke info when external dag is not none and task_id and task_ids are none
Posted by GitBox <gi...@apache.org>.
surabathini commented on code in PR #28097:
URL: https://github.com/apache/airflow/pull/28097#discussion_r1062697560
##########
airflow/sensors/external_task.py:
##########
@@ -140,21 +140,30 @@ def __init__(
f"`{self.allowed_states}` and failed states `{self.failed_states}`"
)
- if external_task_id is not None and external_task_ids is not None:
+ if external_task_id and external_task_ids:
raise ValueError(
"Only one of `external_task_id` or `external_task_ids` may "
- "be provided to ExternalTaskSensor; not both."
+ "be provided to ExternalTaskSensor; "
+ "Use external_task_id or external_task_ids or external_task_group_id."
)
- if external_task_id is not None:
- external_task_ids = [external_task_id]
+ if external_task_group_id and external_task_id:
+ raise ValueError(
+ "Only one of `external_task_group_id` or `external_task_id` may "
+ "be provided to ExternalTaskSensor; "
+ "Use external_task_id or external_task_ids or external_task_group_id."
+ )
if external_task_group_id and external_task_ids:
raise ValueError(
- "Values for `external_task_group_id` and `external_task_id` or `external_task_ids` "
- "can't be set at the same time"
+ "Only one of `external_task_group_id` or `external_task_ids` may "
+ "be provided to ExternalTaskSensor; "
+ "Use external_task_id or external_task_ids or external_task_group_id."
)
Review Comment:
Thanks, added the is not None back.
--
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] surabathini commented on a diff in pull request #28097: logging poke info when external dag is not none and task_id and task_ids are none
Posted by GitBox <gi...@apache.org>.
surabathini commented on code in PR #28097:
URL: https://github.com/apache/airflow/pull/28097#discussion_r1063308061
##########
airflow/sensors/external_task.py:
##########
@@ -143,18 +143,27 @@ def __init__(
if external_task_id is not None and external_task_ids is not None:
raise ValueError(
"Only one of `external_task_id` or `external_task_ids` may "
- "be provided to ExternalTaskSensor; not both."
+ "be provided to ExternalTaskSensor; "
+ "Use external_task_id or external_task_ids or external_task_group_id."
Review Comment:
Done the change.
--
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 commented on pull request #28097: logging poke info when external dag is not none and task_id and task_ids are none
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28097:
URL: https://github.com/apache/airflow/pull/28097#issuecomment-1366779235
Tests are failing.
--
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 diff in pull request #28097: logging poke info when external dag is not none and task_id and task_ids are none
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #28097:
URL: https://github.com/apache/airflow/pull/28097#discussion_r1046795912
##########
airflow/sensors/external_task.py:
##########
@@ -140,21 +140,30 @@ def __init__(
f"`{self.allowed_states}` and failed states `{self.failed_states}`"
)
- if external_task_id is not None and external_task_ids is not None:
+ if external_task_id and external_task_ids:
raise ValueError(
"Only one of `external_task_id` or `external_task_ids` may "
- "be provided to ExternalTaskSensor; not both."
+ "be provided to ExternalTaskSensor; "
+ "Use external_task_id or external_task_ids or external_task_group_id."
)
- if external_task_id is not None:
- external_task_ids = [external_task_id]
+ if external_task_group_id and external_task_id:
+ raise ValueError(
+ "Only one of `external_task_group_id` or `external_task_id` may "
+ "be provided to ExternalTaskSensor; "
+ "Use external_task_id or external_task_ids or external_task_group_id."
+ )
if external_task_group_id and external_task_ids:
raise ValueError(
- "Values for `external_task_group_id` and `external_task_id` or `external_task_ids` "
- "can't be set at the same time"
+ "Only one of `external_task_group_id` or `external_task_ids` may "
+ "be provided to ExternalTaskSensor; "
+ "Use external_task_id or external_task_ids or external_task_group_id."
)
Review Comment:
I’m not quite sure these changes are needed, or even beneficial at all.
--
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 diff in pull request #28097: logging poke info when external dag is not none and task_id and task_ids are none
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #28097:
URL: https://github.com/apache/airflow/pull/28097#discussion_r1060221241
##########
airflow/sensors/external_task.py:
##########
@@ -140,21 +140,30 @@ def __init__(
f"`{self.allowed_states}` and failed states `{self.failed_states}`"
)
- if external_task_id is not None and external_task_ids is not None:
+ if external_task_id and external_task_ids:
Review Comment:
Does `external_task_ids=[]` make sense? If it does, we’d want to keep using `is not 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] surabathini commented on a diff in pull request #28097: logging poke info when external dag is not none and task_id and task_ids are none
Posted by GitBox <gi...@apache.org>.
surabathini commented on code in PR #28097:
URL: https://github.com/apache/airflow/pull/28097#discussion_r1063225098
##########
airflow/sensors/external_task.py:
##########
@@ -217,6 +226,13 @@ def poke(self, context, session=None):
serialized_dttm_filter,
)
+ if self.external_dag_id and not self.external_task_group_id and not self.external_task_ids:
+ self.log.info(
+ "Poking for dag '%s' on %s ... ",
Review Comment:
for us this message is very helpful to know which run_id we are pocking without scrolling around when we are polling for the DAG status.
--
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] boring-cyborg[bot] commented on pull request #28097: logging poke info when external dag is not none and task_id and task_ids are none
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on PR #28097:
URL: https://github.com/apache/airflow/pull/28097#issuecomment-1336462609
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:
- Pay attention to the quality of your code (flake8, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/docs/apache-airflow/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/BREEZE.rst) for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#coding-style-and-best-practices).
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
--
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 diff in pull request #28097: logging poke info when external dag is not none and task_id and task_ids are none
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #28097:
URL: https://github.com/apache/airflow/pull/28097#discussion_r1064364073
##########
tests/sensors/test_external_task_sensor.py:
##########
@@ -351,6 +382,21 @@ def test_external_dag_sensor(self):
)
op.run(start_date=DEFAULT_DATE, end_date=DEFAULT_DATE, ignore_ti_state=True)
+ def test_external_dag_sensor_log(self, caplog):
+ other_dag = DAG("other_dag", default_args=self.args, end_date=DEFAULT_DATE, schedule="@once")
+ other_dag.create_dagrun(
+ run_id="test", start_date=DEFAULT_DATE, execution_date=DEFAULT_DATE, state=State.SUCCESS
+ )
+ op = ExternalTaskSensor(
+ task_id="test_external_dag_sensor_check",
+ external_dag_id="other_dag",
+ dag=self.dag,
+ )
+ with caplog.at_level(logging.INFO, logger=op.log.name):
+ caplog.clear()
Review Comment:
This seems unnecessary?
--
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 diff in pull request #28097: logging poke info when external dag is not none and task_id and task_ids are none
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #28097:
URL: https://github.com/apache/airflow/pull/28097#discussion_r1062158484
##########
airflow/sensors/external_task.py:
##########
@@ -140,21 +140,30 @@ def __init__(
f"`{self.allowed_states}` and failed states `{self.failed_states}`"
)
- if external_task_id is not None and external_task_ids is not None:
+ if external_task_id and external_task_ids:
raise ValueError(
"Only one of `external_task_id` or `external_task_ids` may "
- "be provided to ExternalTaskSensor; not both."
+ "be provided to ExternalTaskSensor; "
+ "Use external_task_id or external_task_ids or external_task_group_id."
)
- if external_task_id is not None:
- external_task_ids = [external_task_id]
+ if external_task_group_id and external_task_id:
+ raise ValueError(
+ "Only one of `external_task_group_id` or `external_task_id` may "
+ "be provided to ExternalTaskSensor; "
+ "Use external_task_id or external_task_ids or external_task_group_id."
+ )
if external_task_group_id and external_task_ids:
raise ValueError(
- "Values for `external_task_group_id` and `external_task_id` or `external_task_ids` "
- "can't be set at the same time"
+ "Only one of `external_task_group_id` or `external_task_ids` may "
+ "be provided to ExternalTaskSensor; "
+ "Use external_task_id or external_task_ids or external_task_group_id."
)
Review Comment:
There are some weird edge cases to consider when you rely on truthiness. For example, `external_task_id=""` is an invalid input that would now fail differently, and arguably passing it through the sensor (and never actually trigger) could be a better behaviour. And `external_task_ids=[]` is arguably a valid-ish input that could be useful for dynamically generated DAGs.
--
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] surabathini commented on a diff in pull request #28097: logging poke info when external dag is not none and task_id and task_ids are none
Posted by GitBox <gi...@apache.org>.
surabathini commented on code in PR #28097:
URL: https://github.com/apache/airflow/pull/28097#discussion_r1046904361
##########
airflow/sensors/external_task.py:
##########
@@ -140,21 +140,30 @@ def __init__(
f"`{self.allowed_states}` and failed states `{self.failed_states}`"
)
- if external_task_id is not None and external_task_ids is not None:
+ if external_task_id and external_task_ids:
raise ValueError(
"Only one of `external_task_id` or `external_task_ids` may "
- "be provided to ExternalTaskSensor; not both."
+ "be provided to ExternalTaskSensor; "
+ "Use external_task_id or external_task_ids or external_task_group_id."
)
- if external_task_id is not None:
- external_task_ids = [external_task_id]
+ if external_task_group_id and external_task_id:
+ raise ValueError(
+ "Only one of `external_task_group_id` or `external_task_id` may "
+ "be provided to ExternalTaskSensor; "
+ "Use external_task_id or external_task_ids or external_task_group_id."
+ )
if external_task_group_id and external_task_ids:
raise ValueError(
- "Values for `external_task_group_id` and `external_task_id` or `external_task_ids` "
- "can't be set at the same time"
+ "Only one of `external_task_group_id` or `external_task_ids` may "
+ "be provided to ExternalTaskSensor; "
+ "Use external_task_id or external_task_ids or external_task_group_id."
)
Review Comment:
Thanks for the review, added the above code to give the explicit error message between external_task_id and external_task_ids.
--
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