Cancel server progress from the python client #8245

freddyaboulton · 2024-05-08T22:07:59Z

Description

Closes: #8188

If the demo has a defined cancel event for the given function, it will be called from job.cancel automatically. This still won't match the 3.x behavior (.cancel would always stop an iterator in the server even if no cancel event was defined) but I think this is the best we can do without restructuring how the client sets up the SSE stream.

The issue is that we have one connection to the server for reading updates for all jobs so we can't close that stream if one job is cancelled because it would stop all currently submitted jobs.

I thought about using the "simple" API in the python client as that would let us have a different stream for each connection but that would not be backwards compatible for all versions of 4.x and would require restructuring the client further so I decided against that.

🎯 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

freddyaboulton · 2024-05-08T22:08:36Z

client/python/gradio_client/client.py

@@ -247,7 +247,9 @@ def stream_messages(
                            resp = json.loads(line[5:])
                            if resp["msg"] == ServerMessage.heartbeat:
                                continue
-                            elif resp["msg"] == ServerMessage.server_stopped:
+                            elif (
+                                resp.get("message", "") == ServerMessage.server_stopped


When the server stops unexpectedly it sends an unexpected_error not a server_stopped message.

Just to make sure I understand, if the server stops unexpectedly, resp will not have a msg key but instead have a message key? That seems weird, can we unify?

Do we need to make any changes in the frontend, e.g.

gradio/client/js/src/helpers/api_info.ts

Line 210 in bed2f82

case "unexpected_error":

?

It sends a response with msg and message keys but the msg key is unexpected_error as opposed to server_stopped so this if block never catches. I don't think it makes sense to check for unexpected_error in that codeblock because unexpected errors should not close the entire stream since they can be isolated to a single event id.

gradio-pr-bot · 2024-05-08T22:08:41Z

🪼 branch checks and previews

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

Install Gradio from this PR

pip install https://gradio-builds.s3.amazonaws.com/7b5ab761112faff49c5cb518aebed7bc2caf8097/gradio-4.29.0-py3-none-any.whl

Install Gradio Python Client from this PR

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

gradio-pr-bot · 2024-05-08T22:08:56Z

🦄 change detected

This Pull Request includes changes to the following packages.

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

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

With the following changelog entry.

Cancel server progress from the python client

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.

pngwn · 2024-05-08T23:01:26Z

What is a cancel event?

abidlabs · 2024-05-09T02:43:50Z

What is a cancel event?

An event that includes cancels=, can be used to cancel a different event that is iterating or hasn't started yet

abidlabs · 2024-05-09T03:09:10Z

client/python/gradio_client/client.py

+            if inferred_fn_index in dep["cancels"]:
+                cancellable = True
+                cancel_fn_index = i


I don't think this will work if we have a single event that cancels multiple events. e.g. something like:

import gradio as gr with gr.Blocks() as demo: t1 = gr.Textbox() t2 = gr.Textbox() t3 = gr.Textbox() t4 = gr.Textbox() e = t1.change(lambda x:x, t1, t2) f = t2.change(lambda x:x, t2, t3) t4.change(lambda x:x, t4, t3, cancels=[e, f])

if I understand correctly, let's say a user starts, via api, jobs e and f. If they cancel one of them, it'll cancel the other as well

I think this is actually ok because when using the app via the UI, the t4.change event will also cancel both events. So I don't think the client should do something different from the UI.

That being said, I will look into your suggestion of creating a unique cancel event for every dependency so that every event can be cancelled from a client going forward!

So I would agree that if someone calls the route corresponding to the t4.change() event, they may expect e and f to get canceled.

But if someone starts the e job on its own via the api, and they cancel it using job_e.cancel(), I don't think the expectation is that job_f should get canceled as well

abidlabs · 2024-05-09T03:10:03Z

I like the approach in this PR @freddyaboulton! I noticed the issue above^ but what if we use essentially the same idea to create a "hidden" cancel event for every dependency, and then we call that via the client?

freddyaboulton · 2024-05-09T23:29:21Z

client/python/gradio_client/client.py

+                    "/" + self.client.config["dependencies"][i].get("api_name")
+                    for i in other_cancelled
+                ]
+                cancel_msg = (


Verified this warning is triggered correctly

freddyaboulton · 2024-05-09T23:29:33Z

client/python/gradio_client/client.py

+            cancel_msg = None
+            cancellable = fn_index is not None
+            cancel_msg = (
+                "Cancelling this job will not stop the server from running. "


Verified this warning is triggered correctly

In my testing, I noticed that this message appears even if the Gradio app (is running 4.29 and) already includes an event that cancels this event

abidlabs · 2024-05-09T23:35:50Z

gradio/package.json

@@ -1,6 +1,6 @@
 {
 	"name": "gradio",
-	"version": "4.29.0",
+	"version": "4.30.0",


I assume this was for testing purposes, let's revert?

Yea I needed this for the tests to test the new logic. I asked @pngwn and he thinks its ok cause the release pr will override this anyways. But I can revert if we don't mind not having the test coverage until we release!

Ah got it, yea we can leave it

abidlabs · 2024-05-09T23:36:02Z

js/lite/package.json

@@ -1,6 +1,6 @@
 {
 	"name": "@gradio/lite",
-	"version": "4.29.0",
+	"version": "4.30.0",


Same as above

client/python/gradio_client/client.py

abidlabs · 2024-05-09T23:52:02Z

gradio/routes.py

@@ -802,6 +803,11 @@ async def queue_join_helper(
                raise HTTPException(status_code=status_code, detail=event_id)
            return {"event_id": event_id}

+        @app.post("/cancel")


abidlabs · 2024-05-10T00:47:09Z

client/python/gradio_client/client.py

+            fn_index, other_cancelled = (
+                min(candidates, key=lambda x: len(x[1])) if candidates else (None, None)
+            )
+            cancel_msg = None


Unnecessary line?

Needs to be set otherwise we'd reference an unbound variable below

abidlabs

LGTM @freddyaboulton! Tested different apps and this works perfectly for Gradio apps going forward and I'm glad we were able to get some level of backwards-compatibility as well.

(see my note above about the warning message appearing even if the Gradio app already includes an event that cancels this event)

freddyaboulton · 2024-05-10T16:26:36Z

Thanks for the review @abidlabs !

freddyaboulton commented May 8, 2024

View reviewed changes

freddyaboulton requested review from aliabd, abidlabs, aliabid94, dawoodkhan82, pngwn and hannahblair May 8, 2024 22:52

freddyaboulton marked this pull request as ready for review May 8, 2024 22:52

abidlabs reviewed May 9, 2024

View reviewed changes

freddyaboulton force-pushed the 8188-python-client-cancel branch from 2755d49 to 6bfa640 Compare May 9, 2024 19:32

freddyaboulton commented May 9, 2024

View reviewed changes

abidlabs reviewed May 9, 2024

View reviewed changes

js/lite/package.json

@@ -1,6 +1,6 @@

{

"name": "@gradio/lite",

"version": "4.29.0",

"version": "4.30.0",

Copy link

Member

abidlabs May 9, 2024

Choose a reason for hiding this comment

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

Same as above

abidlabs reviewed May 9, 2024

View reviewed changes

client/python/gradio_client/client.py Outdated Show resolved Hide resolved

abidlabs reviewed May 9, 2024

View reviewed changes

abidlabs reviewed May 10, 2024

View reviewed changes

abidlabs approved these changes May 10, 2024

View reviewed changes

freddyaboulton and others added 5 commits May 10, 2024 11:09

implementation + test

0061eb7

add changeset

20e2d19

WIP using events

ee59f92

Use /cancel route

8ce65f4

Try bumping lite

034e4f6

freddyaboulton force-pushed the 8188-python-client-cancel branch from ab1f384 to 034e4f6 Compare May 10, 2024 15:09

add changeset

1a5766a

Fix stream closing issue

7b5ab76

freddyaboulton merged commit c562a3d into main May 10, 2024
7 of 8 checks passed

freddyaboulton deleted the 8188-python-client-cancel branch May 10, 2024 16:26

pngwn mentioned this pull request May 10, 2024

chore: update versions #8225

Merged

gradio-pr-bot mentioned this pull request Jun 5, 2024

Client Release Notes #8468

Merged

This was referenced Jun 6, 2024

chore: update versions #8458

Merged

chore: update versions #8482

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cancel server progress from the python client #8245

Cancel server progress from the python client #8245

freddyaboulton commented May 8, 2024

freddyaboulton May 8, 2024

abidlabs May 9, 2024

abidlabs May 9, 2024

freddyaboulton May 9, 2024

gradio-pr-bot commented May 8, 2024 •

edited

Loading

gradio-pr-bot commented May 8, 2024 •

edited

Loading

Something isn't right?

pngwn commented May 8, 2024

abidlabs commented May 9, 2024

abidlabs May 9, 2024

freddyaboulton May 9, 2024

abidlabs May 9, 2024

abidlabs commented May 9, 2024

freddyaboulton May 9, 2024

freddyaboulton May 9, 2024

abidlabs May 10, 2024

abidlabs May 9, 2024

freddyaboulton May 9, 2024

abidlabs May 9, 2024

abidlabs May 9, 2024

abidlabs May 9, 2024

abidlabs May 10, 2024

freddyaboulton May 10, 2024

abidlabs left a comment •

edited

Loading

freddyaboulton commented May 10, 2024

Cancel server progress from the python client #8245

Cancel server progress from the python client #8245

Conversation

freddyaboulton commented May 8, 2024

Description

🎯 PRs Should Target Issues

Tests

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

gradio-pr-bot commented May 8, 2024 • edited Loading

🪼 branch checks and previews

gradio-pr-bot commented May 8, 2024 • edited Loading

🦄 change detected

This Pull Request includes changes to the following packages.

With the following changelog entry.

Something isn't right?

pngwn commented May 8, 2024

abidlabs commented May 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abidlabs commented May 9, 2024

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abidlabs left a comment • edited Loading

Choose a reason for hiding this comment

freddyaboulton commented May 10, 2024

gradio-pr-bot commented May 8, 2024 •

edited

Loading

gradio-pr-bot commented May 8, 2024 •

edited

Loading

abidlabs left a comment •

edited

Loading