manuzhang opened a new pull request, #951: URL: https://github.com/apache/iceberg-cpp/pull/951
## Summary There was no shared way to parse a boolean: `StringUtils` covered only numbers (`ParseNumber<T>` even excludes `bool`), so four call sites each spelled out the `"true"`/`"false"` literals themselves, in two incompatible flavors: | Site | Behavior on an unrecognized value | | --- | --- | | `arrow_s3_file_io.cc` (`ParseOptionalBool`) | error | | `util/config.h` (`DefaultFromString<bool>`) | false | | `auth/auth_managers.cc` (`rest.sigv4-enabled`) | false | | `rest_catalog.cc` (`rest-metrics-reporting-enabled`) | false | This adds the missing primitives and routes every site through them: - `StringUtils::ParseBoolean(std::string_view) -> Result<bool>` — strict and case-insensitive, mirroring `ParseNumber`. Callers that must not fail use `.value_or(false)`. - `PropertyUtil::PropertyAsBoolean(properties, key, default_value)` and `PropertyUtil::PropertyAsOptionalBoolean(properties, key)` for the property-map pattern, the latter distinguishing an unset property from an explicit `"false"`. `rest-metrics-reporting-enabled` was declared as `Entry<std::string>` and compared by hand; it is now an `Entry<bool>` like every other boolean property in the codebase, so `ConfigBase::Get` does the conversion. ## Behavior No intended change. The lenient sites stay lenient (`.value_or(false)`), which `ConfigTest.ParseBooleanIgnoringCase` pins; `arrow_s3_file_io.cc` stays strict, with the error message now naming the offending value as well as the key. Not touched: `avro_schema_util.cc` compares an Avro node attribute (`adjust-to-utc`) rather than a user-supplied property, so its case-sensitive comparison is left alone. ## Tests - `StringUtilsTest.ParseBoolean` — accepted spellings, rejected values, error message. - New `property_util_test.cc` — defaulting when absent, case-insensitivity, rejection of non-boolean values, and the key/value in the error. `pre-commit` could not be run in this environment (no PyPI access to install it or `clang-format`); formatting was matched to `.clang-format` (Google, 90 columns) by hand, so please let CI have the final word on it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
