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 these few values that 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
