Revanth14 commented on code in PR #1286:
URL: https://github.com/apache/iceberg-go/pull/1286#discussion_r3470195271
##########
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:
Added godoc on `DeleteOrphanFiles` noting that the table filesystem must
implement `iceio.ListableIO`.
I left `PurgeFiles` behavior unchanged here: purge is best-effort after
catalog drop, while orphan cleanup needs complete location enumeration before
it can safely decide what is orphaned, so the hard failure is intentional.
--
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]
