[GitHub] [airflow] juhai commented on a diff in pull request #32278: Allow creating unique pod and hostname for KubernetesPodOperator from task job_id

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

juhai commented on code in PR #32278:
URL: https://github.com/apache/airflow/pull/32278#discussion_r1252014543


##########
airflow/providers/cncf/kubernetes/operators/pod.py:
##########
@@ -158,6 +158,8 @@ class KubernetesPodOperator(BaseOperator):
         suffix if random_name_suffix is True) to generate a pod id (DNS-1123 subdomain,
         containing only [a-z0-9.-]).
     :param random_name_suffix: if True, will generate a random suffix.
+    :param job_id_as_suffix: if True, will add ti.job_id to the pod name. random_name_suffix, if set to True
+        takes precedence over this parameter.

Review Comment:
   I've added another proposal which makes the following changes
   - Use name_suffix instead of two arguments to control the suffix operation as suggested
   - Make hostname suffix modification optional via `add_suffix_to_hostname`. I am not aware of situations where hostname could/should be different from pod name but the random suffix has not been applied to hostname before.
   - Change default of `random_suffix` to False and notify about deprecation if it's set to True.
   - Clean `_create_pod_id` as some options were never given. Set the full pod name with suffix inside to take account both random and job_id based suffix.



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