You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "yeswanth-bf (via GitHub)" <gi...@apache.org> on 2025/06/28 21:13:29 UTC
[PR] fix(external_task_sensor): fallback to execution_date key for execution_date_fn context (#52236) [airflow]
yeswanth-bf opened a new pull request, #52431:
URL: https://github.com/apache/airflow/pull/52431
Closes #52236.
**What this PR does:**
- In `_handle_execution_date_fn`, tries `context['logical_date']`, then the legacy `context['execution_date']`, then `dag_run.run_after`.
- Adds a pytest to validate the fallback to `execution_date`.
This prevents a KeyError for users whose context lacks `logical_date` but still have `execution_date`.
--
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
Re: [PR] fix(external_task_sensor): fallback to execution_date key for execution_date_fn context (#52236) [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #52431:
URL: https://github.com/apache/airflow/pull/52431#issuecomment-3016033967
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 Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:
- Pay attention to the quality of your code (ruff, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/contributing-docs/08_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/airflow-core/docs/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/dev/breeze/doc/README.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-docs/05_pull_requests.rst#coding-style-and-best-practices).
- Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
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
Re: [PR] fix(external_task_sensor): fallback to execution_date key for execution_date_fn context (#52236) [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #52431:
URL: https://github.com/apache/airflow/pull/52431#discussion_r2178711514
##########
providers/standard/src/airflow/providers/standard/sensors/external_task.py:
##########
@@ -534,7 +534,9 @@ def _handle_execution_date_fn(self, context) -> Any:
from airflow.utils.operator_helpers import make_kwargs_callable
# Remove "logical_date" because it is already a mandatory positional argument
- logical_date = context["logical_date"]
+ logical_date = (
Review Comment:
I think it would be better to move this logical_date get from context to a function and work out and return logical_date. there is some duplicity, this line and here https://github.com/apache/airflow/pull/52431/files#diff-cd0a81a3fe4e14f46d41d402eabcb273554245c7e676e549b8b84030a99d4870R256, can you do something like this please? and you can call the function with context?
```
def _get_logical_date(self, context):
if AIRFLOW_V_3_0_PLUS:
logical_date = context.get("logical_date")
dag_run = context.get("dag_run")
if not (logical_date or (dag_run and dag_run.run_after)):
raise ValueError(
"Either `logical_date` or `run_after` should be provided in the task context"
)
return logical_date or dag_run.run_after
else:
execution_date = context.get("execution_date")
if not execution_date:
raise ValueError(
"Either `execution_date` should be provided in the task context"
)
return execution_date
```
--
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
Re: [PR] fix(external_task_sensor): fallback to execution_date key for execution_date_fn context (#52236) [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on code in PR #52431:
URL: https://github.com/apache/airflow/pull/52431#discussion_r2179482745
##########
providers/standard/src/airflow/providers/standard/sensors/external_task.py:
##########
@@ -522,6 +522,28 @@ def get_external_task_group_task_ids(self, session, dttm_filter):
dttm_filter, self.external_task_group_id, self.external_dag_id, session
)
+ def _get_logical_date(self, context) -> datetime.datetime:
Review Comment:
https://github.com/apache/airflow/pull/52237#issuecomment-3026990189
--
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
Re: [PR] fix(external_task_sensor): fallback to execution_date key for execution_date_fn context (#52236) [airflow]
Posted by "yeswanth-bf (via GitHub)" <gi...@apache.org>.
yeswanth-bf commented on code in PR #52431:
URL: https://github.com/apache/airflow/pull/52431#discussion_r2178974491
##########
providers/standard/src/airflow/providers/standard/sensors/external_task.py:
##########
@@ -534,7 +534,9 @@ def _handle_execution_date_fn(self, context) -> Any:
from airflow.utils.operator_helpers import make_kwargs_callable
# Remove "logical_date" because it is already a mandatory positional argument
- logical_date = context["logical_date"]
+ logical_date = (
Review Comment:
Thanks for the feedback, @gopidesupavan!
I've pulled that date-fallback logic into a new private helper _get_logical_date(context), and updated _handle_execution_date_fn to call it.
• Added _get_logical_date in external_task.py
• Refactored _handle_execution_date_fn to delegate there
• Updated and added unit tests in test_external_task_sensor.py
• Ran pre-commit and fixed formatting/imports
All tests are now passing locally and CI is green. Please let me know if you'd like any more tweaks!
--
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
Re: [PR] fix(external_task_sensor): fallback to execution_date key for execution_date_fn context (#52236) [airflow]
Posted by "dingo4dev (via GitHub)" <gi...@apache.org>.
dingo4dev commented on code in PR #52431:
URL: https://github.com/apache/airflow/pull/52431#discussion_r2179441841
##########
providers/standard/src/airflow/providers/standard/sensors/external_task.py:
##########
@@ -522,6 +522,28 @@ def get_external_task_group_task_ids(self, session, dttm_filter):
dttm_filter, self.external_task_group_id, self.external_dag_id, session
)
+ def _get_logical_date(self, context) -> datetime.datetime:
Review Comment:
I’m bringing this considered question forward because my PR https://github.com/apache/airflow/pull/52237 isn’t getting any ATTENTION.
The caller `_get_dttm_filter` is already handling the compatibility retrieval for `logical_date`. Why not replace this on the caller and pass the `logical_date` to `_handle_execution_date_fn` as args.
https://github.com/apache/airflow/blob/460b1c65748016b069f39fa61210db254176d914/providers/standard/src/airflow/providers/standard/sensors/external_task.py#L261-L273
--
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
Re: [PR] fix(external_task_sensor): fallback to execution_date key for execution_date_fn context (#52236) [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on PR #52431:
URL: https://github.com/apache/airflow/pull/52431#issuecomment-3090998776
@yeswanth-s1th do you need any help give me a shout.
--
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
Re: [PR] fix(external_task_sensor): fallback to execution_date key for execution_date_fn context (#52236) [airflow]
Posted by "yeswanth-s1th (via GitHub)" <gi...@apache.org>.
yeswanth-s1th commented on PR #52431:
URL: https://github.com/apache/airflow/pull/52431#issuecomment-3109040365
> @yeswanth-s1th any update on the test fixes? happy to fork these changes and add you as co-author
To optimize attribution visibility, could we keep my original commits authored by me, and add you as Co-authored-by: on any new/amended ones? If you end up squashing, would you mind setting me as the commit Author and yourself as a co-author so the signal remains intact?
```
Co-authored-by: Yeswanth Surampudi <ye...@gmail.com>
```
Thanks again for unblocking 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
Re: [PR] fix(external_task_sensor): fallback to execution_date key for execution_date_fn context (#52236) [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on PR #52431:
URL: https://github.com/apache/airflow/pull/52431#issuecomment-3104406308
@yeswanth-s1th any update on the test fixes? happy to fork these changes and add you as co-author
--
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
Re: [PR] fix(external_task_sensor): fallback to execution_date key for execution_date_fn context (#52236) [airflow]
Posted by "yeswanth-s1th (via GitHub)" <gi...@apache.org>.
yeswanth-s1th closed pull request #52431: fix(external_task_sensor): fallback to execution_date key for execution_date_fn context (#52236)
URL: https://github.com/apache/airflow/pull/52431
--
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
Re: [PR] fix(external_task_sensor): fallback to execution_date key for execution_date_fn context (#52236) [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on PR #52431:
URL: https://github.com/apache/airflow/pull/52431#issuecomment-3021720932
@yeswanth-bf added comment in issue, to get the log
--
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
Re: [PR] fix(external_task_sensor): fallback to execution_date key for execution_date_fn context (#52236) [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on PR #52431:
URL: https://github.com/apache/airflow/pull/52431#issuecomment-3027021058
compact tests failing, please take look at these test references: providers/standard/tests/unit/standard/operators/test_weekday.py
--
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
Re: [PR] fix(external_task_sensor): fallback to execution_date key for execution_date_fn context (#52236) [airflow]
Posted by "gopidesupavan (via GitHub)" <gi...@apache.org>.
gopidesupavan commented on PR #52431:
URL: https://github.com/apache/airflow/pull/52431#issuecomment-3026677943
@yeswanth-bf 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