potiuk commented on a change in pull request #19257:
URL: https://github.com/apache/airflow/pull/19257#discussion_r738191070
##########
File path: airflow/models/dag.py
##########
@@ -368,8 +369,11 @@ def __init__(
DeprecationWarning,
stacklevel=2,
)
-
- validate_key(dag_id)
+
+ if not is_ascii(dag_id):
+ # slugify dag id
+ dag_id = slugify(dag_id, lowercase=False)
Review comment:
Manual splitting/joining would be kinda against the purpose of using
library :)
i did check unicode-slugify (from mozilla) and it does a bit better and we
could use it out-of-the-box:
```
>>> slugify('dag-你好_cześć', only_ascii=True)
'dag-ni-hao-_czesc'
>>> slugify('dag_你好_cześć', only_ascii=True)
'dag_ni-hao-_czesc'
```
However there is a problem because I believe it conflicts with
python-slugify (they both use `slugify` as package name) - so unless we can use
some ways to have them both installed at the same time (?) - I looked how we
used python_slugify and from a quick look, it looks we could replace the usage
of python_slugify with mozilla one.
It uses a bit different parameters (so instead of regex_pattern we would
have to simply add more characters to `ok=` it seems and remove some in the
place where kubernetes uses it (it only accepts `-` but this can be easily
changed be limiting `ok=` parameter of mozilla slugify.
WDYT @uranusjr @Melodie97 ? That would be an interesting one also to add
some tests to make sure that the replacement works well.
--
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]
