zeroshade commented on code in PR #1452:
URL: https://github.com/apache/iceberg-go/pull/1452#discussion_r3566743731
##########
cmd/iceberg/clean_orphan_files.go:
##########
@@ -40,50 +40,46 @@ func runCleanOrphanFiles(ctx context.Context, output
Output, cat catalog.Catalog
opts := []table.OrphanCleanupOption{
table.WithFilesOlderThan(olderThan),
- table.WithDryRun(true),
}
if cmd.Location != "" {
opts = append(opts, table.WithLocation(cmd.Location))
}
- result, err := tbl.DeleteOrphanFiles(ctx, opts...)
+ plan, err := tbl.PlanOrphanFiles(ctx, opts...)
if err != nil {
output.Error(fmt.Errorf("orphan file scan failed: %w", err))
os.Exit(1)
}
- cliResult := buildCleanOrphanFilesResult(tbl, result, cmd.DryRun)
+ result := table.OrphanCleanupResult{
+ OrphanFileLocations: plan.Files(),
+ TotalSizeBytes: plan.TotalSizeBytes(),
+ }
+ cliResult := buildCleanOrphanFilesResult(tbl, result, true)
Review Comment:
Non-blocking: this hardcodes `dryRun=true` for the pre-confirmation result,
so a real (non-dry-run) run that finds zero orphans still reports `"dry_run":
true` in JSON. Consider using `cmd.DryRun` for the final/no-op result.
##########
cmd/iceberg/clean_orphan_files.go:
##########
@@ -40,50 +40,46 @@ func runCleanOrphanFiles(ctx context.Context, output
Output, cat catalog.Catalog
opts := []table.OrphanCleanupOption{
table.WithFilesOlderThan(olderThan),
- table.WithDryRun(true),
}
if cmd.Location != "" {
opts = append(opts, table.WithLocation(cmd.Location))
}
- result, err := tbl.DeleteOrphanFiles(ctx, opts...)
+ plan, err := tbl.PlanOrphanFiles(ctx, opts...)
if err != nil {
output.Error(fmt.Errorf("orphan file scan failed: %w", err))
os.Exit(1)
}
- cliResult := buildCleanOrphanFilesResult(tbl, result, cmd.DryRun)
+ result := table.OrphanCleanupResult{
+ OrphanFileLocations: plan.Files(),
+ TotalSizeBytes: plan.TotalSizeBytes(),
+ }
+ cliResult := buildCleanOrphanFilesResult(tbl, result, true)
if cmd.DryRun {
output.CleanOrphanFilesResult(cliResult)
return
}
- if len(result.OrphanFileLocations) == 0 {
+ if len(plan.Files()) == 0 {
output.CleanOrphanFilesResult(cliResult)
return
}
+ output.CleanOrphanFilesResult(cliResult)
Review Comment:
Non-blocking: with `--output json`, emitting this preview and then the
post-deletion result later produces two top-level JSON objects on stdout for a
single invocation (and an aborted confirmation leaves a success-looking dry-run
object behind). Consider keeping the pre-confirmation preview out of the
structured output, or emitting a single JSON result at the end.
--
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]
