[GitHub] [airflow] potiuk commented on a diff in pull request #29760: `KubernetesPodOperator._render_nested_template_fields` improved by changing the conditionals for a map

["potiuk (via GitHub)" <gi...@apache.org>]

potiuk commented on code in PR #29760:
URL: https://github.com/apache/airflow/pull/29760#discussion_r1117897439


##########
airflow/providers/cncf/kubernetes/operators/kubernetes_pod.py:
##########
@@ -378,22 +378,16 @@ def _render_nested_template_fields(
         seen_oids: set,
     ) -> None:
         if id(content) not in seen_oids:
-            template_fields: tuple | None = None
-
-            if isinstance(content, k8s.V1EnvVar):
-                template_fields = ("value", "name")
-
-            if isinstance(content, k8s.V1ResourceRequirements):
-                template_fields = ("limits", "requests")
-
-            if isinstance(content, k8s.V1Volume):
-                template_fields = ("name", "persistent_volume_claim")
-
-            if isinstance(content, k8s.V1VolumeMount):
-                template_fields = ("name",)
-
-            if isinstance(content, k8s.V1PersistentVolumeClaimVolumeSource):
-                template_fields = ("claim_name",)
+            try:

Review Comment:
   Technically speaking it is not equivalent. Isinstance will also work in case the object is derived from the type it checks. Not sure if that might happen here, but I think it would be better to have array of tuples and run for loop and run isinstance for each element. Smth like:
   ```
   for _type, _template_fields in [(k8s.V1EnvVar, ("value", "name"), ....]:
      if isinstance(content, _type):
          template_fields = _template_fields
   ```



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