You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "mhambre (via GitHub)" <gi...@apache.org> on 2026/08/23 20:25:58 UTC
[PR] chore: HF move from OnceCell to LazyCell [opendal]
mhambre opened a new pull request, #8133:
URL: https://github.com/apache/opendal/pull/8133
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
Closes #.
# Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
As requested in this thread https://github.com/apache/opendal/pull/8108#pullrequestreview-4969315994 a `LazyCell` was implemented into `asyncband` to reduce initializer duplication.
# What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
Moves all `OnceCell` usages to `LazyCell` for the HuggingFace service, and reduces the `HfCore` struct back to storing a singular repo stored behind a `LazyCell` rather than having a canonical one and a non-canonical one.
# Are there any user-facing changes?
N/A
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!---
If there are any breaking changes to public APIs, please add the `breaking-changes` label.
-->
# AI Usage Statement
GPT 5.6 sol used to draft out requested implementation. Subsequent human back and forth, review, and fixes cleaned up the API to be more aligned with maintainer requests.
<!--
If AI materially assisted this PR, briefly describe its role and disclose any
assumptions or unknowns that affect review. Do not include routine command output
or repeat information provided elsewhere.
The author remains responsible for every submitted claim and change. If AI did
not materially assist this PR, write "None".
-->
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: HF move from OnceCell to LazyCell [opendal]
Posted by "erickguan (via GitHub)" <gi...@apache.org>.
erickguan commented on PR #8133:
URL: https://github.com/apache/opendal/pull/8133#issuecomment-5403888655
Kudos to you, Matthew!
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: HF move from OnceCell to LazyCell [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #8133:
URL: https://github.com/apache/opendal/pull/8133#issuecomment-5421766826
I'll refactor this on my own.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: HF move from OnceCell to LazyCell [opendal]
Posted by "erickguan (via GitHub)" <gi...@apache.org>.
erickguan commented on PR #8133:
URL: https://github.com/apache/opendal/pull/8133#issuecomment-5421303319
Thanks @tisonkun and @mhambre for prototyping `LazyCell`. We can pause here on more design. I skimmed depending `xet` - it accepts repo_id and gets a JWT token but it doesn't seem to have canonical repo_id logic. It will be easier if xet could provide a canonical repo ID instead.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: HF move from OnceCell to LazyCell [opendal]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #8133:
URL: https://github.com/apache/opendal/pull/8133#issuecomment-5420484905
After prototyping both directions against the redesigned Asyncband `LazyCell`, I think the choice here is primarily semantic rather than stylistic.
**Using `LazyCell`:**
- The initializer is stored when `HfCore` is constructed, so call sites only need to force the value.
- It represents one initialization attempt. If the forcing task is cancelled, a later caller resumes the same pinned future.
- This works best when initialization is infallible (or when a `Result` should be cached) and all inputs are already owned at construction time.
- For this path, however, we would need to capture a construction-time HTTP transport/context or expand `LazyCell` with access-time arguments and retry semantics. The former can ignore per-operation context overrides; the latter effectively turns it back into `OnceCell::get_or_try_init` with extra machinery.
**Using `OnceCell`:**
- `get_or_try_init` receives the initializer at access time, so canonicalization uses the current operation's `OperationContext`.
- An HTTP failure leaves the cell uninitialized. A later operation can retry with its own, potentially different transport/context.
- Cancellation restarts the attempt instead of retaining it, but canonicalization is an idempotent GET, so that appears to be the safer and simpler behavior here.
- Keeping the closure inside `canonical_repo` also means callers do not duplicate initialization logic; the original ergonomics concern is already localized.
The current branch now follows the `OnceCell` direction and includes a deterministic test where the first context fails and the next context succeeds, proving that retry uses the latest context.
My current preference is therefore to keep `OnceCell` for Hugging Face canonicalization, while keeping `LazyCell` for genuinely construction-time, single-attempt initialization. Do reviewers agree with that boundary, or do we want `HfCore` to own a fixed initialization context and use `LazyCell` instead? If we agree on `OnceCell`, we should reframe the PR title/description around the context-aware retry behavior rather than a `LazyCell` migration.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: HF move from OnceCell to LazyCell [opendal]
Posted by "mhambre (via GitHub)" <gi...@apache.org>.
mhambre commented on PR #8133:
URL: https://github.com/apache/opendal/pull/8133#issuecomment-5388291201
Note that this is a draft implementation proving the efficacy of a pending asyncband `LazyLock` implementation and will be moved from the forked version to asyncband prior to being moved out of draft.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore: HF move from OnceCell to LazyCell [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo closed pull request #8133: chore: HF move from OnceCell to LazyCell
URL: https://github.com/apache/opendal/pull/8133
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org