Support jinja2 native Python types #14603
Conversation
There was a problem hiding this comment.
Not a huge fan of needing to add a new arg (I understand why we need it though);
Perhaps we could make something like this work:
from airflow.templates import native_template as nt
task = MockOperator(task_id="op1", arg1=nt("{{ foo }}"(, arg2=nt("{{ bar }}") arg3='{{ ds }}' )
|
I think that flag on DAG level will make things more consistent. The Whatever we will decide - lets extend the docs to cover this feature 👌 |
|
Re-thinking this -- I agree with Tomek that keeping it same per DAG makes it more consistent and reason about and remove complexity. @ashb WDYT? If you agree with current approach, I can add docs to describe this feature |
|
Sure, I guess this will work and it's unlikely people will want to mix them in one dag? Not sure. Docs now please! |
Done in 4aacd2b |
|
The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*. |
This is second attempt to apache#4900 Docs: https://jinja.palletsprojects.com/en/2.11.x/nativetypes/ ```python >>> from jinja2 import nativetypes >>> ne = nativetypes.NativeEnvironment() >>> import pendulum >>> ne.from_string('{{ x }}').render(x=pendulum.now()) <Pendulum [2021-03-04T15:33:17.073343+00:00]> >>> ne.from_string('{{ x }}').render(x=pendulum.now().isoformat()) '2021-03-04T15:33:29.516540+00:00' >>> ne.from_string('{{ x }}').render(x="2012-10-10") '2012-10-10' ``` Current: ```python >>> environment.Environment().from_string('{{ ["w","x"] }}').render() "['w', 'x']" ``` Proposed: ```python >>> nativetypes.NativeEnvironment().from_string('{{ ["w","x"] }}').render() ['w', 'x'] ```
The problem with replacing `NativeEnvironment` as `Environment` only is that `NativeEnvironment` does not raise an error on Undefined templates: https://github.com/pallets/jinja/blob/2.11.3/src/jinja2/nativetypes.py#L70-L94
|
Added docs & Tests are green, ping @ashb @turbaszek |
github-actions
bot
added
the
full tests needed
|
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 master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease. |
This is second attempt to #4900
Docs: https://jinja.palletsprojects.com/en/2.11.x/nativetypes/
Current:
Proposed:
The problem with replacing
EnvironmentasNativeEnvironmentonly isthat
NativeEnvironmentdoes not raise an error on Undefined templates so it won't respectDAG.template_undefined:https://github.com/pallets/jinja/blob/2.11.3/src/jinja2/nativetypes.py#L70-L94
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.