tanmayrauth commented on code in PR #1331:
URL: https://github.com/apache/iceberg-go/pull/1331#discussion_r3486582291
##########
table/orphan_cleanup_test.go:
##########
@@ -157,6 +158,36 @@ func TestNormalizeNonURLPath(t *testing.T) {
}
}
+func TestVersionHintLocation(t *testing.T) {
Review Comment:
The three cases here all either contain `://` (s3, file:///) or fall through
to filepath.Join (local), so none of them actually exercise the
`file:`-prefix-only branch above. A `file:/tmp/table` →
`file:/tmp/table/metadata/version-hint.text` case would lock in the one input
that branch uniquely guards.
##########
table/orphan_cleanup.go:
##########
@@ -597,6 +597,16 @@ func normalizeFilePathWithConfig(path string, cfg
*orphanCleanupConfig) string {
return normalizeNonURLPath(path)
}
+func versionHintLocation(tableLocation string) string {
+ if strings.Contains(tableLocation, "://") ||
strings.HasPrefix(tableLocation, "file:") {
Review Comment:
Nice catch adding `|| strings.HasPrefix(tableLocation, "file:")` — since
`file:///x` and `file://host/x` already contain `://`, this branch is really
only here to cover the single-slash `file:/x` opaque form. Just confirming
that's the intent.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
