fohrloop / dash-uploader

The alternative upload component for python Dash applications.
MIT License
141 stars 29 forks source link

[bug] isCompleted callback is only executed on first file #5

Closed MM-Lehmann closed 2 years ago

MM-Lehmann commented 4 years ago

When uploading a batch of files, isCompleted is fired upon the first uploaded file only. Any subsequent files get upload still, but no further callback is fired.

Also noteworthy, the progressbar jumps back and forth on multiple files. Starting from 0 for every files but progress seems to be tracked in relation to all files and not per file. This should be more consistent.

Thanks for keeping improving this fancy component!

fohrloop commented 4 years ago

Thanks for reporting @MM-Lehmann !

The support for uploading multiple files is experimental, and will be hard to make perfect. Anyway, the reason for the isCompleted not fired on every upload should be investigated (Also proressbar jumps). Would you be able to describe a MWE so I could test the case and perhaps find a solution?

zz-x404 commented 4 years ago

Thanks for this awesome uploader project @np-8 .

Also, I've noticed that in addition to the behavior of callback above, it seems that even if du.Upload(max_files=1), I can still upload multiple files. And with the custom setting of width, the percentage progress would be displayed in the wrong place. Here is my example app.py file:

# Run this app with `python app.py` and
# visit http://127.0.0.1:8088/ in your web browser.
import os
from pathlib import Path

import dash
import dash_html_components as html
import dash_uploader as du
from dash.dependencies import Output, Input, State

UPLOAD_FOLDER = os.path.join(os.path.dirname(__file__), "uploads")

app = dash.Dash(
    __name__
)
server = app.server

# configure dash-uploader
du.configure_upload(app, UPLOAD_FOLDER)

def get_upload_component(component_id):
    return du.Upload(
        id=component_id,
        text='Drag and Drop or Select Files',
        text_completed='Ready! ',
        max_file_size=2048,  # 2048 Mb
        max_files=1  # even with 1 max file can still upload multiple
    )

def get_app_layout():
    return html.Div(children=[
        html.H1(
            children='Upload',
            style={
                'textAlign': 'center',
            }
        ),
        html.Div(
            [
                get_upload_component(component_id='dash-uploader'),
                html.Div(id='callback-output'),
            ],
            style={  # wrapper div style
                'textAlign': 'center',
                'width': '600px',
                'display': 'block'
            }
        )
    ], style={'margin': '10px'})

app.layout = get_app_layout

@app.callback(
    Output('callback-output', 'children'),
    [Input('dash-uploader', 'isCompleted')],
    [State('dash-uploader', 'fileNames'),
     State('dash-uploader', 'upload_id')],
)
def get_a_list(is_completed, filenames, upload_id):
    if not is_completed:
        return html.Div(hidden=True)
    out = []
    if filenames is not None:
        if upload_id:
            root_folder = Path(UPLOAD_FOLDER) / upload_id
        else:
            root_folder = Path(UPLOAD_FOLDER)

        for filename in filenames:
            file = root_folder / filename
            out.append(file)
        return html.Ul([html.Li(str(x)) for x in out])

    return html.Div("No Files Uploaded Yet!")

if __name__ == '__main__':
    app.run_server(host='0.0.0.0', port=8088, debug=True)
github909 commented 3 years ago

Is there a workaround for uploading multiple files? The dashboard prints out only the name of the first file. I'm trying to upload multiple files and concatenate them into one pandas dataframe and not having success. Thanks!

fohrloop commented 3 years ago

Hello, @github909 , @20joseph10 , @MM-Lehmann

I looked at this and could reproduce the problem by just adding multiple files to the queue. I started working on a fix, but since it needs changes on the React/JS parts, which I am not an expert, I will have to look at it later on again..

🎉 I managed to get the callback to fire multiple times, like this:

🐞 However, there are still lot of bugs: it shows wrong text on the "file uploading" part, the progressbar is still jumping a lot, and the props isCompleted and uploadedFiles seems not to be working reliably yet.

The code is at the fix-multiupload branch. The testing code can be seen at the usage.py and the React file at Upload_ReactComponent.react.js

Most of the needed changes are in the React world ( Upload_ReactComponent.react.js). If anyone using this package has React experience, I would be really happy to get some help with some refactoring / bugfixing. 🙏

I am also considering introducing one backwards-incompatible change: The @du.callback should perhaps take just one filename as argument, or there should be two different callbacks: One that fires on each time a file is uploaded (in multifile case, and takes one filename string as argument), and one that fires once when all files are completed, and takes list of filenames as argument.

fohrloop commented 3 years ago

Just a quick update for anyone needing the multifile upload support. I have my priorities in other (non-dash) projects, so I won't see this being fixed in few coming months unless someone has time to create a pull request. I have two different workarounds until that which may help:

(1) Create a .zip file from files and upload it, and unzip in the server after upload. This adds an additional zipping step for the user but is very easy to implement. (2) When upload event starts, create a file watchdog that watches that specific upload folder for changes, and acts when it does not see any changes for some time. Since each chunk of data is written to a separate temporary file, the watchdog should see files written very often.. until it does not (and that's when all the files have been uploaded). Not very elegant, but it should work (although I haven't tried it). This requires more coding but no extra from the app end user. 

AtharvaKatre commented 3 years ago

Any lead on this?

almontoya commented 3 years ago

Hi,

i'm currently working on a Dash app where I need to upload multiple files, 2 of which are ~ 400mb each. I had the same problem with the callback triggering on the first file uploaded. I found a quick and dirty method that seems to work. I added the following at the beginning of callback, before downstream data read/processing:

if is_completed: time.sleep(80)

I timed how long it took to complete copy of files to local folder and then set sleep as higher value. Obviously this isn't ideal, as file sizes that take longer to upload would crash the app.

In any case thanks to @np-8 et al for the uploader. I'd be up the proverbial creek with out a paddle.

Best, Alex

QHQIII commented 2 years ago

filenames can only get the first file's name and the isCompleted is triggered at the same time 🤔

fohrloop commented 2 years ago

Yeah, unfortunately this bug is still not fixed. It might make sense to fix this bug directly on the flowjs branch, as it flow.js is currently the most probable candidate for being the resumable.js replacement. See also: https://github.com/np-8/dash-uploader/issues/21

fohrloop commented 2 years ago

There are now two pull requests, #50 (based on flowjs) and #59 (based on resumable.js from multiupload branch), which try to access the problem. There should be some merging of code to be done. I would prefer to favor flowjs as dash-uploader will probably start using it instead of resumable.js in the future (#21).

There should be at least some tests added to test the multiupload function. Most probably the multiupload fix will be included in the next non-patch version of dash-uploader.

fohrloop commented 2 years ago

This is now fixed in the dev branch in 511557f587a02fbae6a0462e5074abd449c14bd0. The fix will be included in the dash-uploader 0.7.0 release.