You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "jose-lpa (via GitHub)" <gi...@apache.org> on 2023/02/25 10:26:42 UTC

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

jose-lpa commented on code in PR #29760:
URL: https://github.com/apache/airflow/pull/29760#discussion_r1117900064


##########
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:
   It is not equivalent, as you said, if you want to check **any** objects type, also the ones derived from the base type. But in our specific case I see this is not going to happen, so the hashing is a nice fit IMO, faster and cleaner code than conditionals or loops.
   
   Take into account that the hashmap is a direct match, falling back to the default value in case it's a missed hit. For this few values we have to match this pattern is very efficient, and you avoid looping or triggering conditionals which are more expensive to run.
   
   If you need an example of the internal difference, [this SO thread has a very nice insight using `dis`](https://stackoverflow.com/a/15925086/515093).



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