fix(rest): resolve a FileIO without io-impl or warehouse configured#13
Open
smaheshwar-pltr wants to merge 1 commit into
Open
fix(rest): resolve a FileIO without io-impl or warehouse configured#13smaheshwar-pltr wants to merge 1 commit into
smaheshwar-pltr wants to merge 1 commit into
Conversation
smaheshwar-pltr
commented
Jun 27, 2026
| Result<std::shared_ptr<FileIO>> RestCatalog::TableFileIO( | ||
| const SessionContext& /*context*/, | ||
| const std::unordered_map<std::string, std::string>& table_config, | ||
| std::string_view metadata_location, |
Owner
Author
There was a problem hiding this comment.
Why do we even need this change for this PR? Doesn't the change of MakeCatalogFileIO not throwing solve our use case alone and it'd keep the PR smaller? Also, look for comments on the original upstream PR to see if there are any notes about our scenario at all
MakeCatalogFileIO and MakeTableFileIO previously returned InvalidArgument when neither "io-impl" nor "warehouse" was configured, preventing catalogs that rely entirely on vended storage credentials (no client-side IO config) from being created or loading tables. Now: - MakeCatalogFileIO falls back to a local FileIO so the catalog can still be created; per-table resolution supplies remote access where needed. - MakeTableFileIO resolves the implementation in priority order: io-impl, warehouse scheme, then the table's metadata-location scheme, falling back to a local FileIO. The metadata location is threaded through TableFileIO so the scheme comes from the table's actual location rather than a storage-credential prefix (which may be scheme-only, e.g. "s3"). Updates the catalog test for the new fallback and adds table-FileIO tests for metadata-location detection (with a scheme-only credential prefix) and the local fallback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
smaheshwar-pltr
force-pushed
the
rest-fileio-fallback
branch
from
005c648 to
3849099
Compare
smaheshwar-pltr
commented
Jun 27, 2026
| // from the table's metadata location so catalogs that rely solely on | ||
| // vended credentials can resolve a table FileIO. | ||
| ICEBERG_ASSIGN_OR_RAISE(const auto detected_kind, | ||
| DetectBuiltinFileIO(metadata_location)); |
Owner
Author
There was a problem hiding this comment.
Resolving the table FileIO from the location scheme (rather than requiring a client warehouse/io-impl) mirrors Java and PyIceberg:
- Java
ResolvingFileIO(scheme→impl map + fallback): https://github.com/apache/iceberg/blob/874e4096e5d16fddbbf43b91f20e248616232cc3/core/src/main/java/org/apache/iceberg/io/ResolvingFileIO.java#L52-L69 - PyIceberg
load_file_io/_infer_file_io_from_scheme: https://github.com/apache/iceberg-python/blob/01a86f987d1924ca3246a2b985b78890ec0c0fbd/pyiceberg/io/__init__.py#L349 , https://github.com/apache/iceberg-python/blob/01a86f987d1924ca3246a2b985b78890ec0c0fbd/pyiceberg/io/__init__.py#L337
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Resolve a REST catalog
FileIOwhen neitherio-implnorwarehouseis configured:MakeCatalogFileIOfalls back to a localFileIO(instead of returningInvalidArgument).MakeTableFileIOresolves the implementation in orderio-impl>warehousescheme > the table'smetadata-locationscheme, else local. The metadata location is threaded throughTableFileIO.Why
A catalog that relies on vended credentials carries no client-side
io-impl/warehouse, so both factories previously failed and such a catalog could be neither created nor used. This mirrors how Java and PyIceberg resolve a FileIO from the location scheme (with a graceful fallback) rather than requiring a warehouse:ResolvingFileIO(scheme→impl map + fallback): https://github.com/apache/iceberg/blob/874e4096e5d16fddbbf43b91f20e248616232cc3/core/src/main/java/org/apache/iceberg/io/ResolvingFileIO.java#L52-L69load_file_io/_infer_file_io_from_scheme: https://github.com/apache/iceberg-python/blob/01a86f987d1924ca3246a2b985b78890ec0c0fbd/pyiceberg/io/__init__.py#L349 , https://github.com/apache/iceberg-python/blob/01a86f987d1924ca3246a2b985b78890ec0c0fbd/pyiceberg/io/__init__.py#L337Why one PR (not two)
The two changes are one capability and are non-functional apart: with only the
MakeCatalogFileIOfallback a vended-table load still throws inMakeTableFileIO; and themetadata-locationbranch is unreachable without the catalog fallback (a configured catalog's merged config always carriesio-impl/warehouse). Splitting would ship dead code.Notes / follow-ups
ResolvingFileIOfallback, an unconfigured remote catalog now degrades to a local FileIO rather than failing fast, so a genuine misconfiguration surfaces later. Deliberate, for parity.