-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Cancel events from other events #2433
Conversation
All the demos for this PR have been deployed at https://huggingface.co/spaces/gradio-pr-deploys/pr-2433-all-demos |
@freddyaboulton maybe we can consolidate the api with #2436 |
@dawoodkhan82 Do you have something in mind? |
24422fd
to
77a208d
Compare
@@ -119,7 +119,8 @@ def set_event_trigger( | |||
js: Optional[str] = None, | |||
no_target: bool = False, | |||
queue: Optional[bool] = None, | |||
) -> None: | |||
cancels: List[int] | None = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add cancels
to docstring below
@@ -22,6 +76,7 @@ def change( | |||
queue: Optional[bool] = None, | |||
preprocess: bool = True, | |||
postprocess: bool = True, | |||
cancels: List[Dict[str, Any]] | None = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add cancels
to docstring for all events
gradio/routes.py
Outdated
async def reset_iterator(body: ResetBody): | ||
if body.session_hash not in app.iterators: | ||
return {"success": False} | ||
async with asyncio.Lock(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think async with asyncio.Lock():
will have any effect here since each request is instantiating a new asyncio.Lock()
object. A single asyncio.Lock()
object should be instantiated outside of the this function and that object should be acquired here. https://docs.python.org/3/library/asyncio-sync.html
gradio/events.py
Outdated
event_name, | ||
cancel_fn, | ||
inputs=None, | ||
outputs=output, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, do we actually need to supply the outputs?
Hey @freddyaboulton this is a really really cool implementation! I was testing with the following script and everything seems to work well: import gradio as gr
import time
def run(x):
time.sleep(3)
return x
def run2(x):
for i in range(3):
time.sleep(1)
yield i
with gr.Blocks() as demo:
x = gr.Textbox()
y = gr.Textbox()
b = gr.Button()
b2 = gr.Button()
c = gr.Button("Cancel")
e = b.click(run, x, y)
e2 = b2.click(run2, x, y)
c.click(None, None, None, cancels=[e, e2])
demo.queue()
demo.launch() However, one piece of feedback is that currently, when you "cancel" an event, the output component that was being populated clears (i.e. its value gets reset). I don't think this should happen -- rather, the final value should remain there. For example, if someone is generating a sequence of images, they may want to "stop" on a particular image. WDYT? |
One other small thing -- if a gradio app includes cancel events but does not have queue enabled, we should raise an Exception. |
f7cc981
to
af5ace6
Compare
This might close #1323 as well? |
907cbc1
to
43b6987
Compare
@abidlabs I addressed your comments! Cancelling will no longer reset the state of the component. And we raise exceptions if the queue is disabled but there are cancel events. This is that the stop button looks like for interfaces: I ended up creating a new button variant. @pngwn Do you like the approach? Feel free to suggest better colors/styling for the stop button. Should be good for a final review! |
@freddyaboulton looks good to me! I like the extra Could we add |
Fantastic PR @freddyaboulton! One tiny thing I noticed was that when running in a colab / jupyter notebook, because of the iframe's width, the stop button is pushed to the next row: We could either (1) increase the iframe width, but it seems like you need an iframe width of at least 1280 for it to render correctly, which itself can cause the Interface to be cut off: Or (2) we could move it next to the "Flag" button: Otherwise, LGTM! |
@abidlabs Thanks for the catch about rendering in the notebook! I'm worried about putting it under the output component because if it's really long (like an image in diffusion or an RL env frame) the button might be pushed outside the screen. For example, look at the atari agent space: I like the idea of keeping the stop button in the same group as submit and don't mind it's on the next row. Maybe we keep as is for now? Happy to jump on a huddle to discuss further. |
Ok I think that’s fine. LGTM thank you! |
fe0a754
to
55a47b8
Compare
Thanks for the review @abidlabs ! |
@freddyaboulton Good stuff! Just tested it, and works great. |
How to make an event cancel itself? |
if an event can be cancelled, is it possible to recover it? For example, in #1323, after we stop the streaming, how can we resume it? |
@taoari it is not possible to resume an event after it has been canceled. If you have a job that is running, perhaps you could store the intermediate results in a |
@abidlabs Thank you for your suggestion! But currently, I would like to enable webcam streaming again for new results, so it can not store intermediate results in a |
Description
Fixes #2220
This PR allows you to cancel other running events when another event is triggered. The functionality for interface has not been implemented yet. Opening this up as a draft so others can test this out before proceeding.
Changes to user-facing API
Adds a
cancels
parameter to all event handlers. Users can specify a list of events to cancel.Limitations
Example you can use to test this
Build the front-end first.
Cancel on click
Cancel on change
Cancel on submit
Cancel on edit
Cancel on play
Checklist:
A note about the CHANGELOG
Hello 👋 and thank you for contributing to Gradio!
All pull requests must update the change log located in CHANGELOG.md, unless the pull request is labeled with the "no-changelog-update" label.
Please add a brief summary of the change to the Upcoming Release > Full Changelog section of the CHANGELOG.md file and include
a link to the PR (formatted in markdown) and a link to your github profile (if you like). For example, "* Added a cool new feature by
[@myusername](link-to-your-github-profile)
in[PR 11111](https://github.com/gradio-app/gradio/pull/11111)
".If you would like to elaborate on your change further, feel free to include a longer explanation in the other sections.
If you would like an image/gif/video showcasing your feature, it may be best to edit the CHANGELOG file using the
GitHub web UI since that lets you upload files directly via drag-and-drop.