Code

Opened 7 years ago

Closed 6 years ago

Last modified 5 years ago

#4165 closed (fixed)

Upload Progress Middleware and Integration into the Admin

Reported by: Michael Axiak <axiak@…> Owned by: nobody
Component: contrib.admin Version: master
Severity: Keywords: upload, progress, ajax, 2070-docs
Cc: axiak@…, prigun@…, aaron@…, robert.hopson@…, Triage Stage: Someday/Maybe
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: UI/UX:

Description

Currently #2070 is trying to fix the streaming upload problem. If we fix it "right" (which I think we will), then we might want to consider providing middleware and JavaScript to allow progress bars for uploading.
Instead of lumping it all into one (which it was), it is now broken up and this ticket is the step beyond #2070.

Attachments (13)

5099_upload_middleware_and_oldadmin_js.diff (20.9 KB) - added by Michael Axiak <axiak@…> 7 years ago.
The Middleware and JavaScript
5099-streaming_file_upload_with_safe_file_move_progress.diff (15.6 KB) - added by Øyvind Saltvik <oyvind@…> 7 years ago.
Start of new improved version
5100_admin_progress_bar.diff (4.2 KB) - added by Michael Axiak <axiak@…> 7 years ago.
Pretty Admin Upload Survives!
5116_new_admin_progress_bar.diff (10.3 KB) - added by Michael Axiak <axiak@…> 7 years ago.
New and improved version!
5116_progress_bar_newforms_admin.diff (18.5 KB) - added by Michael Axiak <axiak@…> 7 years ago.
Newforms-Admin branch patch
5116_progress_bar_newforms_admin.2.diff (7.6 KB) - added by Michael Axiak <axiak@…> 7 years ago.
Reworked a lot of JS, this is for newforms-admin.
5126_oldforms_admin_update.diff (7.3 KB) - added by Michael Axiak <axiak@…> 7 years ago.
Reworked a lot of JS, this is for oldforms-admin.
5126_file_upload_contrib_app.diff (6.0 KB) - added by Michael Axiak <axiak@…> 7 years ago.
Wrote some example MiddleWare and such. Using caching makes a lot of sense for upload progress.
5313_admin_and_contrib.diff (19.2 KB) - added by Michael Axiak <axiak@…> 7 years ago.
Works in IE etc
5319_removed_doubles.diff (14.3 KB) - added by Michael Axiak 7 years ago.
removed duplicates
5343_javascript_and_admin.diff (15.9 KB) - added by Michael Axiak <axiak@…> 7 years ago.
New batch of JavaScript and Admin interaction
6602_javascript_and_admin.diff (16.0 KB) - added by Faheem Mitha <faheem@…> 6 years ago.
Update progress bar patch to 6602, with corrections. This should work with 6525_all_tests_pass.diff in issue 2070.
6602_javascript_and_admin.2.diff (16.0 KB) - added by Faheem Mitha <faheem@…> 6 years ago.
Sorry, the previous upload was incorrect.

Download all attachments as: .zip

Change History (43)

Changed 7 years ago by Michael Axiak <axiak@…>

The Middleware and JavaScript

Changed 7 years ago by Øyvind Saltvik <oyvind@…>

Start of new improved version

comment:1 Changed 7 years ago by Michael Axiak <axiak@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Øyvind Saltvik, that last patch doesn't belong here (at least I think).
If you read the description in #2070, it clearly states a problem, and we should fix that problem in that ticket.
I meant for this ticket to be for the stuff *beyond* #2070.
(i.e. middleware and javascript)

The point is that we should be careful in how we design #2070, but instead of doing both at the same time, make #2070 powerful enough that we can design this ticket with a good API (provided in #2070) in mind.

comment:2 Changed 7 years ago by Michael Axiak <axiak@…>

  • Needs documentation set
  • Needs tests set
  • Patch needs improvement set

The above comment was remarking on attachment '5099-streaming_file_upload_with_safe_file_move_progress.diff'.
Also, I didn't mean to clear needs_better_patch or any of the others.

comment:3 Changed 7 years ago by Øyvind Saltvik <oyvind@…>

Ok, I see and agree :)

Changed 7 years ago by Michael Axiak <axiak@…>

Pretty Admin Upload Survives!

comment:4 Changed 7 years ago by Michael Axiak <axiak@…>

