potiuk commented on code in PR #70234:
URL: https://github.com/apache/airflow/pull/70234#discussion_r3682331000


##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/operators/pod.py:
##########
@@ -1673,12 +1677,11 @@ def build_pod_request_obj(self, context: Context | None 
= None) -> k8s.V1Pod:
         pod.metadata.labels.update(labels)
         # Add Airflow Version to the label
         # And a label to identify that pod is launched by KubernetesPodOperator
-        pod.metadata.labels.update(
-            {
-                "airflow_version": airflow_version.replace("+", "-"),
-                "airflow_kpo_in_cluster": str(self.hook.is_in_cluster),
-            }
-        )
+        pod.metadata.labels.update({"airflow_version": 
airflow_version.replace("+", "-")})
+        if not dry_run:

Review Comment:
   The side effect is that a dry run now produces a pod spec **missing the 
`airflow_kpo_in_cluster` label** — so the thing `dry_run()` prints is not quite 
the thing that will be submitted, which is the one property a dry run is 
supposed to have.
   
   It's a defensible trade (a slightly incomplete spec beats an unusable 
command), but worth being deliberate: either note the omission in the 
`dry_run()` output so nobody diffs it against a real pod and gets confused, or 
set the label to a placeholder like `"unknown"` so the key is at least present 
in the rendered spec.
   
   ---
   Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting



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

Reply via email to