You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "alessandro-nori (via GitHub)" <gi...@apache.org> on 2025/09/12 12:36:47 UTC
[PR] refactor: replace bucketName with KeyExtractor function for URI preprocessing [iceberg-go]
alessandro-nori opened a new pull request, #562:
URL: https://github.com/apache/iceberg-go/pull/562
## Summary
- Replace hardcoded bucketName field with injectable KeyExtractor function in blobFileIO
- Add specialized adlsKeyExtractor for Azure schemes using regex pattern from Java implementation
- Maintain backward compatibility while improving flexibility and error handling
- Add comprehensive unit tests for both key extractors
## Changes
- **blob.go**: Replaced bucketName with KeyExtractor function parameter
- **azure.go**: Added adlsKeyExtractor with regex pattern matching Azure URI format
- **io.go**: Updated scheme routing to use appropriate key extractors
- **Tests**: Added unit tests covering various URI formats and edge cases
## Test Plan
- [x] All existing tests pass
- [x] New unit tests for defaultKeyExtractor and adlsKeyExtractor
- [x] Linting passes with 0 issues
- [x] Build completes successfully
Fixes #561
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: Azure URI parsing with `KeyExtractor` function for `blobFileIO` preprocessing [iceberg-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #562:
URL: https://github.com/apache/iceberg-go/pull/562#discussion_r2346795854
##########
io/azure_test.go:
##########
@@ -145,3 +145,58 @@ func TestNewAdlsLocationUriParsing(t *testing.T) {
})
}
}
+
+func TestAdlsKeyExtractor(t *testing.T) {
+ extractor := adlsKeyExtractor()
+
+ tests := []struct {
+ name string
+ input string
+ expectedKey string
+ shouldError bool
+ }{
+ {
+ name: "abfs valid URI",
+ input: "abfs://container@account.dfs.core.windows.net/path/to/file.parquet",
+ expectedKey: "path/to/file.parquet",
+ },
+ {
+ name: "abfss valid URI",
+ input: "abfss://container@account.dfs.core.windows.net/path/to/file.parquet",
+ expectedKey: "path/to/file.parquet",
+ },
+ {
+ name: "wasb valid URI",
+ input: "wasb://container@account.blob.core.windows.net/path/to/file.parquet",
+ expectedKey: "path/to/file.parquet",
+ },
+ {
+ name: "wasbs valid URI",
+ input: "wasbs://container@account.blob.core.windows.net/path/to/file.parquet",
+ expectedKey: "path/to/file.parquet",
+ },
+ {
+ name: "URI with no path",
+ input: "abfs://container@account.dfs.core.windows.net",
+ expectedKey: "",
Review Comment:
I agree, let's update this to return an error in this case
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: Azure URI parsing with `KeyExtractor` function for `blobFileIO` preprocessing [iceberg-go]
Posted by "alessandro-nori (via GitHub)" <gi...@apache.org>.
alessandro-nori commented on code in PR #562:
URL: https://github.com/apache/iceberg-go/pull/562#discussion_r2344276340
##########
io/azure.go:
##########
@@ -184,3 +189,18 @@ func createAzureBucket(ctx context.Context, parsed *url.URL, props map[string]st
return azureblob.OpenBucket(ctx, client, nil)
}
+
+// adlsKeyExtractor creates a key extractor for Azure schemes using the adlsURIPattern pattern
+func adlsKeyExtractor() KeyExtractor {
+ return func(path string) (string, error) {
+ matches := adlsURIPattern.FindStringSubmatch(path)
+ if len(matches) < 4 {
+ // If it doesn't match Azure pattern, return path as-is
+ return "", fmt.Errorf("invalid ADLS location: %s", path)
Review Comment:
🙇 right, I changed the code but forgot to remove the comment
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: Azure URI parsing with `KeyExtractor` function for `blobFileIO` preprocessing [iceberg-go]
Posted by "mrcnc (via GitHub)" <gi...@apache.org>.
mrcnc commented on code in PR #562:
URL: https://github.com/apache/iceberg-go/pull/562#discussion_r2345176753
##########
io/azure_test.go:
##########
@@ -145,3 +145,58 @@ func TestNewAdlsLocationUriParsing(t *testing.T) {
})
}
}
+
+func TestAdlsKeyExtractor(t *testing.T) {
+ extractor := adlsKeyExtractor()
+
+ tests := []struct {
+ name string
+ input string
+ expectedKey string
+ shouldError bool
+ }{
+ {
+ name: "abfs valid URI",
+ input: "abfs://container@account.dfs.core.windows.net/path/to/file.parquet",
+ expectedKey: "path/to/file.parquet",
+ },
+ {
+ name: "abfss valid URI",
+ input: "abfss://container@account.dfs.core.windows.net/path/to/file.parquet",
+ expectedKey: "path/to/file.parquet",
+ },
+ {
+ name: "wasb valid URI",
+ input: "wasb://container@account.blob.core.windows.net/path/to/file.parquet",
+ expectedKey: "path/to/file.parquet",
+ },
+ {
+ name: "wasbs valid URI",
+ input: "wasbs://container@account.blob.core.windows.net/path/to/file.parquet",
+ expectedKey: "path/to/file.parquet",
+ },
+ {
+ name: "URI with no path",
+ input: "abfs://container@account.dfs.core.windows.net",
+ expectedKey: "",
Review Comment:
I think an error would be helpful here b/c it's not a valid location for any file that would be tracked in an Iceberg table. Also, the Java implementation could improve its validation logic so there's not a strong reason to copy it here
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: Azure URI parsing with `KeyExtractor` function for `blobFileIO` preprocessing [iceberg-go]
Posted by "alessandro-nori (via GitHub)" <gi...@apache.org>.
alessandro-nori commented on code in PR #562:
URL: https://github.com/apache/iceberg-go/pull/562#discussion_r2344281986
##########
io/azure_test.go:
##########
@@ -145,3 +145,58 @@ func TestNewAdlsLocationUriParsing(t *testing.T) {
})
}
}
+
+func TestAdlsKeyExtractor(t *testing.T) {
+ extractor := adlsKeyExtractor()
+
+ tests := []struct {
+ name string
+ input string
+ expectedKey string
+ shouldError bool
+ }{
+ {
+ name: "abfs valid URI",
+ input: "abfs://container@account.dfs.core.windows.net/path/to/file.parquet",
+ expectedKey: "path/to/file.parquet",
+ },
+ {
+ name: "abfss valid URI",
+ input: "abfss://container@account.dfs.core.windows.net/path/to/file.parquet",
+ expectedKey: "path/to/file.parquet",
+ },
+ {
+ name: "wasb valid URI",
+ input: "wasb://container@account.blob.core.windows.net/path/to/file.parquet",
+ expectedKey: "path/to/file.parquet",
+ },
+ {
+ name: "wasbs valid URI",
+ input: "wasbs://container@account.blob.core.windows.net/path/to/file.parquet",
+ expectedKey: "path/to/file.parquet",
+ },
+ {
+ name: "URI with no path",
+ input: "abfs://container@account.dfs.core.windows.net",
+ expectedKey: "",
Review Comment:
I followed the Java implementation and it returns an empty string in case of empty/null path.
https://github.com/apache/iceberg/blob/723d0998d69f254822efd6f8939e71c0723aaab2/azure/src/main/java/org/apache/iceberg/azure/adlsv2/ADLSLocation.java#L79
I think in our specific case it is ok to return an error and I'll update the code if we agree on it
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: Azure URI parsing with `KeyExtractor` function for `blobFileIO` preprocessing [iceberg-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade merged PR #562:
URL: https://github.com/apache/iceberg-go/pull/562
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: Azure URI parsing with `KeyExtractor` function for `blobFileIO` preprocessing [iceberg-go]
Posted by "twuebi (via GitHub)" <gi...@apache.org>.
twuebi commented on code in PR #562:
URL: https://github.com/apache/iceberg-go/pull/562#discussion_r2344245564
##########
io/azure_test.go:
##########
@@ -145,3 +145,58 @@ func TestNewAdlsLocationUriParsing(t *testing.T) {
})
}
}
+
+func TestAdlsKeyExtractor(t *testing.T) {
+ extractor := adlsKeyExtractor()
+
+ tests := []struct {
+ name string
+ input string
+ expectedKey string
+ shouldError bool
+ }{
+ {
+ name: "abfs valid URI",
+ input: "abfs://container@account.dfs.core.windows.net/path/to/file.parquet",
+ expectedKey: "path/to/file.parquet",
+ },
+ {
+ name: "abfss valid URI",
+ input: "abfss://container@account.dfs.core.windows.net/path/to/file.parquet",
+ expectedKey: "path/to/file.parquet",
+ },
+ {
+ name: "wasb valid URI",
+ input: "wasb://container@account.blob.core.windows.net/path/to/file.parquet",
+ expectedKey: "path/to/file.parquet",
+ },
+ {
+ name: "wasbs valid URI",
+ input: "wasbs://container@account.blob.core.windows.net/path/to/file.parquet",
+ expectedKey: "path/to/file.parquet",
+ },
+ {
+ name: "URI with no path",
+ input: "abfs://container@account.dfs.core.windows.net",
+ expectedKey: "",
Review Comment:
Why is this no error?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: Azure URI parsing with `KeyExtractor` function for `blobFileIO` preprocessing [iceberg-go]
Posted by "twuebi (via GitHub)" <gi...@apache.org>.
twuebi commented on code in PR #562:
URL: https://github.com/apache/iceberg-go/pull/562#discussion_r2344241903
##########
io/azure.go:
##########
@@ -184,3 +189,18 @@ func createAzureBucket(ctx context.Context, parsed *url.URL, props map[string]st
return azureblob.OpenBucket(ctx, client, nil)
}
+
+// adlsKeyExtractor creates a key extractor for Azure schemes using the adlsURIPattern pattern
+func adlsKeyExtractor() KeyExtractor {
+ return func(path string) (string, error) {
+ matches := adlsURIPattern.FindStringSubmatch(path)
+ if len(matches) < 4 {
+ // If it doesn't match Azure pattern, return path as-is
+ return "", fmt.Errorf("invalid ADLS location: %s", path)
Review Comment:
The comment and the return statement don't match up, right?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: Azure URI parsing with `KeyExtractor` function for `blobFileIO` preprocessing [iceberg-go]
Posted by "alessandro-nori (via GitHub)" <gi...@apache.org>.
alessandro-nori commented on code in PR #562:
URL: https://github.com/apache/iceberg-go/pull/562#discussion_r2344276340
##########
io/azure.go:
##########
@@ -184,3 +189,18 @@ func createAzureBucket(ctx context.Context, parsed *url.URL, props map[string]st
return azureblob.OpenBucket(ctx, client, nil)
}
+
+// adlsKeyExtractor creates a key extractor for Azure schemes using the adlsURIPattern pattern
+func adlsKeyExtractor() KeyExtractor {
+ return func(path string) (string, error) {
+ matches := adlsURIPattern.FindStringSubmatch(path)
+ if len(matches) < 4 {
+ // If it doesn't match Azure pattern, return path as-is
+ return "", fmt.Errorf("invalid ADLS location: %s", path)
Review Comment:
🙇 right, I changed the code but forgot to update the comment
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org