dstandish commented on code in PR #30727:
URL: https://github.com/apache/airflow/pull/30727#discussion_r1184597669
##########
airflow/executors/kubernetes_executor.py:
##########
@@ -834,7 +429,7 @@ def try_adopt_task_instances(self, tis:
Sequence[TaskInstance]) -> Sequence[Task
tis_to_flush_by_key = {ti.key: ti for ti in tis if ti.queued_by_job_id}
kube_client: client.CoreV1Api = self.kube_client
for scheduler_job_id in scheduler_job_ids:
- scheduler_job_id =
pod_generator.make_safe_label_value(str(scheduler_job_id))
+ scheduler_job_id =
self._make_safe_label_value(str(scheduler_job_id))
Review Comment:
the `str` calls stand out. i wonder why they are there. perhaps it's
something that should be pushed down. e.g. maybe signature should accept `int`
and wrap with `str` in that case.
##########
airflow/executors/kubernetes_executor.py:
##########
@@ -523,6 +93,19 @@ def _list_pods(self, query_kwargs):
return pods
+ def _make_safe_label_value(self, input_value: str | datetime) -> str:
+ """
+ Normalize a provided label to be of valid length and characters.
+ See airflow.kubernetes.pod_generator.make_safe_label_value for more
details
+ """
+ # airflow.kubernetes is an expensive import, locally import it here to
+ # speed up load times of the kubernetes_executor module.
+ from airflow.kubernetes import pod_generator
+
+ if isinstance(input_value, datetime):
+ return pod_generator.datetime_to_label_safe_datestring(input_value)
Review Comment:
maybe push this logic (handling of datetime) down to
`pod_generator.make_safe_label_value`?
then the two functions (make_safe_label_value) are identical and this one
only serves as a lazy import mechanism
--
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]
