-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Fix zero gpu auth ignoring hf_token #8323
Conversation
🪼 branch checks and previews
Install Gradio from this PR pip install https://gradio-builds.s3.amazonaws.com/ee05c5e1bb523eae5c8483c0afe00c872d148688/gradio-4.31.4-py3-none-any.whl Install Gradio Python Client from this PR pip install "gradio-client @ git+https://github.com/gradio-app/gradio@ee05c5e1bb523eae5c8483c0afe00c872d148688#subdirectory=client/python" |
Nice catch @Saghen! One point though: isn't the Otherwise, totally agreed that we should make the code Node compatible so checking the window object seems reasonnable. @abidlabs do you confirm that the JS client should target NodeJS ? Shouldn't the CI somehow check the compatibility with Node then ? |
🦄 change detectedThis Pull Request includes changes to the following packages.
With the following changelog entry.
Maintainers or the PR author can modify the PR title to modify this entry.
|
Let me tag @hannahblair who can review this PR better than me |
Yep, thanks! Removed that logic |
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.
Beautiful, thank you @Saghen!
The client should (and will eventually) work on all platforms, not just node + browser but also cf workers, service workers, deno, etc.
Yes, and it kind of does but we don't have any tests for zero gpu atm. |
@pngwn I was talking about static (typing) tests (they should catch this "window is possibly undefined" thing I think) |
Since it has to work in both environments it isn't an error to reference the window as sometimes that is present. We don't run type checks twice (once for each environment). |
Since we want both browser and NodeJS compatibility, to me it is actually an error to not check for existence before using the window object (and I think that typescript can be configured to handle this and make projects both browser and NodeJS compatible but I'm not 100% sure) |
Description
When sending requests to a ZeroGPU endpoint via the JS client on node, the client would attempt to index
window
while getting credentials, causing an error. The logic has been changed to prefer thehf_token
, falling back to thepost_message
if thewindow
is available, and finally falling back tonull
.I have not included an issue due to the size of the PR, as per the PR template, but will add if needed!