[GitHub] [airflow] houqp commented on a change in pull request #16636: Added dashes to pod naming

[GitBox <gi...@apache.org>]

houqp commented on a change in pull request #16636:
URL: https://github.com/apache/airflow/pull/16636#discussion_r665076303



##########
File path: airflow/kubernetes/kubernetes_helper_functions.py
##########
@@ -29,15 +29,13 @@ def _strip_unsafe_kubernetes_special_chars(string: str) -> str:
     """
     Kubernetes only supports lowercase alphanumeric characters, "-" and "." in
     the pod name.
-    However, there are special rules about how "-" and "." can be used so let's
-    only keep
-    alphanumeric chars  see here for detail:
+    See here for detail:
     https://kubernetes.io/docs/concepts/overview/working-with-objects/names/
 
     :param string: The requested Pod name
     :return: Pod name stripped of any unsafe characters
     """
-    return ''.join(ch.lower() for ch in list(string) if ch.isalnum())
+    return ''.join(ch.lower() for ch in list(string) if ch.isalnum() or ch in ['-'])

Review comment:
       if we are to use regex, we should use a precompiled regex object. If we are not going to use regex here, then i think we should just do the comparison with `ch == '-'` without creating a new list in memory.




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