Skip to content

Commit e7684ee

Browse files
MisterRaindropclaudehappy-otter
committed
fix: resolve CI failures for S3 endpoint and template deduction
1. ConfigureS3Options now falls back to AWS_ENDPOINT_URL_S3 and AWS_ENDPOINT_URL environment variables when the endpoint is not specified in properties, matching Arrow's S3Options::FromUri behavior. This fixes macOS CI where MinIO endpoint was not being picked up. 2. Wrap string literals with std::string() in rest_catalog_integration_test to fix template argument deduction failure in ConfigBase::Set() on GCC. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
1 parent a27d611 commit e7684ee

2 files changed

Lines changed: 15 additions & 3 deletions

File tree

src/iceberg/arrow/arrow_s3_file_io.cc

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
* under the License.
1818
*/
1919

20+
#include <cstdlib>
2021
#include <mutex>
2122
#include <stdexcept>
2223

@@ -93,6 +94,17 @@ Result<::arrow::fs::S3Options> ConfigureS3Options(
9394
auto endpoint_it = properties.find(S3Properties::kEndpoint);
9495
if (endpoint_it != properties.end()) {
9596
options.endpoint_override = endpoint_it->second;
97+
} else {
98+
// Fall back to AWS standard environment variables for endpoint override
99+
const char* s3_endpoint_env = std::getenv("AWS_ENDPOINT_URL_S3");
100+
if (s3_endpoint_env != nullptr) {
101+
options.endpoint_override = s3_endpoint_env;
102+
} else {
103+
const char* endpoint_env = std::getenv("AWS_ENDPOINT_URL");
104+
if (endpoint_env != nullptr) {
105+
options.endpoint_override = endpoint_env;
106+
}
107+
}
96108
}
97109

98110
auto path_style_it = properties.find(S3Properties::kPathStyleAccess);

src/iceberg/test/rest_catalog_integration_test.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -497,7 +497,7 @@ TEST_F(RestCatalogIntegrationTest, MakeWithUnregisteredIoImplReturnsError) {
497497
auto config = RestCatalogProperties::default_properties();
498498
config.Set(RestCatalogProperties::kUri, CatalogUri())
499499
.Set(RestCatalogProperties::kName, std::string(kCatalogName))
500-
.Set(RestCatalogProperties::kWarehouse, "/local/warehouse");
500+
.Set(RestCatalogProperties::kWarehouse, std::string("/local/warehouse"));
501501
config.mutable_configs()[FileIOProperties::kImpl] = "com.nonexistent.FileIO";
502502

503503
auto result = RestCatalog::Make(config);
@@ -519,7 +519,7 @@ TEST_F(RestCatalogIntegrationTest, MakeWithAutoDetectedLocalFileIO) {
519519
auto config = RestCatalogProperties::default_properties();
520520
config.Set(RestCatalogProperties::kUri, CatalogUri())
521521
.Set(RestCatalogProperties::kName, std::string(kCatalogName))
522-
.Set(RestCatalogProperties::kWarehouse, "/local/warehouse");
522+
.Set(RestCatalogProperties::kWarehouse, std::string("/local/warehouse"));
523523

524524
auto catalog_result = RestCatalog::Make(config);
525525
ASSERT_THAT(catalog_result, IsOk());
@@ -541,7 +541,7 @@ TEST_F(RestCatalogIntegrationTest, MakeWithCustomIoImpl) {
541541
auto config = RestCatalogProperties::default_properties();
542542
config.Set(RestCatalogProperties::kUri, CatalogUri())
543543
.Set(RestCatalogProperties::kName, std::string(kCatalogName))
544-
.Set(RestCatalogProperties::kWarehouse, "/any/warehouse");
544+
.Set(RestCatalogProperties::kWarehouse, std::string("/any/warehouse"));
545545
config.mutable_configs()[FileIOProperties::kImpl] = custom_impl;
546546

547547
auto catalog_result = RestCatalog::Make(config);

0 commit comments

Comments
 (0)