-
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 api recorder #8247
Fix api recorder #8247
Conversation
🪼 branch checks and previews
Install Gradio from this PR pip install https://gradio-builds.s3.amazonaws.com/abb52a0f0105025a520584dd633508bf0eda6b40/gradio-4.31.3-py3-none-any.whl Install Gradio Python Client from this PR pip install "gradio-client @ git+https://github.com/gradio-app/gradio@abb52a0f0105025a520584dd633508bf0eda6b40#subdirectory=client/python" |
🦄 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.
|
@@ -415,8 +416,11 @@ def main(request: fastapi.Request, user: str = Depends(get_current_user)): | |||
|
|||
@app.get("/info/", dependencies=[Depends(login_check)]) | |||
@app.get("/info", dependencies=[Depends(login_check)]) | |||
def api_info(): | |||
# The api info is set in create_app | |||
def api_info(all_endpoints: bool = False): |
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.
Initially, I had implemented it so that the /info
route just returned all of the endpoints, even those with show_api=False
and we filtered in the frontend. However, this led to very, very large payloads for complex apps like the theme builder causing slow load times / crashing. We should figure out a way to reduce the payload for /info
, but in the meantime, here we don't load all endpoints by default -- only if you use the API Recorder.
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.
nice, LGTM! I tested everything and it all works as expected now.
Ignore the website build issue will fix it on a separate PR as it isn't related to this.
Sweet thanks @aliabd! |
Fixes issue where API Recorder would stop working (the fix was to explicitly handle the case where one of the inputs was
gr.State
). Fixed for both the Python and JS code snippets.Closes: #8259 (test either of the demos locally)
Closes: #7982 (test on this Space with this PR installed)
Also fixes the RecordingSnippet for JS Client to ensure that it uses the new kwarg syntax instead of the positional syntax and adds an e2e -- the playwright api recorder helped me write the test hehe