Skip to content

fix(cloud): parse HF ALLOW_HTTP the way object_store does - #10278

Open
jackylee-ch wants to merge 1 commit into
vortex-data:developfrom
jackylee-ch:fix/hf-allow-http-bool
Open

jackylee-ch wants to merge 1 commit into
vortex-data:developfrom
jackylee-ch:fix/hf-allow-http-bool

Conversation

@jackylee-ch

Copy link
Copy Markdown
Contributor

The Hugging Face store read ALLOW_HTTP as true only for true/1 (hf/mod.rs:390),
while object_store itself — and vortex-cloud's own property_as_bool for the OpenDAL stores
(opendal/mod.rs:213) — accept 1/true/on/yes/y case-insensitively. So ALLOW_HTTP=on,
which works for every other scheme, left HF requiring HTTPS and a plain-HTTP Hub endpoint
unreachable. #9782 aligned the OpenDAL side to object_store but left HF behind.

Fix

Pull the object_store boolean spellings into one parse_object_store_bool helper and route both
allow_http and property_as_bool through it, so the two cannot drift. property_as_bool's
behaviour (including the warn-on-non-boolean) is unchanged.

Tests

Adds HF cases in hf/tests.rs for the previously-rejected spellings; the on/yes/y cases
fail before the change and pass after. cargo test -p vortex-cloud --features hf, clippy, and
cargo +nightly fmt --check are clean.

AI assistance

Written with agentic AI assistance; I read all three boolean-parsing sites and reproduced the
on/yes rejection before and after.

The Hugging Face store read `ALLOW_HTTP` as true only for `true`/`1`, while
object_store itself — and vortex-cloud's own `property_as_bool` for the OpenDAL
stores — accept `1`/`true`/`on`/`yes`/`y`. So `ALLOW_HTTP=on`, which works for
every other scheme, left HF requiring HTTPS and a plain-HTTP Hub endpoint
unreachable.

Pull the object_store boolean spellings into one `parse_object_store_bool`
helper and route both `allow_http` and `property_as_bool` through it, so the two
cannot drift. Adds HF cases for the previously-rejected spellings.

Signed-off-by: jackylee-ch <qcsd2011@gmail.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant