potiuk commented on code in PR #29760:
URL: https://github.com/apache/airflow/pull/29760#discussion_r1117904897
##########
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:
I'd argue that efficency in this case has very little importance. Even the
SO answer you pointed out has this very statement (which I generally very much
agree with):
> Generally, you should only bother to optimize code if you really need to,
i.e. if the program's performance is unusably slow.
Having a for loop with array of `type -> "fields" tuples does not seem like
a bad idea (neither from performance POV). Especially in Python 3.11 (which we
hopefully support) with Specialized Adaptive Interpreter, those are precisely
the kinds of optimizations that Python Interpreter will be able to optimize
very efficiently.
I am simply extremely cautious with backwards-compatibility. I am ok with
either approach if others, who get more k8s experience and defined the k8s
configuration scheme. Maybe there are cases when those entities are extended.
cc: @dstandish @dimberman
--
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]
