util.parse should support accessing tags, which would require a less restrictive regex

[gaurav]

util.parse() currently assumes that the branch name is "Only letters, digits, underscores and dash, no leading dash."

lakefs-spec/src/lakefs_spec/util.py

Lines 118 to 130 in 06134be

	# First regex reflects the lakeFS repository naming rules:
	# only lowercase letters, digits and dash, no leading dash, minimum 3, maximum 63 characters
	# https://docs.lakefs.io/understand/model.html#repository
	# Second regex is the branch: Only letters, digits, underscores and dash, no leading dash.
	path_regex = re.compile(r"(?:lakefs://)?([a-z0-9][a-z0-9\-]{2,62})/(\w[\w\-]*)/(.*)")
	results = path_regex.fullmatch(path)
	if results is None:
	raise ValueError(
	f"expected path with structure lakefs://<repo>/<ref>/<resource>, got {path!r}"
	)
	repo, ref, resource = results.groups()
	return repo, ref, resource

However, users might want to use a tag name instead of a branch name, which has a much more complicated syntax. Notably, the current regex doesn't match the two example tags in the LakeFS docs (v2.3 and dev-jane-before-v2.3-merge). It might also be useful to support ref expressions.

I would suggest making the regex less restrictive -- at a minimum (\w[\w\-\.]*), but possibly going all the way to support ref expressions to include [~^]. The error could also be rewritten to be a bit clearer (e.g. because lakefs.ls(...) calls this after calling _strip_protocol(), I get the following error message: ValueError: expected path with structure lakefs://<repo>/<ref>/<resource>, got 'heal-studies/v2.0/' -- which made me think that I had forgotten to add the lakefs:// prefix before realizing that the branch was being rejected for being incorrect).

Alternatively, if I'm misunderstanding how tags and ref expressions should be used in lakefs-spec, please let me know -- and thank you so much for building this great library!

[nicholasjng]

Thank you for the detailed report!

Since we want to fully implement lakeFS's URI specification, we must also match their support for ref expressions. Your regex suggestion sounds great, would you be willing to send a PR?

I agree that the error message is a bit sparse, and even misleading in the case you presented. We might need to check our callsites for the order of util.parse() and _strip_protocol() - alternatively, we could decide to split the protocol off the template path if the input doesn't have one, so we don't confuse people with the protocol unnecessarily.

[gaurav]

This works great now, thanks so much for the quick fix!