Alright, I've cleaned up ideas from #2070 and reformulated to this patch (the JavaScript is left unchanged).
Here's how you can use it:

  1. Add the patch from #2070 (e.g. 5100_file_upload_core.diff:
    patch -p0 < 5100_file_upload_core.diff
    
  1. Add this patch (same as above)
  1. Create the model:
    from django.db import models
    
    # Create your models here.
    class FileList(models.Model):
        name = models.CharField(maxlength=255)
        email = models.EmailField()
    
        class Admin:
            js = ('/admin/media/js/UploadProgress.js',)
    
    class AFile(models.Model):
        descr = models.CharField(maxlength=255,core=True)
        file = models.FileField(upload_to='files')
        inlist = models.ForeignKey(FileList,edit_inline=models.STACKED)
    
  1. Add django.middleware.upload.UploadStateMiddleware to your MIDDLEWARE_CLASSES setting (at the beginning, possibly)
  1. Set FILE_UPLOAD_DIR in your settings file to wherever you want to store temporary files.
  1. Enjoy :)

TODO:

  • Test
  • Write tests/docs
  • Write widget for newforms (that generates uuid)

comment:5 follow-up: Changed 7 years ago by anonymous

Follow above step, not work with

KeyError at /file/upload/
'UPLOAD_PROGRESS_ID'
Request Method: POST
Exception Type: KeyError
Exception Value: 'UPLOAD_PROGRESS_ID'
Exception Location: C:\Python24\lib\site-packages\django\http\init.py in init, line 193

For your info,
-100_file_upload_core.diff
-5100_admin_progress_bar.diff
-Fast cgi mode
Cheer

comment:6 in reply to: ↑ 5 Changed 7 years ago by Michael Axiak <axiak@…>

Replying to anonymous:

Follow above step, not work with
[...]
Cheer

Thank you! It was a line I forgot to add to wsgi.py, should be set now. Please use 5113_file_upload_core_with_middleware_hooks.diff in #2070.

Changed 7 years ago by Michael Axiak <axiak@…>

New and improved version!

comment:7 Changed 7 years ago by Michael Axiak <axiak@…>

  • Cc axiak@… added

With the latest patch, there's a new process for using this:

  1. Add the patch from #2070 (e.g. 5113_file_upload_core_with_middleware_hooks.diff)

patch -p0 < 5113_file_upload_core_with_middleware_hooks.diff

  1. Add this patch:

patch -p0 < 5116_new_admin_progress_bar.diff

  1. Create the model:
    from django.db import models
    
    # Create your models here.
    class FileList(models.Model):
        name = models.CharField(maxlength=255)
        email = models.EmailField()
    
        class Admin:
            pass
    
    class AFile(models.Model):
        descr = models.CharField(maxlength=255,core=True)
        file = models.FileField(upload_to='files')
        inlist = models.ForeignKey(FileList,edit_inline=models.STACKED)
    
  1. Set FILE_UPLOAD_DIR in your settings file to wherever you want to store temporary files.

The changes between this one and the last version:

  • No MiddleWare required. There's no reason to have middleware, the request has to deal with file uploads anyway, it should deal with it in a way that will allow views to use it. (If anyone has any differing opinions, let me know.)
  • No js = () in the class Admin required. It's now an integral part of the oldforms Admin. What's next? newforms-admin.
  • The JavaScript now deals with stopping and restarting uploads in a useful way.

Changed 7 years ago by Michael Axiak <axiak@…>

Newforms-Admin branch patch

comment:8 Changed 7 years ago by Simon G. <dev@…>

  • Triage Stage changed from Unreviewed to Design decision needed

Changed 7 years ago by Michael Axiak <axiak@…>

Reworked a lot of JS, this is for newforms-admin.

Changed 7 years ago by Michael Axiak <axiak@…>

Reworked a lot of JS, this is for oldforms-admin.

Changed 7 years ago by Michael Axiak <axiak@…>

Wrote some example MiddleWare and such. Using caching makes a lot of sense for upload progress.

comment:9 Changed 7 years ago by Michael Axiak <axiak@…>

I reworked the JavaScript from its original carnation. The main goal that I think is reasonably tackled is the case where the server doesn't handle file upload progress well. If it fails, it won't display a broken progress bar, it will just fail mostly silently. The JavaScript is also more a little bit cleaner and more careful.

comment:10 Changed 7 years ago by anonymous

FYI

Not work for IE6,javascript error

Cheer

comment:11 Changed 7 years ago by anonymous

On IE6,

