Python client properly handles hearbeat and log messages. Also handles responses longer than 65k #6693

freddyaboulton · 2023-12-06T21:06:07Z

Description

Closes: #6319
Closes: #6601
Closes: #6541
Closes: #6494
Closes: #6776

🎯 PRs Should Target Issues

Before your create a PR, please check to see if there is an existing issue for this change. If not, please create an issue before you create this PR, unless the fix is very small.

Not adhering to this guideline will result in the PR being closed.

Tests

PRs will only be merged if tests pass on CI. To run the tests locally, please set up your Gradio environment locally and run the tests: bash scripts/run_all_tests.sh
You may need to run the linters: bash scripts/format_backend.sh and bash scripts/format_frontend.sh

gradio-pr-bot · 2023-12-06T21:06:23Z

🪼 branch checks and previews

•	Name	Status	URL
	Spaces	ready!	Spaces preview
	Website	ready!	Website preview
🦄	Changes	detected!	Details
📓	Notebooks	not matching!	Details

The demo notebooks don't match the run.py files. Please run this command from the root of the repo and then commit the changes:

pip install nbformat && cd demo && python generate_notebooks.py

Install Gradio from this PR

pip install https://gradio-builds.s3.amazonaws.com/99250ab27898bfc2b6d47b6302048abfca9bf534/gradio-4.9.0-py3-none-any.whl

Install Gradio Python Client from this PR

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

gradio-pr-bot · 2023-12-06T21:07:01Z

🦄 change detected

This Pull Request includes changes to the following packages.

Package	Version
`@gradio/client`	`patch`
`gradio`	`patch`
`gradio_client`	`patch`

Maintainers can select this checkbox to manually select packages to update.

With the following changelog entry.

Python client properly handles hearbeat and log messages. Also handles responses longer than 65k

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.

freddyaboulton · 2023-12-06T21:35:30Z

client/python/gradio_client/utils.py

@@ -87,6 +87,19 @@ class SpaceDuplicationError(Exception):
    pass


+class ServerMessage(str, Enum):


Added this to make it easier to keep track of all the messages and avoid typos etc

freddyaboulton · 2023-12-13T18:54:47Z

I believe test is a flake. Will look into it but this should be good to review.

abidlabs · 2023-12-13T19:56:37Z

Awesome! Taking a look

abidlabs · 2023-12-13T20:00:37Z

gradio/routes.py

+            succes, event_id = await blocks._queue.push(body, request, username)
+            if not succes:
+                status_code = (
+                    status.HTTP_503_SERVICE_UNAVAILABLE


abidlabs · 2023-12-13T20:00:51Z

Just to understand, what was the root cause of #6319?

freddyaboulton · 2023-12-13T20:30:39Z

Just to understand, what was the root cause of #6319?

I think there were two problems. 1) the "heartbeat" message was not being handled by the client 2) aiter_text was yielding an incomplete message (can't be parsed to json) if it was very long

abidlabs · 2023-12-13T20:45:24Z

gradio/routes.py

+            succes, event_id = await blocks._queue.push(body, request, username)
+            if not succes:


Suggested change

succes, event_id = await blocks._queue.push(body, request, username)

if not succes:

success, event_id = await blocks._queue.push(body, request, username)

if not success:

abidlabs · 2023-12-13T20:48:41Z

client/python/gradio_client/utils.py

+            async for line in response.aiter_lines():
+                line = line.rstrip("\n")
+                if len(line) == 0:
+                    continue
                if line.startswith("data:"):
                    resp = json.loads(line[5:])
+                    if resp["msg"] in [ServerMessage.log, ServerMessage.heartbeat]:
+                        continue


Note that these changes were made in stream_sse_v0. Do you also want to make similar changes (particularly the heartbeat one) in stream_sse_v1?

Whoops - yea I think it should be both!

Actually the stream_sse_v1 case is handled in Client.stream_messages! Stream_sse_v1 doesn't directly connect to the SSE endpoint like v0 does. It just pulls messages off a message queue.

abidlabs · 2023-12-13T20:55:27Z

gradio/queueing.py

@@ -144,15 +145,16 @@ def _resolve_concurrency_limit(self, default_concurrency_limit):

    async def push(
        self, body: PredictBody, request: fastapi.Request, username: str | None
-    ):
+    ) -> tuple[bool, str]:


This might have broken max_size.

I ran this demo and submitted jobs on 3 separate tabs:

import time import gradio as gr def greet(name): time.sleep(20) return "Hello " + name + "!" demo = gr.Interface(fn=greet, inputs="text", outputs="text").queue(max_size=1) if __name__ == "__main__": demo.launch(show_api=False)

On the final tab, I see this error modal (instead of one telling me that the queue is full):

@abidlabs Did you build the frontend before running?

I did not in fact :) but after rebuilding it, I still don't see any modal saying that the queue is full. Instead, on the 3rd tab, I just see a job that never enters the queue:

abidlabs · 2023-12-13T21:00:13Z

I took a first pass on this PR, and I can confirm #6319 and #6601 have been fixed.

I left some small comments above @freddyaboulton if you can take a look. Happy to take another pass on this PR soon!

freddyaboulton · 2023-12-13T18:46:46Z

client/python/test/test_client.py

+                job2.result()
+                job3.result()
+
+    def test_json_parse_error(self):


This is the repro from #6494 and it's passing so I think we can close the issue

freddyaboulton · 2023-12-13T18:47:04Z

client/python/gradio_client/client.py

-                                    self.stream_open = False
-                                    return
-                            elif message == "":
+                    async for line in response.aiter_lines():


I think this works too @aliabid94 ?

freddyaboulton · 2023-12-13T18:48:00Z

client/js/src/client.ts

