-
Notifications
You must be signed in to change notification settings - Fork 571
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow passing custom headers to HfApi #2098
Conversation
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for the changes.
repo_id=repo_id, | ||
repo_type=repo_type, | ||
revision=revision, | ||
endpoint=endpoint, | ||
headers=headers, | ||
token=None, # already passed in 'headers' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the token
be generally expected in the headers in that case, rather than put as a kwarg? Or is it a public-facing function and we can't change the API maybe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I removed token
whenever possible but this one is not a private method so just in case I did not remove token
from the params.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(though it's not a documented / promoted method)
Thanks for the review @LysandreJik! I'll merge it as it is |
No, using tokens is the recommended way of handling authentication. |
cc @rafaelpierrehf @philschmid I'll document + add tests for it later but the current implementation should be good as it is now. Here is a short example on how it can be used to authenticate via cookies:
Which resulted in https://huggingface.co/Wauplin/test_with_cookie.
EDIT: failing tests are mostly due to warnings.