jedcunningham commented on code in PR #27202:
URL: https://github.com/apache/airflow/pull/27202#discussion_r1004949553
##########
airflow/providers/cncf/kubernetes/hooks/kubernetes.py:
##########
@@ -319,9 +319,32 @@ def get_custom_object(
raise AirflowException(f"Exception when calling ->
get_custom_object: {e}\n")
def get_namespace(self) -> str | None:
- """Returns the namespace that defined in the connection"""
+ """
+ Returns the namespace defined in the connection or 'default'.
+
+ TODO: in provider version 6.0, return None when namespace not defined
in connection
+ """
+ namespace = self._get_namespace()
+ if self.conn_id and not namespace:
+ warnings.warn(
+ "Airflow connection defined but namespace is not set;
'default'. In "
Review Comment:
```suggestion
"Airflow connection defined but namespace is not set;
returning 'default'. In "
```
##########
airflow/providers/cncf/kubernetes/CHANGELOG.rst:
##########
@@ -39,6 +39,13 @@ Features
* KubernetsPodOperator argument ``namespace`` is now optional. If not
supplied via KPO param or pod template file or full pod spec, then we'll check
the airflow conn,
then if in a k8s pod, try to infer the namespace from the container, then
finally will use the ``default`` namespace.
+Deprecations
+~~~~~~~~~~~~
+
+* In KubernetesHook if connection defined but namespace unset, we currently
return 'default'; this behavior is deprecated. In next release, we'll return
``None``.
Review Comment:
```suggestion
* In ``KubernetesHook.get_namespace``, if a connection is defined but a
namespace isn't set, we currently return 'default'; this behavior is
deprecated. In the next release, we'll return ``None``.
```
##########
airflow/providers/cncf/kubernetes/hooks/kubernetes.py:
##########
@@ -319,9 +319,32 @@ def get_custom_object(
raise AirflowException(f"Exception when calling ->
get_custom_object: {e}\n")
def get_namespace(self) -> str | None:
- """Returns the namespace that defined in the connection"""
+ """
+ Returns the namespace defined in the connection or 'default'.
+
+ TODO: in provider version 6.0, return None when namespace not defined
in connection
+ """
+ namespace = self._get_namespace()
+ if self.conn_id and not namespace:
+ warnings.warn(
+ "Airflow connection defined but namespace is not set;
'default'. In "
+ "cncf.kubernetes provider version 6.0 we will return None when
namespace is "
+ "not defined in the connection so that it's clear whether user
intends 'default' or "
+ "whether namespace is unset (which is required in order to
apply precedence logic in "
+ "KubernetesPodOperator.",
Review Comment:
```suggestion
"KubernetesPodOperator).",
```
--
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]
