Start queue when gradio is a sub application by freddyaboulton · Pull Request #2319 · gradio-app/gradio

freddyaboulton · 2022-09-21T19:48:28Z

Description

Closes: #2292

There were two problems as noted on the original issue:

The ws socket path was incorrect when running as a sub application
The thread to run the queue was not started because that's started by the startup-events route that's called during Blocks.launch.

I think I figured out the ws path issue although I'd like some feedback from @aliabid94 .

The queue thread not starting automatically is an interesting problem. I think the cleanest thing would be to use fastapi startup events to start the queue however there are two issues with that:

Startup events are not started for sub applications (docs) so it wouldn't solve the problem of the queue not starting when gradio is mounted
The queue needs to know about the URL where the gradio app is running in order to get the predictions. Since startup events happen before the app is running, the URL is not defined when the event runs.

I opted for adding a helper method called mount_gradio_app that will add a startup event to the parent application that runs the queue. This is not as nice as using the built-in FastAPI app.mount(gradio_app, ...) syntax but it's the best solution I see now.

Changes to the guide

Checklist:

I have performed a self-review of my own code
My code follows the style guidelines of this project
I have commented my code in hard-to-understand areas
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

github-actions · 2022-09-21T19:50:37Z

All the demos for this PR have been deployed at https://huggingface.co/spaces/gradio-pr-deploys/pr-2319-all-demos

abidlabs · 2022-09-22T18:55:24Z

Nice PR @freddyaboulton! I had a question about supplying the server_name and port to the mount_gradio_app() function. It would be great if the user didn't have to supply that manually and ensure its consistency with what's written on the terminal.

Do we really need to know the absolute path for the local server? It seems like that is used inside the Queue.call_prediction() function, but it's unclear to me why we need the absolute path here.

freddyaboulton · 2022-09-22T19:02:45Z

@abidlabs I agree it's not ideal to have to specify the server and port in two places. I think mount_gradio_app needs the full url so that it can call gradio_app.blocks._queue.set_url. Otherwise the queue won't know which url to send the post requests to in call_prediction.

Do you have other solutions in mind?

aliabid94 · 2022-09-22T19:12:13Z

websocket part LGTM.

abidlabs · 2022-09-22T19:45:02Z

@abidlabs I agree it's not ideal to have to specify the server and port in two places. I think mount_gradio_app needs the full url so that it can call gradio_app.blocks._queue.set_url. Otherwise the queue won't know which url to send the post requests to in call_prediction.

I might be missing something but can't the queue just use the relative URL like all of the other route? Basically here:

gradio/gradio/queue.py

Line 217 in 581fbab

url=f"{self.server_path}api/predict",

Why can't just be:

url="/api/predict",

freddyaboulton · 2022-09-22T20:06:41Z

@abidlabs The queue is running on a separate thread than the main gradio app so it needs the full url path to the predict endpoint to access the gradio api.

Just tried out your suggestion and I got an error with the blocks_essay demo with queue enabled:

abidlabs · 2022-09-22T23:38:46Z

Gotcha, testing the PR now

abidlabs · 2022-09-23T01:08:17Z

    return type(val) is dict and "update" in val.get("__type__", "")
+
+
+def mount_gradio_app(


Nit/suggestion: it feels more natural to me for this method to belong in routes.py with all of the other fastapi-related stuff

abidlabs · 2022-09-23T01:12:08Z

+
 io = gr.Interface(lambda x: "Hello, " + x + "!", "textbox", "textbox")
 gradio_app = gr.routes.App.create_app(io)
+app = gr.mount_gradio_app(app, gradio_app, server_name="localhost", port="8000", path="/gradio")


A suggestion for this function: can we have it take a Gradio Blocks object instead of a FastAPI app as its second parameter? And then internally it calls it gr.routes.App.create_app(io). This saves our users a line of code and needing to reference two different methods both of which are deep in our code base

Sure thing! Good point.

abidlabs · 2022-09-23T01:23:49Z

@freddyaboulton I tested and this PR works great, and allows a FastAPI app to mount a Gradio app with or without queue enabled.

My main suggestion would be to try to simplify the usage for our users. I noticed that there is actually a way to get the server URL without needing the user to specify it. In any FastAPI route, if you add the request: Request parameter, you can access the server's URL with request.url._url (or str(request.url) which may be better since it doesn't require accessing a private variable). Similarly, for a websocket route, you can do websocket.url._url.

So this suggests an idea:

When the FastAPI server first launches, set _queue.server_path=None (this is already the case)
In the @app.websocket("/queue/join") route, check the queue's server_path. If it is currently None, then set it to a stripped down version of websocket.url._url

This should eliminate the need for the server_name and port parameters from mount_gradio_app()

freddyaboulton · 2022-09-23T18:07:46Z

Thanks for the awesome suggestion @abidlabs ! I agree that it's much better to not have users specify the same information twice. I pushed up the suggestion you made - we can now set the gradio api endpoint dynamically from the queue join route if it's not specified.

This works! I deployed a fastapi app with gradio as a sub application here: https://gradio-sub-app.onrender.com/

If you go to https://gradio-sub-app.onrender.com/gradio/ you will see a gradio app!

You can also access the lion image here: https://gradio-sub-app.onrender.com/lion.jpg (I mounted a static file app on the root path).

The problem with this approach is that it won't work on spaces because spaces only expose websockets on a specific endpoint that's different from the one gradio is running on, e.g. wss://spaces.huggingface.tech/space-name/queue/join.

What if we keep an optional parameter called gradio_api_endpoint to mount_gradio_app that let's users manually set the gradio endpoint if they desire.

