Clean-up huggingface_hub integration#263
Conversation
| return repo_id.split("/")[-1].replace("-", "_") | ||
|
|
||
|
|
||
| def _get_user_agent( |
There was a problem hiding this comment.
IMO, it wouldn't hurt to keep this function and use it inside _get_hf_api(). I think that's more idiomatic and structured.
|
|
||
| return user_agent | ||
| return HfApi( | ||
| library_name="kernels", library_version=__version__, user_agent=user_agent_str |
| if isinstance(user_agent, dict): | ||
| user_agent.update( | ||
| { | ||
| "kernels": __version__, | ||
| "python": python, | ||
| "torch": torch.__version__, | ||
| "build_variant": build_variant(), | ||
| "file_type": "kernel", | ||
| } | ||
| ) |
There was a problem hiding this comment.
Hopefully, it doesn't break anything. Cc: @MekkCyber do you reckon anything to be breaking for this?
There was a problem hiding this comment.
my goal was to deduplicate logic (the user agent was defined both in a dictionary format and a string format, now we only define it once)
There was a problem hiding this comment.
good for me since we still convert the dict to str so not breaking
| if isinstance(user_agent, dict): | ||
| user_agent.update( | ||
| { | ||
| "kernels": __version__, | ||
| "python": python, | ||
| "torch": torch.__version__, | ||
| "build_variant": build_variant(), | ||
| "file_type": "kernel", | ||
| } | ||
| ) |
There was a problem hiding this comment.
good for me since we still convert the dict to str so not breaking
|
|
||
| return user_agent | ||
| return HfApi( | ||
| library_name="kernels", library_version=__version__, user_agent=user_agent_str |
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
|
@sayakpaul @MekkCyber thanks for the reviews. I've solved the merged conflict + fixed the CI. Can I let you do a last check and merge? |
|
Resolved another merge conflict. @sayakpaul once CI is green could you merge so that we don't get new conflicts. I'm not maintainer on this repo so can't do it myself :) |
|
@sayakpaul @danieldk I had to resolved conflicts due to this refactor PR #269 + fixed some formatting corrections I pushed by mistake but it should now be good to merge again. I'd really prefer not to have to do it again 🙏 |
|
closing as #274 was merged. Thanks again! |
5 participants
This PR cleans up a bit the integration with
huggingface_hub. In particular, it defines a_get_hf_apihelper that instantiate anHfApiclient with the correct user agent set. The goal is to have consistent user agent across all calls made to the Hub.List of changes:
kernels.utils._get_hf_apito return aHfApiinstancekernels. No morefrom huggingface_hub import create_repo, snapshot_download, etc.anymore_user_agentshelper (not used anymore)huggingface_hub.constants.HF_HUB_DISABLE_TELEMETRYinstead ofos.getenv("DISABLE_TELEMETRY"). The hfh constant is more robust (e.g. handlesDO_NOT_TRACKenv variable)requestsfrom benchmark dependencies. Useget_session,build_hf_headersandhf_raise_for_statusinstead. This should get you better logs in case of errors and relies either onrequestsorhttpxdepending on what's available. It also handles token retrieval by default.Overall
kernelsbehavior and API remains exactly the same. Changes are meant to be purely internal.I am sorry in advance for all the style changes. I've run
make stylelocally and inadvertently pushed more changes than what I thought. Let me know if you want me to try to revert them.