SameerMesiah97 commented on code in PR #60999:
URL: https://github.com/apache/airflow/pull/60999#discussion_r2724461497
##########
task-sdk/src/airflow/sdk/definitions/_internal/templater.py:
##########
@@ -108,14 +373,6 @@ def resolve_template_files(self) -> None:
log.exception("Failed to get source %s", item)
self.prepare_template()
- def _should_render_native(self, dag: DAG | None = None) -> bool:
- # Operator explicitly set? Use that value, otherwise inherit from DAG
- render_op_template_as_native_obj = getattr(self,
"render_template_as_native_obj", None)
- if render_op_template_as_native_obj is not None:
- return render_op_template_as_native_obj
-
- return dag.render_template_as_native_obj if dag else False
-
Review Comment:
I do not understand why you removed this conditional check. Would it not
render operator level overrides for `render_template_as_native_obj` inert? If
you have a strong justification for this, I would advise explaining this change
in the PR description as this is breaking change in terms of behavior. Unless,
it was an accidental refactor, you might want to remove.
##########
task-sdk/src/airflow/sdk/definitions/_internal/templater.py:
##########
@@ -57,6 +64,264 @@ def resolve(self, context: Context) -> Any:
log = logging.getLogger(__name__)
+class ZipAwareFileSystemLoader(jinja2.FileSystemLoader):
+ """
+ A Jinja2 template loader that extends FileSystemLoader to support loading
templates from within zip archives.
+
+ This loader handles the case where DAGs are packaged as zip files.
+
+ This loader handles the case where DAGs are packaged as zip files. When a
+ searchpath contains a zip file path (e.g., "/path/to/dags.zip" or
+ "/path/to/dags.zip/subdir"), templates inside the zip can be loaded
transparently.
+
+ For regular filesystem paths, it delegates to the parent FileSystemLoader
+ for optimal performance.
+
+ This addresses Issue #59310 where template files in zipped DAG packages
+ could not be resolved by the standard FileSystemLoader.
+
+ :param searchpath: A list of paths to search for templates. Paths can be:
+ - Regular filesystem directories
+ - Paths to zip files (will search inside the zip root)
+ - Paths inside zip files (e.g., "archive.zip/subdir")
+ :param encoding: The encoding to use when reading template files.
+ :param followlinks: Whether to follow symbolic links (for regular
directories).
+
+ Example usage::
+
+ loader = ZipAwareFileSystemLoader(["/path/to/dags.zip",
"/path/to/templates"])
+ env = jinja2.Environment(loader=loader)
+ template = env.get_template("query.sql")
+
+ See: https://github.com/apache/airflow/issues/59310
+ """
Review Comment:
Minor nit: this docstring feels a bit long and repetitive for something that
shows up in IDE tooltips. Some of the contextual detail (like issue references)
might be better as a regular comment. I would suggest something like this:
```
A Jinja2 template loader that supports resolving templates from zipped DAG
packages.
Search paths may include filesystem directories, zip files, or subdirectories
within zip files. Searchpath ordering is preserved across zip and non-zip
entries.
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
