Support tags with colon (:)

zyd14 commented 3 months ago

What's the use case?

I recently noticed that we are getting warnings when compiling assets / ops etc. that have tags with a colon in them.

DeprecationWarning: Non-compliant tag keys like ['etx:usage_type', 'etx:owner', 'etx:deployment'] are deprecated. Allowed characters: alpha-numeric, '_', '-', '.'. Tag keys can also contain a namespace section, separated by a '/'. Each section must have <= 63 characters.

Would the Dagster team please consider allowing colons in tags? They are a very common way to namespace tags, and for orgs like mine where we've already put substantial work into tagging with colon-separated namespaces it will be difficult to shift to tags separated with '/'

Ideas of implementation

No response

Additional information

No response

Message from the maintainers

Impacted by this issue? Give it a 👍! We factor engagement into prioritization.

garethbrickman commented 3 months ago

Relates to this commit: https://github.com/dagster-io/dagster/commit/b326b9763c4013cb81bcf1a69f8695cf8686ca6d

sryza commented 3 months ago

Hey @zyd14 - this isn't set in stone, but the two main reasons we went in this direction were:

Tags are key-value pairs, and it's easy for "a:b" to look like "a" is the key and "b" is the value
We largely cargo-culted the Kubernetes syntax for labels, for the sake of consistency and because they seem to have a pretty principled approach

zyd14 commented 3 months ago

Fair points. We took inspiration from AWS as that's where most of our tags surface anyway and they do colon-separated namespaces for keys. I've ran into a few services that don't allow it now though which makes me feel like maybe we've got it wrong.

sryza commented 3 months ago

To be clear, I think it is extremely unlikely that we'd actually be able to drop support for these keys. The deprecation warning is there to guide new users towards a particular direction, which will allow us to build UI that favors it (e.g. visually distinguishing the part before and after the "/").

If the deprecation warning itself is cramping your style, we might be able to add a way to disable it more comprehensively in your setup?

dagster-io / dagster