Re: [PR] add convert_env_vars_from_list_of_dicts on backwards_compat_converters [airflow]

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

Nataneljpwd commented on code in PR #47050:
URL: https://github.com/apache/airflow/pull/47050#discussion_r2992860595


##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/backcompat/backwards_compat_converters.py:
##########
@@ -68,6 +68,18 @@ def convert_port(port) -> k8s.V1ContainerPort:
     return _convert_kube_model_object(port, k8s.V1ContainerPort)
 
 
+def convert_env_vars_from_list_of_dicts(env_vars: list[dict[str, str]]) -> list[k8s.V1EnvVar]:
+    """
+    Coerce env var collection for kubernetes.
+
+    If the collection is a str-str list of dict, convert it into a list of ``V1EnvVar`` variables.
+    """
+    if isinstance(env_vars, list) and \
+        all(isinstance(item, dict) and "name" in item and "value" in item for item in env_vars):
+        return [k8s.V1EnvVar(name=env_var.get("name"), value=env_var.get("value")) for env_var in env_vars]
+    return env_vars

Review Comment:
   And also I suggest adding tests for this



##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/backcompat/backwards_compat_converters.py:
##########
@@ -68,6 +68,18 @@ def convert_port(port) -> k8s.V1ContainerPort:
     return _convert_kube_model_object(port, k8s.V1ContainerPort)
 
 
+def convert_env_vars_from_list_of_dicts(env_vars: list[dict[str, str]]) -> list[k8s.V1EnvVar]:
+    """
+    Coerce env var collection for kubernetes.
+
+    If the collection is a str-str list of dict, convert it into a list of ``V1EnvVar`` variables.
+    """
+    if isinstance(env_vars, list) and \
+        all(isinstance(item, dict) and "name" in item and "value" in item for item in env_vars):
+        return [k8s.V1EnvVar(name=env_var.get("name"), value=env_var.get("value")) for env_var in env_vars]
+    return env_vars

Review Comment:
   So if it is a list of dicts, yet one does not have the name / value key (or there is a none value) this will just return the env_vars which will raise an error, I think this needs to be addressed as well, also what about just allowing to pass a dict where the key is the env name, and the value is the value?



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