Stop running iterators when js client disconnects #7835

freddyaboulton · 2024-03-26T00:58:36Z

Description

If you run an iterator event and close the page the python function will keep running in the backend.

To test, run the following demo, click the button and close the page. Things will still be being printed to the console in main. On this branch, they do not. Also added a playwright test for this.

    def iterate():
        for i in range(8):
            print(f"UPDATE: {i}")
            yield i
            time.sleep(0.5)

    with gr.Blocks() as demo:
        with gr.Row():
            c_time2 = gr.Textbox(label="Iteration Step")
            btn = gr.Button("Start")
            btn.click(iterate, None, c_time2, api_name="start")


   demo.launch()

🎯 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 · 2024-03-26T00:59:17Z

🪼 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/68608040ca8d10d38bb626ac969134afe4e50c50/gradio-4.23.0-py3-none-any.whl

Install Gradio Python Client from this PR

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

gradio-pr-bot · 2024-03-26T00:59:29Z

🦄 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.

Stop running iterators when js client disconnects

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.

abidlabs · 2024-03-26T20:18:13Z

demo/cancel_events/run.py

Let's revert these changes?

need this for the 2e2 test actually! I don't know how else to verify the server process stopped running

Ah got it! We don't use this demo in any of our docs so it should be fine

abidlabs · 2024-03-26T20:33:17Z

demo/cancel_events/run.py

    for i in range(steps):
        print(f"Current step: {i}")
+        log_file.write_text(f"Current step: {i}\n")
        time.sleep(0.5)


Could we make this faster? So that we could reduce the time that we have to wait in the test

Suggested change

time.sleep(0.5)

time.sleep(0.2)

abidlabs · 2024-03-26T20:34:11Z

gradio/routes.py

@@ -803,11 +803,11 @@ async def sse_stream(request: fastapi.Request):
                        message=str(e),
                    )
                    response = process_msg(message)
-                    if response is not None:


Ah so we already had the deletion logic in place, but it was never called?

Yea we yielded the response before the clean up logic happened so we never reached that point in the code

abidlabs · 2024-03-26T20:37:44Z

js/app/test/cancel_events.spec.ts

+		"../../demo/cancel_events/cancel_events_output_log.txt",
+		"utf-8"
+	);
+	expect(data).not.toContain("Current step: 8");


Let's also ensure that at least one of the steps that should be logged are logged

Suggested change

expect(data).not.toContain("Current step: 8");

expect(data).toContain("Current step: 1");

expect(data).not.toContain("Current step: 8");

Reasonable!

abidlabs

LGTM with a few suggestions. Cool test!

freddyaboulton · 2024-03-27T16:41:52Z

Thanks for the review @abidlabs ! I addressed your comments so will merge once ci passes

freddyaboulton added the v: patch A change that requires a patch release label Mar 26, 2024

freddyaboulton force-pushed the close-running-iterators branch from 131e866 to aba293f Compare March 26, 2024 15:25

freddyaboulton marked this pull request as ready for review March 26, 2024 15:44

freddyaboulton requested review from aliabid94, abidlabs, aliabd, dawoodkhan82, pngwn and hannahblair March 26, 2024 15:44

abidlabs reviewed Mar 26, 2024

View reviewed changes

abidlabs approved these changes Mar 26, 2024

View reviewed changes

freddyaboulton and others added 7 commits March 27, 2024 12:34

Add code

a3a02a0

add changeset

503c733

move

e9cc00e

add changeset

3bc3477

Add code

0f2af77

Format notebook

c712bc0

Address comments

6860804

freddyaboulton force-pushed the close-running-iterators branch from aba293f to 6860804 Compare March 27, 2024 16:39

freddyaboulton enabled auto-merge (squash) March 27, 2024 16:41

freddyaboulton merged commit ee804b2 into main Mar 27, 2024
8 checks passed

freddyaboulton deleted the close-running-iterators branch March 27, 2024 17:41

pngwn mentioned this pull request Mar 27, 2024

chore: update versions #7826

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stop running iterators when js client disconnects #7835

Stop running iterators when js client disconnects #7835

freddyaboulton commented Mar 26, 2024 •

edited

gradio-pr-bot commented Mar 26, 2024 •

edited

gradio-pr-bot commented Mar 26, 2024 •

edited

Something isn't right?

abidlabs Mar 26, 2024

freddyaboulton Mar 26, 2024

abidlabs Mar 26, 2024

abidlabs Mar 26, 2024

abidlabs Mar 26, 2024

freddyaboulton Mar 26, 2024

abidlabs Mar 26, 2024

abidlabs Mar 26, 2024 •

edited

freddyaboulton Mar 27, 2024

abidlabs left a comment

freddyaboulton commented Mar 27, 2024

	expect(data).not.toContain("Current step: 8");
	expect(data).toContain("Current step: 1");
	expect(data).not.toContain("Current step: 8");

Stop running iterators when js client disconnects #7835

Stop running iterators when js client disconnects #7835

Conversation

freddyaboulton commented Mar 26, 2024 • edited

Description

🎯 PRs Should Target Issues

Tests

gradio-pr-bot commented Mar 26, 2024 • edited

🪼 branch checks and previews

gradio-pr-bot commented Mar 26, 2024 • 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

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 Mar 26, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abidlabs left a comment

Choose a reason for hiding this comment

freddyaboulton commented Mar 27, 2024

freddyaboulton commented Mar 26, 2024 •

edited

gradio-pr-bot commented Mar 26, 2024 •

edited

gradio-pr-bot commented Mar 26, 2024 •

edited

abidlabs Mar 26, 2024 •

edited