zhangfengcdt commented on PR #5935: URL: https://github.com/apache/datafusion-comet/pull/5935#issuecomment-5688061471
> The added fallback examples match the current gate at `251fb7d4`. My [existing approval](https://github.com/apache/datafusion-comet/pull/5935#pullrequestreview-5214954348) stands. The source is unchanged, and the resolved fixture P2 remains fixed. > > Please align two description details with the reviewed behavior: the helper invokes `io(location)` to inspect the instantiated delegate, and the resolver tests exercise delegate resolution and write planning. On pre-1.10 runtimes, their passing result alone does not prove that `GCSFileIO.initialize` ran. The prior 1.5.2/1.8.1 component controls lacked Google authentication classes, so that direct initialization coverage remains unverified. > > [Required Checks](https://github.com/apache/datafusion-comet/actions/runs/35008700813/job/104548144254) is now green: 53 successful checks and 12 skips. The completed Spark 3.4/3.5 scan jobs also passed the focused GCS cases on merge `ab0aa7bd`, whose full tree matches this head. No unchanged broad suites were rerun locally, and no native GCS write is claimed. I have updated the following to align with the reviewed behavior. 2. `IcebergReflection`: a helper that returns the effective `FileIO` class for a location. For a `ResolvingFileIO` it invokes `io(location)` and inspects the delegate Iceberg actually instantiates, including the fallback to `HadoopFileIO` when the scheme's `FileIO` cannot be loaded or initialized, so a table that resolves to `GCSFileIO` stays eligible. Reflection failures fail closed. AND - Two end-to-end tests of `ResolvingFileIO` delegate resolution and write planning. One uses an unparseable `gcs.channel.read.chunk-size-bytes`, which fails `GCSFileIO.initialize` on Iceberg versions before 1.10 and is ignored at initialization on 1.10+; the expectation is version-aware. In CI the pre-1.10 profiles lack the Google authentication classes, so `GCSFileIO` never constructs there and the test passes through the class-load fallback. Direct coverage of the initialization-failure path therefore remains unverified in CI; it holds by source inspection of 1.5.2 and 1.8.1. -- 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]
