plusplusjiajia commented on PR #703: URL: https://github.com/apache/iceberg-cpp/pull/703#issuecomment-4665698013
> Thanks for @plusplusjiajia contribution. There are a few minor issues with this PR. I agree that the allowlist in DetectBuiltinFileIO covers the initial catalog-level FileIO selection from warehouse/io-impl. > > My remaining concern is that later metadata/manifest/delete locations may reach the already-created FileIO directly, so they are not necessarily guarded by DetectBuiltinFileIO. That said, I also agree that ResolvePath is not the right place to implement global scheme routing or rejection. > > For the scope of this PR, this looks acceptable to me. I think we should follow up with a separate issue to discuss a higher-level resolving FileIO, similar to Java's ResolvingFileIO, for per-location scheme dispatch/enforcement. Thanks @MisterRaindrop! Agreed on both. I'll open a follow-up issue to track it. Thanks for the careful review! -- 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]