An ajax request is not work, I try this;

function createRequestObject(){

var req = false;


if(window.XMLHttpRequest){

req = new XMLHttpRequest();


if(req.overrideMimeType){ req.overrideMimeType('text/xml'); }

}
else if(window.ActiveXObject){

try{ req = new ActiveXObject("Msxml2.XMLHTTP"); }
catch(e){

try{ req = new ActiveXObject("Microsoft.XMLHTTP"); }
catch(e){}

}

}


if(!req){

return false;

}
else{ return req; }

}

It work for first request and stop after that. The view function never reach by an ajax call.

Any comment?

comment:12 Changed 7 years ago by Michael Axiak <axiak@…>

I am currently using the xmlhttp function given in admin/js/core.js . I don't think that's the problem. I'll look around in IE to see where the problem is and how I can fix it.

comment:13 Changed 7 years ago by anonymous

Hi,

Hoola, I search a google and found that IE not allow calling same uri repeatively, So, I append a random number after uri, it work.

http://www.robertnyman.com/2007/04/04/weird-xmlhttprequest-error-in-ie-just-one-call-allowed/

Thank

Changed 7 years ago by Michael Axiak <axiak@…>

Works in IE etc

comment:14 Changed 7 years ago by Michael Axiak <axiak@…>

  • Keywords progress, ajax added; progress removed

Thank you previous poster (anonymous), adding an incremental (why random?) number after the URI was one of the several changes I've made in order to get this to work in IE (5.5,6, and 7).

Thanks to all who have been testing, this new patch should be a great improvement -- especially for those using IE. However, there are a few things that need to be changed.

Since #2070 has now been stripped of its default FileProgressDescriptor, you need to add some middleware/add models to get this to work.

INSTALLATION

  1. Add #2070 to your source tree
  1. Add the latest patch to your source tree.
  1. Setup your middleware/models to track file progress.
  1. Set FILE_UPLOAD_DIR to a writable folder in your settings.py file.

