fix(cloud): parse OpenDAL boolean properties the way object_store does - #9782
Open
jackylee-ch wants to merge 2 commits into
Open
fix(cloud): parse OpenDAL boolean properties the way object_store does#9782jackylee-ch wants to merge 2 commits into
jackylee-ch wants to merge 2 commits into
Conversation
`cos` and `oss` compared the raw property string against `"true"`, so `True`, `1` and `yes` — all accepted by `object_store` for its own configuration, and by the `allow_http` sibling in this crate — read as `false` without a warning. Add a shared `property_as_bool` next to `property_or_env` that trims, lowercases and matches `object_store`'s set, and warns on a value that is not a boolean instead of silently answering `false`. Signed-off-by: jackylee-ch <qcsd2011@gmail.com>
robert3005
reviewed
Sep 9, 2026
| #[case("True")] | ||
| #[case("1")] | ||
| #[case("yes")] | ||
| fn cos_reads_disable_config_load_beyond_lowercase_true(#[case] value: &str) { |
Contributor
There was a problem hiding this comment.
I think it's enough to only test property_as_bool not this and oss methods
Contributor
Author
There was a problem hiding this comment.
Dropped both — property_as_bool covers the parsing, and the call sites are a one-line change each. 101 passed, 84 on develop.
The `property_as_bool` cases cover the parsing; the cos and oss tests only asserted that each call site reaches it, which is a one-line change per service. Signed-off-by: jackylee-ch <qcsd2011@gmail.com>
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
2 participants
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.
cos.rsandoss.rscompared the property string against"true"exactly, soTrue,1andyesread asfalse.object_storeaccepts all of those for its own configuration(
config.rs,impl Parse for bool:1/true/on/yes/y, lowercased), and so does theallow_httpsibling inhf/mod.rs— only the two OpenDAL services did not.Values reach
make_opendal_storeverbatim from the caller's property map, andwarn_on_unknown_propertiesonly warns about unknown keys, so an unusable value was silent:skip_signature = "True"keeps signing a public bucket (403), anddisable_config_load = "True"leaves OpenDAL loading the ambient credentials the caller asked to turn off.
The new shared
property_as_booltrims, lowercases, matchesobject_store's set, and warnsinstead of answering
false.goosefshas no boolean property.Tests
cargo test --release -p vortex-cloud --features cos,oss,goosefs,hf,registry: 101 passed, 84 ondevelop; feature matrix checked one flag at a time. Restoring the exact"true"compare fails7 of the new cases.
AI assistance
Written with agentic AI assistance; I read
object_store'sParse for boolin the vendored0.13.2 source before matching it.