search

iterative / gto

🏷️ Git Tag Ops. Turn your Git repository into Artifact Registry or Model Registry.
https://dvc.org/doc/gto
Apache License 2.0
140 stars 16 forks source link

Allow uppercase letters #403

Closed dberenbaum closed 12 months ago

dberenbaum commented 1 year ago

See discussion in https://github.com/iterative/gto/pull/398#discussion_r1312155921

dberenbaum commented 1 year ago

See also https://iterativeai.slack.com/archives/C05L7S1TWAW/p1695604913336239?thread_ts=1695300068.062779&cid=C05L7S1TWAW

shcheklein commented 1 year ago

One extra consideration. In case of a monorepo, or even nested dvc.yaml (?) we pass path name as part of the Git tag. I'm not sure if we are sanitizing it. Also, it means we have to support pretty much everything we allow in a path (except for some special symbols may be). @aguschin do you remember if you had any thoughts on this while developing monorepo scenario?

aguschin commented 1 year ago

In case of a monorepo, or even nested dvc.yaml (?) we pass path name as part of the Git tag.

Yes, it's true.

I'm not sure if we are sanitizing it

No. But GTO should error out if path contains symbols that aren't allowed. There is a regex for this in constants.py IIRC.

do you remember if you had any thoughts on this while developing monorepo scenario?

No specific thoughts, IIRC, except that I wanted to get back to this once I hear some user problem / concern around it.

dacbd commented 12 months ago

@dberenbaum, with the PR merged, are there any other concerns, or is this safe to close?

dberenbaum commented 12 months ago

There's still discussion in https://github.com/iterative/dvc/issues/9821, but let's open a separate issue if needed.