Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow uppercase letters #403

Closed
dberenbaum opened this issue Sep 8, 2023 · 5 comments
Closed

Allow uppercase letters #403

dberenbaum opened this issue Sep 8, 2023 · 5 comments
Labels
discussion Discussion is needed to reach conclusion

Comments

@dberenbaum
Copy link
Contributor

See discussion in #398 (comment)

@shcheklein shcheklein added the discussion Discussion is needed to reach conclusion label Sep 23, 2023
@dberenbaum
Copy link
Contributor Author

@shcheklein
Copy link
Member

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
Copy link
Contributor

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
Copy link
Contributor

dacbd commented Oct 5, 2023

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

@dberenbaum
Copy link
Contributor Author

There's still discussion in iterative/dvc#9821, but let's open a separate issue if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discussion is needed to reach conclusion
Projects
None yet
Development

No branches or pull requests

4 participants