zeroshade commented on code in PR #1781:
URL: https://github.com/apache/iceberg-go/pull/1781#discussion_r3832668932


##########
table/scanner.go:
##########
@@ -900,14 +924,23 @@ func (scan *Scan) planFilesLocal(ctx context.Context, acc 
*scanMetricsAccumulato
                }
        }()
 
+       snap, err := scan.ResolveSnapshot()
+       if err != nil || snap == nil {
+               return nil, err
+       }
+       fs, err := scan.ioF(ctx)

Review Comment:
   Retaining this concrete FileIO for the entire plan bypasses later factory 
calls that may refresh credentials. REST tables wire 
`vendedCredentialRefresher.loadFS` as `ioF`, and each invocation checks 
`needsRenewal()` before returning an IO. A deterministic expiring-credential 
factory probe passes on `main`—later manifest workers reacquire FileIO—but 
fails on this branch with `credentials expired before open`. Long-running plans 
that cross the renewal window can therefore fail with storage authorization 
errors. Please preserve credential-renewal checkpoints while avoiding repeated 
backend construction, and add coverage for expiry during planning.



-- 
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]

Reply via email to