You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "michalsosn (via GitHub)" <gi...@apache.org> on 2023/10/18 12:11:07 UTC
[PR] Add LiteralValue wrapper rendered without templating [airflow]
michalsosn opened a new pull request, #35017:
URL: https://github.com/apache/airflow/pull/35017
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
Closes: #33694
The PR introduces the airflow.template.templater.LiteralValue class, which wraps a single value and makes the Templater immediately return, bypassing Jinja templating for the content.
This class allows you to use strings that include Jinja macros or recognized file extensions as-is, without the need to modify the Operator by altering its template_fields or template_ext.
In cases where a field contains a complex object, it also enables you to selectively disable rendering for a portion of the object. This level of granularity is not achievable by overriding the template_fields.
Additionally, it provides a cleaner approach to avoid rendering files, in contrast to the documented method of leaving a trailing space within the string.
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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] Add LiteralValue wrapper rendered without templating [airflow]
Posted by "michalsosn (via GitHub)" <gi...@apache.org>.
michalsosn commented on code in PR #35017:
URL: https://github.com/apache/airflow/pull/35017#discussion_r1363840911
##########
airflow/template/templater.py:
##########
@@ -167,6 +179,8 @@ def render_template(
return {k: self.render_template(v, context, jinja_env, oids) for k, v in value.items()}
elif isinstance(value, set):
return {self.render_template(element, context, jinja_env, oids) for element in value}
+ elif isinstance(value, LiteralValue):
+ return value.value
Review Comment:
Yeah, that's a better location
--
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] Add LiteralValue wrapper rendered without templating [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #35017:
URL: https://github.com/apache/airflow/pull/35017#discussion_r1364213778
##########
docs/apache-airflow/core-concepts/operators.rst:
##########
@@ -185,10 +185,22 @@ This will fail with ``TemplateNotFound: cat script.sh``, but we can prevent airf
fixed_print_script = BashOperator(
task_id="fixed_print_script",
- bash_callable="cat script.sh",
+ bash_command="cat script.sh",
)
fixed_print_script.template_ext = ()
+
+It is also possible to exclude a string from templating and use it directly by wrapping it in
+:class:`~airflow.template.templater.LiteralValue`. This approach disables the rendering of both macros and files and
+can be applied to selected nested fields, while retaining the default templating rules for the remainder of the content.
+
+.. code-block:: python
+
+ fixed_print_script = BashOperator(
+ task_id="fixed_print_script",
+ bash_command=LiteralValue("cat script.sh"),
+ )
+
Review Comment:
It’d be a good idea to add a `.. versionadded` marker somewhere around here.
Also, [`airflow.template.templater` is not a public module](https://airflow.apache.org/docs/apache-airflow/stable/public-airflow-interface.html), and I’m not sure if we want to open it up (all its other contents are private); we may want to move the class somewhere else.
##########
docs/apache-airflow/core-concepts/operators.rst:
##########
@@ -185,10 +185,22 @@ This will fail with ``TemplateNotFound: cat script.sh``, but we can prevent airf
fixed_print_script = BashOperator(
task_id="fixed_print_script",
- bash_callable="cat script.sh",
+ bash_command="cat script.sh",
)
fixed_print_script.template_ext = ()
+
+It is also possible to exclude a string from templating and use it directly by wrapping it in
+:class:`~airflow.template.templater.LiteralValue`. This approach disables the rendering of both macros and files and
+can be applied to selected nested fields, while retaining the default templating rules for the remainder of the content.
+
+.. code-block:: python
+
+ fixed_print_script = BashOperator(
+ task_id="fixed_print_script",
+ bash_command=LiteralValue("cat script.sh"),
+ )
+
Review Comment:
It’d be a good idea to add a `.. versionadded` marker somewhere around here.
Also, [`airflow.template.templater` is not a public module](https://airflow.apache.org/docs/apache-airflow/stable/public-airflow-interface.html), and I’m not sure if we want to open it up (all its other contents are private); we may want to move the class somewhere else.
--
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] Add LiteralValue wrapper rendered without templating [airflow]
Posted by "michalsosn (via GitHub)" <gi...@apache.org>.
michalsosn commented on code in PR #35017:
URL: https://github.com/apache/airflow/pull/35017#discussion_r1370785630
##########
tests/template/test_templater.py:
##########
@@ -60,3 +60,23 @@ def test_render_template(self):
templater.template_ext = [".txt"]
rendered_content = templater.render_template(templater.message, context)
assert rendered_content == "Hello world"
+
+ def test_not_render_literal_value(self):
+ templater = Templater()
+ templater.template_ext = []
+ context = Context({"name": "world"}) # type: ignore
Review Comment:
I guess if the test is meant to keep a macro unresolved, it's better to leave the context empty or it can be confusing
--
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] Add LiteralValue wrapper rendered without templating [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr merged PR #35017:
URL: https://github.com/apache/airflow/pull/35017
--
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] Add LiteralValue wrapper rendered without templating [airflow]
Posted by "michalsosn (via GitHub)" <gi...@apache.org>.
michalsosn commented on code in PR #35017:
URL: https://github.com/apache/airflow/pull/35017#discussion_r1390545309
##########
airflow/template/templater.py:
##########
@@ -155,6 +167,8 @@ def render_template(
return self._render(template, context)
if isinstance(value, ResolveMixin):
return value.resolve(context)
+ if isinstance(value, LiteralValue):
+ return value.value
Review Comment:
I've come across this mixin, but the `iter_references` method appeared to be intended for a different use case. However, since `LiteralValue`, by definition, should have no references, I can implement it sensibly nonetheless.
--
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] Add LiteralValue wrapper rendered without templating [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #35017:
URL: https://github.com/apache/airflow/pull/35017#discussion_r1368236102
##########
airflow/utils/template.py:
##########
@@ -0,0 +1,32 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from typing import Any
+
+from airflow.template.templater import LiteralValue
+
+
+def literal_value(value: Any) -> LiteralValue:
Review Comment:
Maybe just `literal` would be good enough. Having a shorter name for DAG authors can be somewhat beneficial.
--
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] Add LiteralValue wrapper rendered without templating [airflow]
Posted by "michalsosn (via GitHub)" <gi...@apache.org>.
michalsosn commented on code in PR #35017:
URL: https://github.com/apache/airflow/pull/35017#discussion_r1365088714
##########
docs/apache-airflow/core-concepts/operators.rst:
##########
@@ -185,10 +185,22 @@ This will fail with ``TemplateNotFound: cat script.sh``, but we can prevent airf
fixed_print_script = BashOperator(
task_id="fixed_print_script",
- bash_callable="cat script.sh",
+ bash_command="cat script.sh",
)
fixed_print_script.template_ext = ()
+
+It is also possible to exclude a string from templating and use it directly by wrapping it in
+:class:`~airflow.template.templater.LiteralValue`. This approach disables the rendering of both macros and files and
+can be applied to selected nested fields, while retaining the default templating rules for the remainder of the content.
+
+.. code-block:: python
+
+ fixed_print_script = BashOperator(
+ task_id="fixed_print_script",
+ bash_command=LiteralValue("cat script.sh"),
+ )
+
Review Comment:
I see. In that case, `airflow.utils.template` might be a better location. The `airflow.utils` package contains other classes used to configure operators, such as `TriggerRule`.
I could define a function there, instead of directly exposing `LiteralValue`, which would make it easier to change the implementation in the future.
For example:
```
from airflow.template.templater import LiteralValue
def literal_value(value: Any) -> LiteralValue:
return LiteralValue(value)
```
--
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] Add LiteralValue wrapper rendered without templating [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #35017:
URL: https://github.com/apache/airflow/pull/35017#discussion_r1396735049
##########
docs/apache-airflow/core-concepts/operators.rst:
##########
@@ -158,37 +158,50 @@ See the `Jinja documentation <https://jinja.palletsprojects.com/en/2.11.x/api/#j
Some operators will also consider strings ending in specific suffixes (defined in ``template_ext``) to be references to files when rendering fields. This can be useful for loading scripts or queries directly from files rather than including them into DAG code.
-For example, consider a BashOperator which runs a multi-line bash script, this will load the file at ``script.sh`` and use its contents as the value for ``bash_callable``:
+For example, consider a BashOperator which runs a multi-line bash script, this will load the file at ``script.sh`` and use its contents as the value for ``bash_command``:
.. code-block:: python
run_script = BashOperator(
task_id="run_script",
- bash_callable="script.sh",
+ bash_command="script.sh",
)
By default, paths provided in this way should be provided relative to the DAG's folder (as this is the default Jinja template search path), but additional paths can be added by setting the ``template_searchpath`` arg on the DAG.
-In some cases you may want to disable template rendering on specific fields or prevent airflow from trying to read template files for a given suffix. Consider the following task:
+In some cases, you may want to exclude a string from templating and use it directly. Consider the following task:
.. code-block:: python
print_script = BashOperator(
task_id="print_script",
- bash_callable="cat script.sh",
+ bash_command="cat script.sh",
)
+This will fail with ``TemplateNotFound: cat script.sh``, but we can prevent airflow from treating this value as a reference to a file by wrapping it in :func:`~airflow.util.template.literal`.
Review Comment:
```suggestion
This will fail with ``TemplateNotFound: cat script.sh`` since Airflow would treat the string as a path to a file, not a command.
We can prevent airflow from treating this value as a reference to a file by wrapping it in :func:`~airflow.util.template.literal`.
```
##########
docs/apache-airflow/core-concepts/operators.rst:
##########
@@ -158,37 +158,50 @@ See the `Jinja documentation <https://jinja.palletsprojects.com/en/2.11.x/api/#j
Some operators will also consider strings ending in specific suffixes (defined in ``template_ext``) to be references to files when rendering fields. This can be useful for loading scripts or queries directly from files rather than including them into DAG code.
-For example, consider a BashOperator which runs a multi-line bash script, this will load the file at ``script.sh`` and use its contents as the value for ``bash_callable``:
+For example, consider a BashOperator which runs a multi-line bash script, this will load the file at ``script.sh`` and use its contents as the value for ``bash_command``:
.. code-block:: python
run_script = BashOperator(
task_id="run_script",
- bash_callable="script.sh",
+ bash_command="script.sh",
)
By default, paths provided in this way should be provided relative to the DAG's folder (as this is the default Jinja template search path), but additional paths can be added by setting the ``template_searchpath`` arg on the DAG.
-In some cases you may want to disable template rendering on specific fields or prevent airflow from trying to read template files for a given suffix. Consider the following task:
+In some cases, you may want to exclude a string from templating and use it directly. Consider the following task:
.. code-block:: python
print_script = BashOperator(
task_id="print_script",
- bash_callable="cat script.sh",
+ bash_command="cat script.sh",
)
+This will fail with ``TemplateNotFound: cat script.sh``, but we can prevent airflow from treating this value as a reference to a file by wrapping it in :func:`~airflow.util.template.literal`.
+This approach disables the rendering of both macros and files and can be applied to selected nested fields while retaining the default templating rules for the remainder of the content.
-This will fail with ``TemplateNotFound: cat script.sh``, but we can prevent airflow from treating this value as a reference to a file by overriding ``template_ext``:
+.. code-block:: python
+
Review Comment:
```suggestion
from airflow.utils.template import literal
```
--
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] Add LiteralValue wrapper rendered without templating [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #35017:
URL: https://github.com/apache/airflow/pull/35017#discussion_r1368238095
##########
tests/template/test_templater.py:
##########
@@ -60,3 +60,23 @@ def test_render_template(self):
templater.template_ext = [".txt"]
rendered_content = templater.render_template(templater.message, context)
assert rendered_content == "Hello world"
+
+ def test_not_render_literal_value(self):
+ templater = Templater()
+ templater.template_ext = []
+ context = Context({"name": "world"}) # type: ignore
+ content = LiteralValue("Hello {{ name }}")
+
+ rendered_content = templater.render_template(content, context)
+
+ assert rendered_content == "Hello {{ name }}"
+
+ def test_not_render_file_literal_value(self):
+ templater = Templater()
+ templater.template_ext = [".txt"]
+ context = Context({}) # type: ignore
Review Comment:
```suggestion
context = Context()
```
I this this would 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
Re: [PR] Add LiteralValue wrapper rendered without templating [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #35017:
URL: https://github.com/apache/airflow/pull/35017#discussion_r1390706598
##########
docs/apache-airflow/core-concepts/operators.rst:
##########
@@ -185,10 +185,23 @@ This will fail with ``TemplateNotFound: cat script.sh``, but we can prevent airf
fixed_print_script = BashOperator(
task_id="fixed_print_script",
- bash_callable="cat script.sh",
+ bash_command="cat script.sh",
)
fixed_print_script.template_ext = ()
Review Comment:
Let’s move the `literal` solution before this. We want to recommend it over this somewhat hacky workaround.
##########
docs/apache-airflow/core-concepts/operators.rst:
##########
@@ -185,10 +185,23 @@ This will fail with ``TemplateNotFound: cat script.sh``, but we can prevent airf
fixed_print_script = BashOperator(
task_id="fixed_print_script",
- bash_callable="cat script.sh",
+ bash_command="cat script.sh",
)
fixed_print_script.template_ext = ()
Review Comment:
Let’s move the `literal` solution before this. We want to recommend it over this somewhat hacky workaround.
--
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] Add LiteralValue wrapper rendered without templating [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #35017:
URL: https://github.com/apache/airflow/pull/35017#discussion_r1395224033
##########
docs/apache-airflow/core-concepts/operators.rst:
##########
@@ -158,37 +158,48 @@ See the `Jinja documentation <https://jinja.palletsprojects.com/en/2.11.x/api/#j
Some operators will also consider strings ending in specific suffixes (defined in ``template_ext``) to be references to files when rendering fields. This can be useful for loading scripts or queries directly from files rather than including them into DAG code.
-For example, consider a BashOperator which runs a multi-line bash script, this will load the file at ``script.sh`` and use its contents as the value for ``bash_callable``:
+For example, consider a BashOperator which runs a multi-line bash script, this will load the file at ``script.sh`` and use its contents as the value for ``bash_command``:
.. code-block:: python
run_script = BashOperator(
task_id="run_script",
- bash_callable="script.sh",
+ bash_command="script.sh",
)
By default, paths provided in this way should be provided relative to the DAG's folder (as this is the default Jinja template search path), but additional paths can be added by setting the ``template_searchpath`` arg on the DAG.
-In some cases you may want to disable template rendering on specific fields or prevent airflow from trying to read template files for a given suffix. Consider the following task:
+.. versionadded:: 2.8
+ In some cases, you may want to exclude a string from templating and use it directly. Consider the following task:
.. code-block:: python
print_script = BashOperator(
task_id="print_script",
- bash_callable="cat script.sh",
+ bash_command="cat script.sh",
)
+This will fail with ``TemplateNotFound: cat script.sh``, but we can prevent airflow from treating this value as a reference to a file by wrapping it in :func:`~airflow.util.template.literal`.
+This approach disables the rendering of both macros and files and can be applied to selected nested fields while retaining the default templating rules for the remainder of the content.
-This will fail with ``TemplateNotFound: cat script.sh``, but we can prevent airflow from treating this value as a reference to a file by overriding ``template_ext``:
+.. code-block:: python
+
+ fixed_print_script = BashOperator(
+ task_id="fixed_print_script",
+ bash_command=literal("cat script.sh"),
+ )
+
Review Comment:
```suggestion
.. versionadded:: 2.8
:func:`~airflow.util.template.literal` was added.
```
##########
docs/apache-airflow/core-concepts/operators.rst:
##########
@@ -158,37 +158,48 @@ See the `Jinja documentation <https://jinja.palletsprojects.com/en/2.11.x/api/#j
Some operators will also consider strings ending in specific suffixes (defined in ``template_ext``) to be references to files when rendering fields. This can be useful for loading scripts or queries directly from files rather than including them into DAG code.
-For example, consider a BashOperator which runs a multi-line bash script, this will load the file at ``script.sh`` and use its contents as the value for ``bash_callable``:
+For example, consider a BashOperator which runs a multi-line bash script, this will load the file at ``script.sh`` and use its contents as the value for ``bash_command``:
.. code-block:: python
run_script = BashOperator(
task_id="run_script",
- bash_callable="script.sh",
+ bash_command="script.sh",
)
By default, paths provided in this way should be provided relative to the DAG's folder (as this is the default Jinja template search path), but additional paths can be added by setting the ``template_searchpath`` arg on the DAG.
-In some cases you may want to disable template rendering on specific fields or prevent airflow from trying to read template files for a given suffix. Consider the following task:
+.. versionadded:: 2.8
+ In some cases, you may want to exclude a string from templating and use it directly. Consider the following task:
Review Comment:
```suggestion
In some cases, you may want to exclude a string from templating and use it directly. Consider the following task:
```
--
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] Add LiteralValue wrapper rendered without templating [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #35017:
URL: https://github.com/apache/airflow/pull/35017#discussion_r1382744257
##########
airflow/template/templater.py:
##########
@@ -155,6 +167,8 @@ def render_template(
return self._render(template, context)
if isinstance(value, ResolveMixin):
return value.resolve(context)
+ if isinstance(value, LiteralValue):
+ return value.value
Review Comment:
Instead of adding a special case, it may be better to make LiteralValue inherit ResolveMixin and implement `resolve` to just return `value`. This would avoid surprises if the resolution logic is used elsewhere (can’t think of one but you never know with a large code base).
##########
airflow/template/templater.py:
##########
@@ -155,6 +167,8 @@ def render_template(
return self._render(template, context)
if isinstance(value, ResolveMixin):
return value.resolve(context)
+ if isinstance(value, LiteralValue):
+ return value.value
Review Comment:
Instead of adding a special case, it may be better to make LiteralValue inherit ResolveMixin and implement `resolve` to just return `value`. This would avoid surprises if the resolution logic is used elsewhere (can’t think of one but you never know with a large code base).
--
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] Add LiteralValue wrapper rendered without templating [airflow]
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on code in PR #35017:
URL: https://github.com/apache/airflow/pull/35017#discussion_r1363820365
##########
airflow/template/templater.py:
##########
@@ -167,6 +179,8 @@ def render_template(
return {k: self.render_template(v, context, jinja_env, oids) for k, v in value.items()}
elif isinstance(value, set):
return {self.render_template(element, context, jinja_env, oids) for element in value}
+ elif isinstance(value, LiteralValue):
+ return value.value
Review Comment:
I think it is not a good place for this condition because this section marked as `# Fast path for common built-in collections.`
Maybe better move it right after `ResolveMixin`
--
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] Add LiteralValue wrapper rendered without templating [airflow]
Posted by "uranusjr (via GitHub)" <gi...@apache.org>.
uranusjr commented on code in PR #35017:
URL: https://github.com/apache/airflow/pull/35017#discussion_r1368237626
##########
tests/template/test_templater.py:
##########
@@ -60,3 +60,23 @@ def test_render_template(self):
templater.template_ext = [".txt"]
rendered_content = templater.render_template(templater.message, context)
assert rendered_content == "Hello world"
+
+ def test_not_render_literal_value(self):
+ templater = Templater()
+ templater.template_ext = []
+ context = Context({"name": "world"}) # type: ignore
Review Comment:
Why not just use a key that’s defined here?
##########
tests/template/test_templater.py:
##########
@@ -60,3 +60,23 @@ def test_render_template(self):
templater.template_ext = [".txt"]
rendered_content = templater.render_template(templater.message, context)
assert rendered_content == "Hello world"
+
+ def test_not_render_literal_value(self):
+ templater = Templater()
+ templater.template_ext = []
+ context = Context({"name": "world"}) # type: ignore
Review Comment:
Why not just use a key that’s defined here?
--
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