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



##########
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:
       Not many reasons TBH, just it’s probably easier to understand IMO.
   
   ```python
   return re.sub(r"[^-a-z0-9]+", string.lower())
   ```
   
   It may also be easier to implement additional logic, such as avoiding `-` in 
the beginning of the string (which the logic you implement right now does not 
do).




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to