With that approach, I was able to get gradio sub-applications to work on spaces here: https://huggingface.co/spaces/freddyaboulton/gradio-subapp

abidlabs · 2022-09-23T18:22:57Z

Thank you so much for trying out the suggestions @freddyaboulton!

What if we keep an optional parameter called gradio_api_endpoint to mount_gradio_app that let's users manually set the gradio endpoint if they desire.

Sounds good! Let's add and document this parameter :)

freddyaboulton · 2022-09-23T19:18:15Z

@abidlabs Just pushed up the doc changes!

Documentation here:

Updated guide here:

Should be good for a final look now!

abidlabs · 2022-09-23T19:32:31Z

+    app: fastapi.FastAPI,
+    blocks: gradio.Blocks,
+    path: str,
+    gradio_api_url: Optional[str],


Suggested change

gradio_api_url: Optional[str],

gradio_api_url: Optional[str] = None,

abidlabs · 2022-09-23T19:39:43Z

Works great @freddyaboulton. I'm going to push some small tweaks to the documentation (easier than commenting in all of the places) if you don't mind! Feel free to take or leave

…adio into fix-queue-when-app-mounted

abidlabs · 2022-09-23T19:42:22Z

Awesome @freddyaboulton! LGTM

freddyaboulton · 2022-09-23T20:01:41Z

Thanks for the review @abidlabs !

freddyaboulton force-pushed the fix-queue-when-app-mounted branch from 1853b1d to e3da8e3 Compare September 21, 2022 19:49

freddyaboulton commented Sep 21, 2022

View reviewed changes

Comment thread ui/packages/app/src/api.ts

freddyaboulton commented Sep 21, 2022

View reviewed changes

Comment thread gradio/blocks.py

freddyaboulton changed the title ~~Fix queue when app mounted~~ Start queue when gradio is a sub application Sep 21, 2022

freddyaboulton requested review from abidlabs, aliabd, aliabid94, dawoodkhan82 and pngwn September 21, 2022 21:20

freddyaboulton marked this pull request as ready for review September 21, 2022 21:21

abidlabs reviewed Sep 23, 2022

View reviewed changes

Comment thread gradio/utils.py Outdated

abidlabs reviewed Sep 23, 2022

View reviewed changes

freddyaboulton added 6 commits September 23, 2022 10:26

First stab at it

8cfa6ff

Use util methos

086b5f7

lint

1129c4a

Test

91117b1

Fix formatting

fa9ef1b

Try out setting predict endpoint from websocket request

7af2f04

freddyaboulton force-pushed the fix-queue-when-app-mounted branch from 363131b to 7af2f04 Compare September 23, 2022 15:42

freddyaboulton added 2 commits September 23, 2022 11:49

lint

566e552

Fix bug

84a6d35

freddyaboulton added 2 commits September 23, 2022 13:31

Address comments - remove server and port

bc68ecc

Skip in 3.7

c3a4af6

freddyaboulton requested a review from abidlabs September 23, 2022 18:08

Fix documentation

701b884

abidlabs reviewed Sep 23, 2022

View reviewed changes

Add default 🤦

fbc4e75

abidlabs added 2 commits September 23, 2022 14:41

docs tweak

cae4374

Merge branch 'fix-queue-when-app-mounted' of github.com:gradio-app/gr…

62ce1d2

…adio into fix-queue-when-app-mounted

abidlabs approved these changes Sep 23, 2022

View reviewed changes

Add back imports that were deleted by wrong linter version

51a83ac

freddyaboulton merged commit 9f7dd05 into main Sep 23, 2022

freddyaboulton deleted the fix-queue-when-app-mounted branch September 23, 2022 20:01

radames mentioned this pull request Sep 26, 2022

Cant mount gradio as a sub-application if the queue is enabled #2292

Closed

1 task

		return type(val) is dict and "update" in val.get("__type__", "")


		def mount_gradio_app(

	gradio_api_url: Optional[str],
	gradio_api_url: Optional[str] = None,

Conversation

freddyaboulton commented Sep 21, 2022 • edited by abidlabs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes to the guide

Checklist:

Uh oh!

github-actions Bot commented Sep 21, 2022

Uh oh!

Uh oh!

Uh oh!

abidlabs commented Sep 22, 2022

Uh oh!

freddyaboulton commented Sep 22, 2022

Uh oh!

aliabid94 commented Sep 22, 2022

Uh oh!

abidlabs commented Sep 22, 2022

Uh oh!

freddyaboulton commented Sep 22, 2022

Uh oh!

abidlabs commented Sep 22, 2022

Uh oh!

Uh oh!

abidlabs Sep 23, 2022

Choose a reason for hiding this comment

Uh oh!

freddyaboulton Sep 23, 2022

Choose a reason for hiding this comment

Uh oh!

abidlabs Sep 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

freddyaboulton Sep 23, 2022

Choose a reason for hiding this comment

Uh oh!

abidlabs commented Sep 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

freddyaboulton commented Sep 23, 2022

Uh oh!

abidlabs commented Sep 23, 2022

Uh oh!

freddyaboulton commented Sep 23, 2022

Uh oh!

abidlabs Sep 23, 2022

Choose a reason for hiding this comment

Uh oh!

abidlabs commented Sep 23, 2022

Uh oh!

abidlabs commented Sep 23, 2022

Uh oh!

freddyaboulton commented Sep 23, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

freddyaboulton commented Sep 21, 2022 •

edited by abidlabs

Loading

abidlabs Sep 23, 2022 •

edited

Loading

abidlabs commented Sep 23, 2022 •

edited

Loading