Skip to content
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: Added support for show_api in mount_gradio_app #10097

Merged
merged 12 commits into from
Dec 8, 2024

Conversation

HongweiRuan
Copy link
Contributor

Fix: Added support for show_api in mount_gradio_app

Description

The mount_gradio_app function did not previously support the show_api parameter, which caused the "Use via API" button to always appear, regardless of the user's intent.

Changes

  • Added logic to handle show_api in mount_gradio_app.
  • Ensured show_api is correctly passed to the Blocks instance.

Closes: #9980

@HongweiRuan
Copy link
Contributor Author

Hi @freddyaboulton , could you please approve the workflows for this PR? Thanks!

@abidlabs
Copy link
Member

abidlabs commented Dec 3, 2024

Thanks for the contribution @HongweiRuan! The app_kwargs parameter is meant for "additional keyword arguments to pass to the underlying FastAPI app" -- since show_api is actually an attribute of Blocks, not FastAPI, I would actually add it as a separate argument in mount_gradio_app

@gradio-pr-bot
Copy link
Collaborator

gradio-pr-bot commented Dec 3, 2024

🪼 branch checks and previews

Name Status URL
Spaces ready! Spaces preview
Website ready! Website preview
🦄 Changes detected! Details

Install Gradio from this PR

pip install https://gradio-pypi-previews.s3.amazonaws.com/c0dfe5fd58b0e3dbad416e6552ef35e11d7ece52/gradio-5.8.0-py3-none-any.whl

Install Gradio Python Client from this PR

pip install "gradio-client @ git+https://github.com/gradio-app/gradio@c0dfe5fd58b0e3dbad416e6552ef35e11d7ece52#subdirectory=client/python"

Install Gradio JS Client from this PR

npm install https://gradio-npm-previews.s3.amazonaws.com/c0dfe5fd58b0e3dbad416e6552ef35e11d7ece52/gradio-client-1.8.0.tgz

Use Lite from this PR

<script type="module" src="https://gradio-lite-previews.s3.amazonaws.com/c0dfe5fd58b0e3dbad416e6552ef35e11d7ece52/dist/lite.js""></script>

@gradio-pr-bot
Copy link
Collaborator

gradio-pr-bot commented Dec 3, 2024

🦄 change detected

This Pull Request includes changes to the following packages.

Package Version
gradio patch
  • Maintainers can select this checkbox to manually select packages to update.

With the following changelog entry.

Fix: Added support for show_api in mount_gradio_app

Maintainers or the PR author can modify the PR title to modify this entry.

Something isn't right?

  • Maintainers can change the version label to modify the version bump.
  • If the bot has failed to detect any changes, or if this pull request needs to update multiple packages to different versions or requires a more comprehensive changelog entry, maintainers can update the changelog file directly.

@HongweiRuan
Copy link
Contributor Author

Hi @abidlabs,

Thank you for the clarification! This is my very first time trying to make contributions. The original issue described the problem as follows:

"I create a Gradio app using mount_gradio_app so I don't need to use the launch function call. I want to hide the "Use via API" on the footer. I try to add app_kwargs={"show_api": False} when calling the mount_gradio_app() and the "Use via API" message is still shown on the footer."

Based on this description, I initially thought the user wanted to achieve this functionality via app_kwargs={"show_api": False}, which led to my initial implementation.

I now understand from your explanation that show_api is an attribute of Blocks, not FastAPI. I’m happy to continue fixing this issue following your suggestion to add show_api as a separate argument in mount_gradio_app.

@abidlabs
Copy link
Member

abidlabs commented Dec 3, 2024

Yes thanks for the explanation @HongweiRuan please feel free to make any modifications to this PR and we can review

@abidlabs
Copy link
Member

abidlabs commented Dec 4, 2024

@HongweiRuan are you able to tweak your PR based on my suggestion above?

@HongweiRuan
Copy link
Contributor Author

@HongweiRuan are you able to tweak your PR based on my suggestion above?

Hi @abidlabs, sorry I was preparing for my final exams these days and haven't got a time to fix it, can I fix it in several days?

@freddyaboulton
Copy link
Collaborator

Yea should be fine @HongweiRuan . We just don't want to leave forgotten PRs open or stale!

@HongweiRuan
Copy link
Contributor Author

Yea should be fine @HongweiRuan . We just don't want to leave forgotten PRs open or stale!

Yes, I will definitely look into it when I got some time.

@HongweiRuan
Copy link
Contributor Author

Hi @abidlabs @freddyaboulton, could you check my newest commit please? I think that fixes the problem, thank you!

@HongweiRuan
Copy link
Contributor Author

Hi @abidlabs @freddyaboulton ,sorry there was one blank line contained a whitespace in my code causes one test to fail, I have fixed that and should be good to test again!

gradio/routes.py Outdated Show resolved Hide resolved
gradio/routes.py Outdated Show resolved Hide resolved
Copy link
Member

@abidlabs abidlabs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm thanks @HongweiRuan!

@abidlabs abidlabs merged commit 43d88c3 into gradio-app:main Dec 8, 2024
22 checks passed
@HongweiRuan
Copy link
Contributor Author

@abidlabs Thank you very much for your support and patience on my first contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot set show_api to False in mount_gradio_app
4 participants