rjgoyln commented on code in PR #71174:
URL: https://github.com/apache/airflow/pull/71174#discussion_r3998951826


##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/operators/pod.py:
##########
@@ -533,6 +530,23 @@ def __init__(
         self.container_name_log_prefix_enabled = 
container_name_log_prefix_enabled
         self.log_formatter = log_formatter
 
+    @property
+    def container_logs(self) -> Iterable[str] | str | Literal[True]:
+        # Falls back lazily rather than in __init__: base_container_name is a 
template field, so
+        # the fallback has to read it once rendering has happened.
+        return self._container_logs or self.base_container_name
+
+    @container_logs.setter
+    def container_logs(self, value: Iterable[str] | str | Literal[True] | 
None) -> None:
+        self._container_logs = value
+
+    def render_template_fields(self, context: Context, jinja_env: 
jinja2.Environment | None = None) -> None:
+        # A str-str mapping has to become V1EnvVar objects before rendering: 
rendering a dict
+        # covers only its values, whereas an env var name is a template field 
of V1EnvVar.
+        if isinstance(self.env_vars, dict):
+            self.env_vars = convert_env_vars(self.env_vars)
+        super().render_template_fields(context, jinja_env)

Review Comment:
   Confirmed — thanks. I moved the conversion into _do_render_template_fields, 
which is shared by both the regular and mapped rendering paths.
   
   While auditing the same gap in @task.kubernetes, I found that its execute() 
additionally expands self.env_vars directly, so the mapped case could treat 
mapping keys as env var names. Since the same conversion fixes both cases, I’ve 
included it here and added a regression test covering .partial().expand().



-- 
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]

Reply via email to