mixermt commented on PR #6106: URL: https://github.com/apache/datafusion-comet/pull/6106#issuecomment-5773977478
Second review round, three findings, all addressed in the latest push: 1. **A read-side provider init failure became sticky.** `build_s3_credential_loader` falls back to opendal's default chain on a read, and that `FileIO` was cached like a success, so later tasks never retried `ensureInitialized`. `storage_factory_for` and `build_file_io` now report whether the build is cacheable; a fallback is returned but not inserted, so the next task retries. Covered by `cached_file_io_does_not_cache_a_degraded_build`. 2. **The S3 operator cache follow-up lived only in #6105, which this PR closes.** Split into #6109 and linked from the description. 3. **Two comments still described per-task lifetimes.** The `FILE_IO_CACHE` rustdoc now says what is shared at this pin, and the missing-expiry latch comment in `credential_bridge.rs` reflects that bridges live as long as their cache entry. -- 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]
