Skip to content
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

fix upload file delay #4661

Merged
merged 4 commits into from Jun 26, 2023
Merged

fix upload file delay #4661

merged 4 commits into from Jun 26, 2023

Conversation

aliabid94
Copy link
Collaborator

There was a delay between an uploaded file and then trigger for change and upload listeners. This was caused by the fact that files were actually getting uploaded twice, once in the svelte File component, and once in the client library when submitting data. This us because the client looks to see if there's any blob in the component data and uploads it. I removed the blob in the component so that the client does not resubmit - ideally we could let the client handle all uploading but the upload animation and dispatching of "change" and "submit" within the component won't work with that, so it's handled only in the component now.

Fixes: #4385

To test, run any demo that has a change or upload trigger on file, example below. Previously, you would see a lag between the finishing of the upload animation and triggering of the backend. You would also two network requests made "/upload" made sequentially. These should be fixed.

import os
from zipfile import ZipFile
import gradio as gr


def zip_files(files):
    with ZipFile("tmp.zip", "w") as zipObj:
        for idx, file in enumerate(files):
            zipObj.write(file.name, file.name.split("/")[-1])
    return "tmp.zip"

with gr.Blocks() as demo:
    file = gr.File(label="a", file_count="multiple")
    zipped = gr.File(label="b")
    file.change(zip_files, file, zipped)

demo.queue().launch()

@aliabid94 aliabid94 requested review from abidlabs and pngwn June 23, 2023 19:45
@gradio-pr-bot
Copy link
Contributor

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

@pngwn
Copy link
Member

pngwn commented Jun 23, 2023

Ah, my bad. Nice catch.

@@ -80,10 +81,11 @@
file_data.orig_name = file_data.name;
file_data.name = response.files[i];
file_data.is_file = true;
file_data.blob = null;
Copy link
Member

Choose a reason for hiding this comment

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

Think we just need to use undefined for all of these empty blob values and those errors.

Copy link
Member

Choose a reason for hiding this comment

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

beat me to it.

Copy link
Member

@pngwn pngwn left a comment

Choose a reason for hiding this comment

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

Tested and working. Looks good to me! Thanks @aliabid94!

@abidlabs
Copy link
Member

Thanks guys! Will go ahead and merge in so that we can work on reducing the number of open PRs!

@abidlabs abidlabs merged commit 7b8fbb1 into main Jun 26, 2023
8 checks passed
@abidlabs abidlabs deleted the upload_file_fix branch June 26, 2023 13:14
dawoodkhan82 pushed a commit that referenced this pull request Jun 27, 2023
* fix

* changes

* changes

---------

Co-authored-by: Abubakar Abid <abubakar@huggingface.co>
dawoodkhan82 added a commit that referenced this pull request Jun 28, 2023
* playwright tests for chatbot

* more tests

* changelog

* Update CHANGELOG.md

* fix upload file delay (#4661)

* fix

* changes

* changes

---------

Co-authored-by: Abubakar Abid <abubakar@huggingface.co>

* Forward `tqdm` constructor arguments to `Progress` component (#4475)

* Forward tqdm constructor arguments to Progress component

Signed-off-by: Ilya Trushkin <ilya.trushkin@intel.com>

* lint

* reorder args

* added tests

---------

Signed-off-by: Ilya Trushkin <ilya.trushkin@intel.com>
Co-authored-by: Abubakar Abid <abubakar@huggingface.co>

* Remove cleared_value (#4685)

* Remove cleared_value

* CHANGELOG

* Add requirements.txt to dialogpt demo (#4686)

* Add requirements.txt

* Update demo notebook

* Add torch

* remove streaming demo + more tests

* Fix blocks_kitchen_sink and streaming_stt demos (#4699)

* Add code

* Add json file

* Remove streaming_stt demo

* Undo generate_notebooks

* Add blocks_kitchen_sink

* fix tests

* Update js/app/test/chatbot_multimodal.spec.ts

Co-authored-by: pngwn <hello@pngwn.io>

* Update js/app/test/chatbot_multimodal.spec.ts

Co-authored-by: pngwn <hello@pngwn.io>

* Update js/app/test/chatbot_multimodal.spec.ts

Co-authored-by: pngwn <hello@pngwn.io>

* Update js/app/test/chatbot_multimodal.spec.ts

Co-authored-by: pngwn <hello@pngwn.io>

* Update js/app/test/chatbot_multimodal.spec.ts

Co-authored-by: pngwn <hello@pngwn.io>

* Update js/app/test/chatbot_multimodal.spec.ts

Co-authored-by: pngwn <hello@pngwn.io>

* Update js/app/test/chatbot_multimodal.spec.ts

Co-authored-by: pngwn <hello@pngwn.io>

* Update js/app/test/chatbot_multimodal.spec.ts

Co-authored-by: pngwn <hello@pngwn.io>

* Update js/app/test/chatbot_multimodal.spec.ts

Co-authored-by: pngwn <hello@pngwn.io>

* update notebook

* remove debug

* remove debug

---------

Signed-off-by: Ilya Trushkin <ilya.trushkin@intel.com>
Co-authored-by: aliabid94 <aabid94@gmail.com>
Co-authored-by: Abubakar Abid <abubakar@huggingface.co>
Co-authored-by: Ilya Trushkin <ilya.trushkin@intel.com>
Co-authored-by: Freddy Boulton <alfonsoboulton@gmail.com>
Co-authored-by: pngwn <hello@pngwn.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Delay between file upload and change file handler with no animation
4 participants