hf-hub icon indicating copy to clipboard operation
hf-hub copied to clipboard

chore: `ApiBuilder` should support `Default` without a token

Open polarathene opened this issue 9 months ago • 0 comments

When creating an API instance the current methods enforce checking for a local token file, even when you intend to provide your own via with_token(), thus finding a token is redundant, as is the info log of one missing being emitted.

  • from_cache() calls cache.token() to set the token field in the builder.
  • You cannot call with_token() prior, both default() and new() methods use the same initialization. Only workaround is to duplicate from_cache() and omit the token check.
  • The token is later used by the private build_headers() method during the final build() call.

This PR changes:

  • The Default impl for both sync/async structs to provide the same struct value except for the token field, leaving it as None by default. from_cache() will use the "Struct Update" syntax to spread the remaining default field values (this communicates more clearly what the method is actually modifying).
  • The order of fields for the Api and ApiBuilder structs were grouped contextually for better visibility vs interleaved. Handled via a separate commit.

I've tried to minimize affecting anything else, but technically if someone was relying on ApiBuilder::default() before instead of ::new(), the only difference should be that token is not potentially set for them. Anyone who is using ::new() should be unaffected by this change.


Related: https://github.com/huggingface/hf-hub/issues/54#issuecomment-2118679106

polarathene avatar May 19 '24 00:05 polarathene