Revanth14 commented on code in PR #1286:
URL: https://github.com/apache/iceberg-go/pull/1286#discussion_r3464733370
##########
table/orphan_cleanup.go:
##########
@@ -424,64 +422,7 @@ func walkDirectory(fsys iceio.IO, root string, fn
func(path string, info stdfs.F
})
}
- // Fallback to original implementation for IO types that don't
- // implement ListableIO yet.
- switch v := fsys.(type) {
- case iceio.LocalFS:
- cleanRoot := strings.TrimPrefix(root, "file://")
- if cleanRoot == "" {
- cleanRoot = "."
- }
-
- return filepath.WalkDir(cleanRoot, makeFileWalkFunc(fn,
func(path string) string {
- return path
- }))
-
- default:
- // For blob storage: direct field access since we know the
structure.
- bucket := getBucketName(v)
-
- parsed, err := url.Parse(root)
- if err != nil {
- return fmt.Errorf("invalid URL %s: %w", root, err)
- }
-
- walkPath := strings.TrimPrefix(parsed.Path, "/")
- if walkPath == "" {
- walkPath = "."
- }
-
- return stdfs.WalkDir(bucket, walkPath, makeFileWalkFunc(fn,
func(path string) string {
- return parsed.Scheme + "://" + parsed.Host + "/" + path
- }))
- }
-}
-
-// getBucketName gets the Bucket field from blob storage - absolute minimal
approach.
-func getBucketName(fsys iceio.IO) stdfs.FS {
- v := reflect.ValueOf(fsys).Elem()
-
- return v.FieldByName("Bucket").Interface().(stdfs.FS)
-}
-
-// makeFileWalkFunc creates a WalkDirFunc that processes only files with path
transformation.
-func makeFileWalkFunc(fn func(path string, info stdfs.FileInfo) error,
pathTransform func(string) string) stdfs.WalkDirFunc {
- return func(path string, d stdfs.DirEntry, err error) error {
- if err != nil {
- return err
- }
-
- if d.IsDir() {
- return nil
- }
-
- info, err := d.Info()
- if err != nil {
- return err
- }
-
- return fn(pathTransform(path), info)
- }
+ return fmt.Errorf("filesystem %T does not implement io.ListableIO",
fsys)
Review Comment:
Thanks, agreed. `PurgeFiles` is best-effort so skipping a non-listable FS
makes sense there, but orphan cleanup needs to know whether it fully walked the
location before deciding what is orphaned. Keeping this as a hard error feels
safer.
--
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]
