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`, covering the chain against
the real registered Arrow implementations on both legs: a local write/read
round-trip with real bytes, and an `oss://` case where any break in
routing/forwarding/prefix handling shows up as the warning, which the test
asserts is absent. Also fixed `add_rest_iceberg_test`, which 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]
