plusplusjiajia commented on code in PR #828:
URL: https://github.com/apache/iceberg-cpp/pull/828#discussion_r3650029438
##########
src/iceberg/test/rest_file_io_test.cc:
##########
@@ -158,16 +103,29 @@ TEST(RestFileIOTest,
MakeCatalogFileIOUnregisteredCustomImplReturnsNotFound) {
EXPECT_THAT(result, IsError(ErrorKind::kNotFound));
}
-TEST(RestFileIOTest, MakeCatalogFileIOSkipsCheckWhenWarehouseAbsent) {
+TEST(RestFileIOTest, TableFileIOBindsCredentialsWithLogicalWarehouseName) {
+ // Regression: credential-vending catalogs often use a logical warehouse name
+ // (bucket ARN / catalog name), not a storage URI; the S3 implementation must
+ // still be resolved per path scheme and receive the vended credentials, even
+ // when non-S3 credentials are vended alongside.
+ captured_storage_credentials.clear();
FileIORegistry::Register(
- std::string(FileIORegistry::kArrowLocalFileIO),
+ std::string(FileIORegistry::kArrowS3FileIO),
[](const std::unordered_map<std::string, std::string>& /*properties*/)
- -> Result<std::unique_ptr<FileIO>> { return
std::make_unique<MockFileIO>(); });
+ -> Result<std::unique_ptr<FileIO>> {
+ return std::make_unique<MockCredentialedFileIO>();
+ });
- auto config = RestCatalogProperties::FromMap(
- {{"io-impl", std::string(FileIORegistry::kArrowLocalFileIO)}});
- auto result = MakeCatalogFileIO(config);
+ std::vector<StorageCredential> credentials = {
+ {.prefix = "oss", .config = {{"k1", "v1"}}},
+ {.prefix = "s3", .config = {{"k2", "v2"}}}};
+ auto result = MakeTableFileIO({{"warehouse", "logical_warehouse_name"}},
+ /*table_config=*/{}, credentials);
ASSERT_THAT(result, IsOk());
+
+ // Reaching a data file routes to the S3 FileIO with the full credential
list.
+
(void)result.value()->NewInputFile("oss://bucket/db/table/data/file.parquet");
Review Comment:
@wgtmac Added `test/rest_arrow_file_io_test.cc`: RegisterAll +
MakeTableFileIO with a logical-name warehouse and an `oss://` credential, then
opening an `oss://` file — any break in routing/forwarding/prefix handling
shows up as the warning, which the test asserts is absent. Needed one build
fix: `add_rest_iceberg_test` declared `USE_BUNDLE` but never used it, so REST
tests couldn't link the bundle.
--
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]