For (3), there are two options that come in the patch (as uploadprogress contrib):

  • FileProgressStore -- uses the cache framework for quick ajax (will NOT work if your cache framework doesn't)
  • FileProgressDB -- uses Database models

Depending on what you choose, continue below.

FileProgressStore

To enable this, add 'django.contrib.uploadprogress.middleware.FileProgressStore' to your MIDDLEWARE_CLASSES.

FileProgressDB

To enable this:

  • Add 'django.contrib.uploadprogress.middleware.FileProgressStore' to your MIDDLEWARE_CLASSES.
  • Add 'django.contrib.uploadprogress' to your INSTALLED_APPS.
  • run a ./manage.py syncdb

Changed 7 years ago by Michael Axiak

removed duplicates

Changed 7 years ago by Michael Axiak <axiak@…>

New batch of JavaScript and Admin interaction

comment:15 follow-up: Changed 7 years ago by Michael Axiak <axiak@…>

A new version has been uploaded, making a few changes over the last version:

  • The JavaScript will now infer the path of Admin for you. (You can even make admin the root of the domain...)
  • Removed upload_progress}} portions of middleware...now using {{{process_request
  • Raises error whenever client asks for data that's not asked for. This signals the JavaScript to silently fail (and Stop!) after 10 tries.
  • Tested / tweaked for different edge cases regarding what users will have set.
  • Included a missing __init__.py file.

INSTALLATION

The above docs don't really work. Here's a revised version.

  1. Add #2070 to your source tree
  1. Add the latest patch (of this ticket) to your source tree.
  1. Setup your middleware/models to track file progress.
  1. Set FILE_UPLOAD_DIR to a writable folder in your settings.py file.

For (3), there are two options that come in the patch (as uploadprogress contrib):

  • FileProgressCached -- uses the cache framework for quicker ajax (will NOT work if your cache framework doesn't)
  • FileProgressDB -- uses Database models

Depending on what you choose, continue below.

FileProgressCached

To enable this, add 'django.contrib.uploadprogress.middleware.FileProgressCached' to your MIDDLEWARE_CLASSES.

FileProgressDB

To enable this:

  • Add 'django.contrib.uploadprogress.middleware.FileProgressDB' to your MIDDLEWARE_CLASSES.
  • Add 'django.contrib.uploadprogress' to your INSTALLED_APPS.
  • Issue a ./manage.py syncdb

comment:16 Changed 7 years ago by anonymous

I have problem with upload progress with Apache 1.3 + FastCGI:

with request:
http://test-fcgi/admin/upload_progress/?0

i have answer:
Exception Type: OperationalError
Exception Value: (1048, "Column 'progress_id' cannot be null")
Exception Location: c:\web\Python24\lib\site-packages\MySQLdb\connections.py in defaulterrorhandler, line 35

exception in: django\contrib\uploadprogress\middleware\uploaddb.py
in line: self._db_row, created = FileProgress.objects.get_or_create(remote_addr = request.METAREMOTE_ADDR?, progress_id = request.METAUPLOAD_PROGRESS_ID?)

may be request.METAUPLOAD_PROGRESS_ID? is empty ?

comment:17 Changed 7 years ago by anonymous

http://test-fcgi/admin/upload_progress/?0 --> Is this a target location of upload form? It should be a random 32 character upload id. Check you javascript to see what exactly created.

comment:18 follow-up: Changed 7 years ago by anonymous

Ok, my fault. Check your javascript that set http request header (req.setRequestHeader("X-Upload-Id", uuid); )

comment:19 in reply to: ↑ 18 Changed 7 years ago by Michael Axiak <axiak@…>

Replying to anonymous:

Ok, my fault. Check your javascript that set http request header (req.setRequestHeader("X-Upload-Id", uuid); )

Yes. The GET querystring is an incremental number. This changes the URI for each request which supposedly is needed because IE (6?) will not do another async request for the same URI repeatedly if one of them haven't answered yet or IE has "given up" on one of them.

As of the newest release of #2070, the only two ways to specify the upload ID is through the header X-Upload-Id and the GET variable progress_id.

comment:20 in reply to: ↑ 15 Changed 7 years ago by anonymous

By following the instructions in Michael Axiak <axiak@mit.edu> as exactly as possible, using the 5343_javascript_and_admin.diff patch I found that file uploads cause lots of broken pipes with the development server. File uploads DO however seem to work, allthough not with edit_inline. Nevertheless I get no file progress reports.

Is this patch meant to work with the development server at all, should I migrate to Apache and mod_python for my development instead - or am I missing something?

By the way; in django.contrib.uploadprogress there is still an init file missing. ;)

FYI; using Firefox 2, the FileField is in my case a subclassed version of the ImageField (source available on request)

Replying to Michael Axiak <axiak@mit.edu>:

A new version has been uploaded, making a few changes over the last version:

  • The JavaScript will now infer the path of Admin for you. (You can even make admin the root of the domain...)
  • Removed upload_progress}} portions of middleware...now using {{{process_request
  • Raises error whenever client asks for data that's not asked for. This signals the JavaScript to silently fail (and Stop!) after 10 tries.
  • Tested / tweaked for different edge cases regarding what users will have set.
  • Included a missing __init__.py file.

comment:21 Changed 7 years ago by Faheem Mitha <faheem@…>

Hi,

I've been unable to get the most recent patch from #4165 (http://code.djangoproject.com/ticket/4165)
to work. If anyone suggest what I might be doing wrong, or provide a minimal working example, that would be most helpful.

My understanding is that #4165 should work more or less out of the box with the default admin view, once a few configurations have been made. Specifically, I should see an upload progress bar in a model which contains eg a FileField when I try to upload something. Note also that JS is enabled in the browsers I'm using -- I tried a couple of different ones.

So, I have followed the most recent instructions on the wiki as far as I was able. See below.

The model I am using is

extract from models.py

class FileUpload(models.Model):
    upload_date = models.DateTimeField(default=datetime.now(), blank=True, editable=False)
    upload = models.FileField(upload_to="bixfile")
    name = models.CharField(core=True, maxlength=100)
    description = models.CharField(blank=True, maxlength=200)
    folder = models.ForeignKey(FolderUpload, null=True, blank=True, related_name='parentfolder',
edit_inline=models.TABULAR)

    class Admin:
        pass

I checked that a table called uploadprogress_fileprogress is created in the db, as see below

sqlite output

sqlite> .dump uploadprogress_fileprogress
BEGIN TRANSACTION;
CREATE TABLE "uploadprogress_fileprogress" (
    "id" integer NOT NULL PRIMARY KEY,
    "remote_addr" varchar(64) NOT NULL,
    "progress_id" varchar(32) NOT NULL,
    "progress_text" text NOT NULL,
    "last_ts" datetime NOT NULL,
    UNIQUE ("remote_addr", "progress_id")
);

I see the expected listing in the admin view.

admin listing for add file progress

Add file progress

Remote addr:
Progress id:
Last ts:

Date:  Today | Calendar
Time:  Now | Clock

However, there is nothing corresponding to a progress bar in FileUpload. What am I doing wrong, if anything? If you don't have the time or inclination to debug my setup, could you provide a minimal working example?

Thanks, Faheem.

installation notes

INSTALLATION

The above docs don't really work. Here's a revised version.

  1. Add #2070 to your source tree

Used 6469_streaming_file_upload.diff as uploaded to
http://code.djangoproject.com/ticket/2070. When tested separately,
this seems to work fine.

  1. Add the latest patch (of this ticket) to your source tree.

Patched against svn 6469 + 6469_streaming_file_upload.diff, and added an empty
django/contrib/uploadprogress/init.py.

  1. Setup your middleware/models to track file progress.

Did the following as detailed below:

  • Added 'django.contrib.uploadprogress.middleware.FileProgressDB' to my MIDDLEWARE_CLASSES.
  • Added 'django.contrib.uploadprogress' to my INSTALLED_APPS.
  • Issued a ./manage.py syncdb
  1. Set FILE_UPLOAD_DIR to a writable folder in your settings.py file.

Set this as

FILE_UPLOAD_DIR='/tmp'.

For (3), there are two options that come in the patch (as uploadprogress contrib):

  • FileProgressCached -- uses the cache framework for quicker ajax (will NOT > work if your cache framework doesn't)
  • FileProgressDB -- uses Database models

Depending on what you choose, continue below.

See above.

FileProgressCached

To enable this, add 'django.contrib.uploadprogress.middleware.FileProgressCached' to your
MIDDLEWARE_CLASSES.

FileProgressDB

To enable this:

  • Add 'django.contrib.uploadprogress.middleware.FileProgressDB' to your MIDDLEWARE_CLASSES.
  • Add 'django.contrib.uploadprogress' to your INSTALLED_APPS.
  • Issue a ./manage.py syncdb

Changed 6 years ago by Faheem Mitha <faheem@…>

Update progress bar patch to 6602, with corrections. This should work with 6525_all_tests_pass.diff in issue 2070.

Changed 6 years ago by Faheem Mitha <faheem@…>

Sorry, the previous upload was incorrect.

comment:22 Changed 6 years ago by Faheem Mitha <faheem@…>

I got the progress bar patch to work. There are two changes with respect to the previous patch.

1) (Minor): Added missing django/contrib/uploadprogress/init.py.
Note that diff ignores empty files, which may explain why this keeps going missing.

2) For some reason, the pickled object corresponding to self.progress.text in _get_dict in the FileProgress class in models.py is unicode, when it should of course be str. So pickle refuses to unpickle, and wackiness ensues. Well, no progress bar, anyway.

A workaround is to do

pickle.loads(str(self.progress_text))

as reflected in the patch. This isn't very satisfactory, however, so I'm hoping someone can tell me what is the problem here and how to fix it. I was hoping that the recent unicode related fix would take care of this, but apparently not.

Faheem.

comment:23 Changed 6 years ago by jacob

  • Triage Stage changed from Design decision needed to Someday/Maybe

I'm not sure if upload progress in the admin is really needed, so this might want to wait until we think about redesigning the admin interface.

comment:24 Changed 6 years ago by andrewsk

  • Cc prigun@… added

comment:25 Changed 6 years ago by axiak

  • Keywords ajax, 2070-docs added; ajax removed
  • Resolution set to fixed
  • Status changed from new to closed

I'm closing this fixed and marking it as 2070-docs. That is the documentation from the merger of #2070 describes how one can write this outside of Django's core.

I am very close to posting code to http://code.google.com/p/django-uploadutils/ that implements the feature described in this ticket.

Cheers,
Mike

comment:26 Changed 6 years ago by aaronfay

  • Cc aaron@… added

comment:27 Changed 6 years ago by mrts

Mike, where's the promised code :)? As far as I can see, http://code.google.com/p/django-uploadutils/ is empty and dead.

comment:28 Changed 6 years ago by hopson

  • Cc robert.hopson@… added

I would be very interested in helping to test this code as well.

comment:29 Changed 6 years ago by Andre

  • Cc andre.miras@… added

comment:30 Changed 5 years ago by Andre

  • Cc andre.miras@… removed

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.