+			output = await response.json();
+			status = response.status;
+		} catch (e) {
+			output = { error: `Could not parse server response: ${e}` };


What was happening is that we were raising exceptions in the server, which means FastAPI returns the payload as a plain string. Since it can't be coverted to JSON, this errors out and the whole client/app hangs.

Also fixed in the server by raising HTTPException

freddyaboulton · 2023-12-13T18:48:57Z

gradio/routes.py

+            succes, event_id = await blocks._queue.push(body, request, username)
+            if not succes:
+                status_code = (
+                    status.HTTP_503_SERVICE_UNAVAILABLE


Using a different status code for QUEUE_FULL so that we can show the right error message in the app

freddyaboulton · 2023-12-13T21:09:03Z

gradio/queueing.py

@@ -144,15 +145,16 @@ def _resolve_concurrency_limit(self, default_concurrency_limit):

    async def push(
        self, body: PredictBody, request: fastapi.Request, username: str | None
-    ):
+    ) -> tuple[bool, str]:


@abidlabs Did you build the frontend before running?

freddyaboulton · 2023-12-13T21:09:32Z

client/python/gradio_client/utils.py

+            async for line in response.aiter_lines():
+                line = line.rstrip("\n")
+                if len(line) == 0:
+                    continue
                if line.startswith("data:"):
                    resp = json.loads(line[5:])
+                    if resp["msg"] in [ServerMessage.log, ServerMessage.heartbeat]:
+                        continue


Whoops - yea I think it should be both!

abidlabs · 2023-12-13T22:05:08Z

Thanks @freddyaboulton I took another look and this is great!

Just wanted to note a few things:

KeyError: 'heartbeat' when python client request took more than approximately 16 seconds #6319 is fixed
gradio 4 critical bug -- all return messages truncated to 65k #6601 is fixed
gradio_client fails when full queue #6541 is fixed in the sense that now we get QueueError: Queue is full! Please try again. in the Client.
gradio_client fails most of the time with JSONDecodeError (on a Chat interface that provides results in streaming) #6494 - the original Space is down so I can't repro but it looks like you've captured the gist of the repro in a unit test
js client hangs forever when you submit a prediction to a full queue #6776 is not fixed for me, even after rebuilding the frontend

abidlabs

Fixed for me now, thank you! Great PR @freddyaboulton

Let's do a patch release with this PR and #6525

freddyaboulton · 2023-12-13T22:47:03Z

Thanks @abidlabs !

freddyaboulton mentioned this pull request Dec 6, 2023

gradio 4 critical bug -- all return messages truncated to 65k #6601

Closed

1 task

freddyaboulton added the v: patch A change that requires a patch release label Dec 6, 2023

freddyaboulton commented Dec 6, 2023

View reviewed changes

freddyaboulton mentioned this pull request Dec 7, 2023

gradio_client fails when full queue #6541

Closed

1 task

freddyaboulton force-pushed the client-heartbeat-truncation-issue branch from 85c3b97 to 58a060c Compare December 7, 2023 02:00

freddyaboulton and others added 11 commits December 13, 2023 10:08

first commit

976f6ae

newlines

9fb5b07

test

65538eb

Fix depends

1559ef2

revert

6a11c6e

add changeset

201e66d

add changeset

169e1eb

Lint

4cf41c3

queue full test

075fdf8

Add code

521f7ab

Update + fix

89b0a83

freddyaboulton force-pushed the client-heartbeat-truncation-issue branch from 446ded5 to 89b0a83 Compare December 13, 2023 18:32

gradio-pr-bot and others added 2 commits December 13, 2023 18:34

add changeset

bdcb16c

Revert demo

9db0cbc

freddyaboulton marked this pull request as ready for review December 13, 2023 18:54

freddyaboulton requested review from aliabid94, pngwn, abidlabs, aliabd, dawoodkhan82 and hannahblair December 13, 2023 18:54

abidlabs mentioned this pull request Dec 13, 2023

Gradio REST API + bash curl always skips the queue #6350

Closed

1 task

Merge branch 'main' into client-heartbeat-truncation-issue

eb4a97b

abidlabs reviewed Dec 13, 2023

View reviewed changes

freddyaboulton commented Dec 13, 2023

View reviewed changes

Typo in success

7c40cd7

Fix

d6d8f5c

abidlabs approved these changes Dec 13, 2023

View reviewed changes

freddyaboulton merged commit 34f9431 into main Dec 13, 2023
15 checks passed

freddyaboulton deleted the client-heartbeat-truncation-issue branch December 13, 2023 22:47

abidlabs mentioned this pull request Dec 13, 2023

Fixes Drag and Drop for Upload #6525

Merged

pngwn mentioned this pull request Dec 13, 2023

chore: update versions #6779

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Python client properly handles hearbeat and log messages. Also handles responses longer than 65k #6693

Python client properly handles hearbeat and log messages. Also handles responses longer than 65k #6693

freddyaboulton commented Dec 6, 2023 •

edited

gradio-pr-bot commented Dec 6, 2023 •

edited

gradio-pr-bot commented Dec 6, 2023 •

edited

Something isn't right?

freddyaboulton Dec 6, 2023

freddyaboulton commented Dec 13, 2023

abidlabs commented Dec 13, 2023

abidlabs Dec 13, 2023

abidlabs commented Dec 13, 2023

freddyaboulton commented Dec 13, 2023

abidlabs Dec 13, 2023

abidlabs Dec 13, 2023

freddyaboulton Dec 13, 2023

freddyaboulton Dec 13, 2023

abidlabs Dec 13, 2023

freddyaboulton Dec 13, 2023

abidlabs Dec 13, 2023

abidlabs commented Dec 13, 2023 •

edited

freddyaboulton Dec 13, 2023

freddyaboulton Dec 13, 2023

freddyaboulton Dec 13, 2023

freddyaboulton Dec 13, 2023

freddyaboulton Dec 13, 2023

freddyaboulton Dec 13, 2023

abidlabs commented Dec 13, 2023

abidlabs left a comment

freddyaboulton commented Dec 13, 2023

		@@ -87,6 +87,19 @@ class SpaceDuplicationError(Exception):
		pass


		class ServerMessage(str, Enum):

		succes, event_id = await blocks._queue.push(body, request, username)
		if not succes:

Python client properly handles hearbeat and log messages. Also handles responses longer than 65k #6693

Python client properly handles hearbeat and log messages. Also handles responses longer than 65k #6693

Conversation

freddyaboulton commented Dec 6, 2023 • edited

Description

🎯 PRs Should Target Issues

Tests

gradio-pr-bot commented Dec 6, 2023 • edited

🪼 branch checks and previews

gradio-pr-bot commented Dec 6, 2023 • edited

🦄 change detected

This Pull Request includes changes to the following packages.

With the following changelog entry.

Something isn't right?

Choose a reason for hiding this comment

freddyaboulton commented Dec 13, 2023

abidlabs commented Dec 13, 2023

Choose a reason for hiding this comment

abidlabs commented Dec 13, 2023

freddyaboulton commented Dec 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abidlabs commented Dec 13, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abidlabs commented Dec 13, 2023

abidlabs left a comment

Choose a reason for hiding this comment

freddyaboulton commented Dec 13, 2023

freddyaboulton commented Dec 6, 2023 •

edited

gradio-pr-bot commented Dec 6, 2023 •

edited

gradio-pr-bot commented Dec 6, 2023 •

edited

abidlabs commented Dec 13, 2023 •

edited