jedcunningham commented on code in PR #53477:
URL: https://github.com/apache/airflow/pull/53477#discussion_r2220941767
##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/operators/pod.py:
##########
@@ -1132,7 +1132,10 @@ def _build_find_pod_label_selector(self, context:
Context | None = None, *, excl
**self.labels,
**self._get_ti_pod_labels(context, include_try_number=False),
}
- label_strings = [f"{label_id}={label}" for label_id, label in
sorted(labels.items())]
+ label_strings = [
+ f"{label_id}={str(label) if label is not None else ''}"
+ for label_id, label in sorted(labels.items())
+ ]
Review Comment:
Unfortunately, folks do have that code in the wild or we wouldn't see this
problem. And I'm not sure it's worth a breaking change to change it.
##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/operators/pod.py:
##########
@@ -1132,7 +1132,10 @@ def _build_find_pod_label_selector(self, context:
Context | None = None, *, excl
**self.labels,
**self._get_ti_pod_labels(context, include_try_number=False),
}
- label_strings = [f"{label_id}={label}" for label_id, label in
sorted(labels.items())]
+ label_strings = [
+ f"{label_id}={str(label) if label is not None else ''}"
Review Comment:
I wonder if we'd be better off with a helper function that consistently
processes these labels instead of relying on the k8s client to empty string it
for us, then we can use it in both places.
##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/operators/pod.py:
##########
@@ -1132,7 +1132,10 @@ def _build_find_pod_label_selector(self, context:
Context | None = None, *, excl
**self.labels,
Review Comment:
My immediate instinct is to not include user provided labels in the first
place! But, I'm a [bit late to that
objection](https://github.com/apache/airflow/pull/33057).
--
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-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
