Skip to content

bug fix: DateTimeSensor can't render jinja template if use native obj #50744

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
May 27, 2025

Conversation

nailo2c
Copy link
Contributor

@nailo2c nailo2c commented May 18, 2025

Closes: #38336

🐞 Problem

This is quite an interesting case.

  • With render_template_as_native_obj=True, the Jinja expression is stored as a raw string when DateTimeSensorAsync is instantiated
    {{ (data_interval_end + macros.timedelta(minutes=1)) }} → falls through line 91 and is kept as‑is.
  • At run‑time the template is rendered to a pendulum.DateTime object.
    When execute() calls
    moment = timezone.parse(self.target_time)
    timezone.parse expects a string and raises TypeError.

✅ Fix

  • Add a private helper _moment() that
    • returns self.target_time directly if it is already a datetime / pendulum.DateTime;
    • otherwise parses the string with timezone.parse.
  • Replace all direct uses of timezone.parse(self.target_time) with _moment().
  • No behaviour change for DAGs that keep render_template_as_native_obj=False.

📝Example DAG (runs successfully after this patch)

from datetime import datetime
from airflow import DAG
from airflow.providers.standard.sensors.date_time import DateTimeSensorAsync

with DAG(
    dag_id="test_date_time_sensor",
    start_date=datetime(2025, 1, 1),
    schedule=None,
    render_template_as_native_obj=True,
):
    DateTimeSensorAsync(
        task_id="wait",
        target_time="{{ (data_interval_end + macros.timedelta(minutes=1)) }}",
    )

截圖 2025-05-17 下午9 37 11

@nailo2c nailo2c changed the title fix bug: can't render jinja template if use native obj bug fix: can't render jinja template if use native obj May 18, 2025
Copy link
Member

@Lee-W Lee-W left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few nits. In general, it looks good. I won't mind if someone wants to merge it directly

@eladkal eladkal changed the title bug fix: can't render jinja template if use native obj bug fix: DateTimeSensor can't render jinja template if use native obj May 19, 2025
@nailo2c
Copy link
Contributor Author

nailo2c commented May 26, 2025

Hi @Lee-W, if everything still looks good to you, could you please merge it at your convenience? That would close #38336.
Thanks a lot! 🙏

@dwreeves
Copy link
Contributor

For whatever it's worth, looks good to me. Thanks for addressing the issue! I can't wait to remove all those | string's from my code 🙂

@Lee-W Lee-W merged commit 36cb4f3 into apache:main May 27, 2025
97 checks passed
@nailo2c
Copy link
Contributor Author

nailo2c commented May 27, 2025

I'm so glad I could help with this issue 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DateTimeSensor() does not work with datetimes rendered via render_template_as_native_obj=True
4 participants