Streaming Uploads Discussion

[Mike Axiak]

Hello,

It's become clear to me that the API for 2070 [1] (which is currently
a mess :) has never really been discussed by the developers, and I've
been thinking about ways of organizing it such that one can add
features to file uploading without modifying the core.
Here's an overview of what I had in mind.

(Any comments/suggestions/angry cries are greatly appreciated.)

Extremely Visible Changes
================

request.set_upload_handler(<upload_handler>)
--------------------------------------------------------------
This new function will allow one to register a new FileUploadHandler
object to deal with the incoming data.
The API for the FileUploadHandler (and the default child
TemporaryFileUploadHandler) is discussed below.

request.FILES
-----------------
This is no longer a MultiValueDict of raw content, but a
MultiValueDict of UploadedFile objects.
This will probably hurt the most, as there are probably applications
assuming that they can take the content from this dict.
The API of the UploadedFile (and the default child
TemporaryUploadedFile) is discussed below.


APIs for New Objects
=============

FileUploadHandler
-----------------------
Objects of this type must define the following methods:
   1. new_file(filename, content_length) -- A signal that a new file
       has been reached by the parser.
   2. receive_data_chunk(raw_data, start, stop) -- Some data has been
       received by the parser. A non-nonzero return value indicates
that
       the file upload should cease.
   3. file_complete(file_size) -- A signal that the current file has
       completed (no more than one file will ever be dealt with).
       The expected return value is an object of UploadedFile.
   4. upload_complete() -- A signal that all uploads have finished
successfully.
   5. get_chunk_size() -- Returns a number that represents the maximum
number
       of bytes read into memory for each chunk.

By adding a set_upload_handler() method to request, anyone can
override the default upload handler. However, this must be done before
the POST was accessed, and it is probably recommended we raise an
error if someone tries to set a new upload handler after the FILES
MultiValueDict is populated.

The default handler (TemporaryFileUploadHandler) will stream the data
to a temporary file for each part of the upload, and return a
corresponding TemporaryUploadedFile object that represents that data.
In addition, the TemporaryFileUploadHandler can know to stream it to
memory if the content_size is small enough (and start streaming it
into disk if the header lied for some grace period).

UploadedFile
----------------
Objects of this class represent files that were just uploaded and
handled by some handler above. I'm assuming that this will be the most
difficult class to nail, since it has to balance a few different
aspects:
   1. Encapsulating what it means to be a uploaded file.
   2. Making it easy for a developer to send it to a FileBackend and
       have it Do The Right Thing.
   3. Making it easy for a developer to get the content and do
       something with it.

Methods of the UploadedFile:
   1. open() -- Open the file for reading.
   2. read([num_bytes]) -- Read a number of bytes from the file.
   3. close() -- Round out the standard read file operations.
   4. chunk([chunk_size]) -- Generates chunks from the file (if the
       content is in memory already, it should ignore chunk_size).
   5. filename() -- The filename from the content-disposition header.
   6. multiple_chunks([chunk_size]) -- Whether or not you can expect
       multiple chunks from chunk() (Boolean). Useful if you want to
do
       something particularly when you know you can put it all into
       memory at once.
   7. file_size() -- The size in bytes of the uploaded file.

Added method of a TemporaryUploadedFile:
   8. temporary_file_path() -- The whole path (directory and basename)
       of the temporary file on disk. A FileBackend may decide to move
       a file (using the OS move operations) if it has a non-empty
       temporary_file_path.


Notes: After talking with Marty online, it seems that this will work
with the FileBackends, provided that they know how to deal with it.
For instance, if the upload is going to a temporary file, and the
FileBackend is a standard file, then the FileBackend for standard
files should know from temporary_file_path() that it can move, and
perform an OS move operation. Otherwise, the FileBackends should use
the chunk() iterator and do whatever it needs in chunks.

It's interesting to note that with this framework a lot of interesting
possibilities open up. I will not write any of the code to do anything
but the basic temporary disk storage, but here are a few interesting
examples of what can happen:
   - Gzipping data on the fly [GZipFileUploadHandler +
GZipFileBackend].
   - Saving file to another FTP Server on the fly
                                            [FTPFileUploadHandler +
NoOpFileBackend].
   - Having Cool Ajax-y file uploads [AjaxProgressUploadHandler + Any
Backend].
   - Having user-based quotas [QuotaUploadHandler + Any Backend].
   ...

Anyway, the list immediately above isn't what's important right now,
but I'd like to get the API
close enough so we can break things once.

Cheers,
Michael Axiak
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-developers@...
To unsubscribe from this group, send email to django-developers-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---

[Mike Axiak]

One slight modification so far...

On Mar 19, 12:30 am, Mike Axiak <mcax...@...> wrote:

> Hello,
>
> [...]
>
> FileUploadHandler
> -----------------------
> Objects of this type must define the following methods:
>    1. new_file(filename, content_length) -- A signal that a new file
>        has been reached by the parser.
>    2. receive_data_chunk(raw_data, start, stop) -- Some data has been
>        received by the parser. A non-nonzero return value indicates
> that
>        the file upload should cease.

Scratch that return value. You will have to raise StopUpload and
SkipFile to stop the entire upload and skip the file respectively.

>
>
> Cheers,
> Michael Axiak

-Mike
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-developers@...
To unsubscribe from this group, send email to django-developers-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---

[Casper Jensen]

The filename returned by UploadedFile, will that be an unicode object
or a string? Currently request.FILES uses string objects for the
filenames, which results in some problems with uploaded files with
non-latin filenames. See #6009.

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-developers@...
To unsubscribe from this group, send email to django-developers-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---

[Ivan Sagalaev]

Mike Axiak wrote:
> Extremely Visible Changes
> ================
>
> request.set_upload_handler(<upload_handler>)
> --------------------------------------------------------------
> This new function will allow one to register a new FileUploadHandler
> object to deal with the incoming data.
> The API for the FileUploadHandler (and the default child
> TemporaryFileUploadHandler) is discussed below.

I remember long time ago I had a bit different idea.

There are actually two aspects of what happens to an uploaded file:

- its data should be stored somewhere
- a user might want to do some additional processing (count bytes, unzip
it on the fly, resend it to a remote machine)

And I thought that for the first part -- storing it somewhere -- there
shouldn't be actually any handlers, Django should just store it in temp
files indefinitely. However this your proposal may be better. Because,
given my example of unzipping files on the fly, user might not want
event store original file as it is. What do you think about it?

> request.FILES
> -----------------
> This is no longer a MultiValueDict of raw content, but a
> MultiValueDict of UploadedFile objects.
> This will probably hurt the most, as there are probably applications
> assuming that they can take the content from this dict.

I believe this can be made backwards compatible. In my patch[1] to
ticket 1484 (which was duplicated by 2070 long ago) I had a FileDict
class that was lazily fetching file content upon accessing
request.FILES['some_file']['content']. Have a look at it.

>    2. receive_data_chunk(raw_data, start, stop) -- Some data has been
>        received by the parser.

Am I right thinking that raw_data is not a raw data from socket but is
already decoded from whatever content-transfer-encoding it might be in
(i.e. base64)?

>    5. get_chunk_size()

Why not just an attribute chunk_size? It's shorter and it's almost
always will be just a plain class attribute.

> By adding a set_upload_handler() method to request, anyone can
> override the default upload handler. However, this must be done before
> the POST was accessed, and it is probably recommended we raise an
> error if someone tries to set a new upload handler after the FILES
> MultiValueDict is populated.

Instead of having users figure out where to stick a call to
set_upload_handler we could steal the design from middleware and just
have UPLOAD_HANDLERS setting... It might not be such a good idea if
people will often want different logic per view. However I think a
single global setting is needed for the most common use case: store
everything in temp files.

> It's interesting to note that with this framework a lot of interesting
> possibilities open up. I will not write any of the code to do anything
> but the basic temporary disk storage, but here are a few interesting
> examples of what can happen:
>    - Gzipping data on the fly [GZipFileUploadHandler +
> GZipFileBackend].
>    - Saving file to another FTP Server on the fly
>                                             [FTPFileUploadHandler +
> NoOpFileBackend].
>    - Having Cool Ajax-y file uploads [AjaxProgressUploadHandler + Any
> Backend].
>    - Having user-based quotas [QuotaUploadHandler + Any Backend].

Heh :-). I was inventing my use cases before I get to this point :-).

[1]: http://code.djangoproject.com/attachment/ticket/1484/1484.m-r.6.diff

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-developers@...
To unsubscribe from this group, send email to django-developers-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---

[Jacob Kaplan-Moss-3]

Hi Mike --

*Very* well done -- I'm in agreement with nearly every aspect of your
proposal. Major props for taking on such a sticky issue -- walking
into #2070 is a bit like exploring an overgrown jungle :) A few
comments inline below, but in general I quite like your API and would
like to see your code.

On Tue, Mar 18, 2008 at 11:30 PM, Mike Axiak <mcaxiak@...> wrote:
>  request.set_upload_handler(<upload_handler>)

I especially like this -- it neatly encapsulates the fact that
different folks are going to want quite different file upload
behavior. A few things to think about:

* Do you think it's worth firing a signal
(file_upload_started/file_upload_finished) from request or from the
base upload handler? This would let decentralized processing of file
uploads, but could get confusing.
* Do you think we should allow multiple upload handlers (which makes
this call into something like request.add_upload_handler(handler))?
The other option would be to just make folks compose upload handlers
with a wrapper class, which is hardly a hardship.

I don't have answers to either of these questions; something to think
about, at least.

>  request.FILES
>  -----------------
>  This is no longer a MultiValueDict of raw content, but a
>  MultiValueDict of UploadedFile objects.
>  This will probably hurt the most, as there are probably applications
>  assuming that they can take the content from this dict.

It seems to me that you could pretty easily provide a
backwards-compatible API by defining __getitem__ on UploadedFile;
raise a DeprecationWarning there but provide the data in the "old"
style.

>    5. get_chunk_size() -

Just make this FileUploadHandler.chunk_size -- no reason for getters/setters.

Again, thanks -- this is good stuff.

Jacob

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-developers@...
To unsubscribe from this group, send email to django-developers-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---

[Ivan Sagalaev]

Jacob Kaplan-Moss wrote:
>>  request.set_upload_handler(<upload_handler>)
>
> I especially like this -- it neatly encapsulates the fact that
> different folks are going to want quite different file upload
> behavior.

But what about default behaviour? There should be some place to say "all
file uploads should go on disk".

P.S. In my other response we seem to agree on __getitem__ for
request.FILES and an attribute .chunk_size almost exactly :-).

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-developers@...
To unsubscribe from this group, send email to django-developers-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---

[Mike Axiak]

Good question. The issue of encoding is a little murky. As of now, my
(almost working) patch will pretty much try to encode everything
(using force_to_unicode) *except* data in a file upload.
I'm not exactly sure we'll ever support encoding data in file uploads,
but if we do it's definitely a tricky problem.
The issue is two-fold:
   - What files should we encode and which shouldn't we? (do we do
matching on the mimetype? that doesn't sound nice)
   - Dealing with multibyte boundaries in the strings while streaming
the data. (Not a question, just a fun night of work.)

Anyway, I'll go back to ignoring that issue for now unless I get angry
cries for it.

-Mike



On Mar 19, 7:06 am, "Casper Jensen" <t...@...> wrote:
> The filename returned by UploadedFile, will that be an unicode object
> or a string? Currently request.FILES uses string objects for the
> filenames, which results in some problems with uploaded files with
> non-latin filenames. See #6009.
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-developers@...
To unsubscribe from this group, send email to django-developers-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---

[Mike Axiak]

On Mar 19, 7:31 am, Ivan Sagalaev <man...@...> wrote:
> [...]
> Am I right thinking that raw_data is not a raw data from socket but is
> already decoded from whatever content-transfer-encoding it might be in
> (i.e. base64)?

Yes. The idea is to abstract away much of the HTTP work and just get
the raw data after it's been pulled from whatever casing it's in.

> Instead of having users figure out where to stick a call to
> set_upload_handler we could steal the design from middleware and just
> have UPLOAD_HANDLERS setting... It might not be such a good idea if
> people will often want different logic per view. However I think a
> single global setting is needed for the most common use case: store
> everything in temp files.

The idea is that you can set this anywhere, so long as you haven't
accessed POST and FILES yet. So a view will be a place you can do
this.
The TemporaryFileUploadHandler will be default, and is what will
happen "out-of-the-box".
(Well...there's the idea of using the MemoryFileUploadHandler if the
Content-Length exists and is small...but that's beside the point. :)

-Mike

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-developers@...
To unsubscribe from this group, send email to django-developers-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---

[Jacob Kaplan-Moss-3]

On Wed, Mar 19, 2008 at 8:19 AM, Ivan Sagalaev
<maniac@...> wrote:
>  But what about default behaviour? There should be some place to say "all
>  file uploads should go on disk".

I think the default behavior doesn't need to be a setting:
upload-to-disk for all files over N bytes (1M, maybe?), and
upload-to-RAM for anything smaller.

Jacob

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-developers@...
To unsubscribe from this group, send email to django-developers-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---

[Ivan Sagalaev]

Mike Axiak wrote:
> The idea is that you can set this anywhere, so long as you haven't
> accessed POST and FILES yet. So a view will be a place you can do
> this.
> The TemporaryFileUploadHandler will be default, and is what will
> happen "out-of-the-box".

My concern is for this "out-of-the-box" default not to be hard-coded.
There should be some way for a user to set its own global (i.e. non
per-view) handler. I understand that everyone can write a one-line
middleware that sets the handler but while we're at designing stage I
thought we could make it a setting instead of another FAQ :-)

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-developers@...
To unsubscribe from this group, send email to django-developers-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---

[Ivan Sagalaev]

Mike Axiak wrote:
> Good question. The issue of encoding is a little murky. As of now, my
> (almost working) patch will pretty much try to encode everything
> (using force_to_unicode) *except* data in a file upload.

This is the right way. Uploaded file is almost never will be treated as
text and unicode doesn't make sense for binary data. It's also
especially useless for the default behaviour of storing uploaded data to
disk: you'll have to encode just decoded data back to a stream of bytes
to store it in a file.

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-developers@...
To unsubscribe from this group, send email to django-developers-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---

[Ivan Sagalaev]

Jacob Kaplan-Moss wrote:
> I think the default behavior doesn't need to be a setting:
> upload-to-disk for all files over N bytes (1M, maybe?), and
> upload-to-RAM for anything smaller.

Sorry, I didn't mean "default" in a sense of absent user settings. I
want to have a way to set a user-specific handler not only on per-view
basis but as a "default" for all upload requests.


To think of a use-case... Imagine some system where /tmp directory is on
a very small partition. An admin would want to direct all uploads
(potentially big) to some other device. And he wants to do it only for
this Django site, not the whole system. Something like this...

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-developers@...
To unsubscribe from this group, send email to django-developers-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---

[Mikhail Gusarov-3]

Twas brillig at 19:53:00 19.03.2008 UTC+03 when Ivan Sagalaev did gyre and gimble:

 IS> An admin would want to direct all uploads (potentially big) to some
 IS> other device. And he wants to do it only for this Django site, not
 IS> the whole system.

Security also comes in mind: it may be undesirable to use same directory
for different sites to prevent information leakage (even the fact of
file upload may be security-sensitive).

--

[Mike Axiak]

On Mar 19, 12:53 pm, Ivan Sagalaev <man...@...> wrote:
> [...]
> To think of a use-case... Imagine some system where /tmp directory is on
> a very small partition. An admin would want to direct all uploads
> (potentially big) to some other device. And he wants to do it only for
> this Django site, not the whole system. Something like this...

Since this is part of the request, the user is welcome to write a
middleware class that sets this before any view gets started in the
entire project.
Does this not do what you want?

-Mike
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-developers@...
To unsubscribe from this group, send email to django-developers-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---

[Ivan Sagalaev]

Mike Axiak wrote:
> Since this is part of the request, the user is welcome to write a
> middleware class that sets this before any view gets started in the
> entire project.
> Does this not do what you want?

It does. But I was thinking of a setting. However I don't insist on it.
Since you and Jacob seem to not feel that it's necessary let it be just
a method on request. I was merely raising a concern.


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-developers@...
To unsubscribe from this group, send email to django-developers-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---

[Mike Axiak]

The way I see it is that there's no reason we can't add a contrib
middleware later that adds a setting for the default request handler.

-Mike

On Mar 19, 1:49 pm, Ivan Sagalaev <man...@...> wrote:
> Mike Axiak wrote:
> > Since this is part of the request, the user is welcome to write a
> > middleware class that sets this before any view gets started in the
> > entire project.
> > Does this not do what you want?
>
> It does. But I was thinking of a setting. However I don't insist on it.
> Since you and Jacob seem to not feel that it's necessary let it be just
> a method on request. I was merely raising a concern.
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-developers@...
To unsubscribe from this group, send email to django-developers-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---

[Chad Maine]

This looks great.  I expect it will make proper integration with something like Tramline (www.infrae.com/products/tramline) very easy to do but perhaps no longer necessary.

Chad

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-developers@...
To unsubscribe from this group, send email to django-developers-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---

[Mike Axiak]

Now that we actually have a working patch [1], there are a few details
I'd like to raise here.

Major Issues
=======

Supporting dictionaries in form code
------------------------------------
While we can have file objects that support the dictionary lookup to
make those backwards compatible, there's still the API of the forms
code.
As an example, there are a lot of tests that look like::

   TextFileForm(data={'description': u'Assistance'}, files={'file':
{'filename': 'test1.txt', 'content': 'hello world'}})

This is an example of something using a dictionary instead of a
UploadedFile instance when using the forms. However, the forms should
*probably* be using the new fileobject API (use .chunks()/.read()
rather than ['content']), so that would mean they would break if fed a
dictionary like the above example. I see three options to deal with
this:

 1. Leave forms code alone, causing (a) DeprecationWarnings and (b)
the files to be read into memory when saving.
 2. Modify the form code to access the attributes of the
UploadedFile, but on AttributeError use the old dict-style interface
and emit a DeprecationWarning.
 3. Modify form code to use the new UploadedFile object, but just
break on the old dictionary access as it's largely an internal API
anyway.

Without thinking about it, I wrote the patch for (3) and modified the
tests appropriately. I'm now thinking people will probably want (2)
more. The other issue is what we should do with the tests. Should we
leave them? Should we copy them and create a copy of them for the new
style? Should we replace them with the new style?


Having upload_handlers be a list object
---------------------------------------
I know that .set_upload_handler() was really cool, but I've discarded
it to support multiple handlers (see below for details). So now I had
to think of what to replace it with. At first I thought:
.append_upload_handler(), but then I realized that I may want
.insert_upload_handler(), and .delete_upload_handler(). So before long
I realized that I just want people to have a list interface.
Therefore, I decided to just leave it as a plain old list. Thus, to
add an upload handler, you'd just write::

   request.upload_handlers.append(some_upload_handler)

And to replace them all::

   request.upload_handlers = [some_upload_handler]

I've made a few efforts to ensure that it will raise an error if the
upload has already been handled. I know this isn't as simple as the
.set_upload_handler() interface, but I think it's the simplest way we
can support the list modification/replacement in a useful fashion.
What do people think about this?


(Mis)Handling Content-Length
----------------------------
There's probably not much room for argument here, but it's worth
asking a larger group. Currently when you try uploading Large files
(~2GB and greater), you will get a weird Content-Length header (less
than zero, overflowing). The problem is two-fold:
 1. We can't easily exhaust the request data without knowing its
length.
 2. If we don't exhaust the request data, we get a sad Connect Reset
error from the browsers.
This means that even if we wanted to display a useful error page to
people telling them why this happened, it won't happen because we'll
just get Connection Reset errors. We can just ignore it, or we can try
modifying the request streams to set timeouts and exhaust the input.
My perusal of Apache docs tells me that there's discard_request_body,
but unfortunately even this seems to depend on Content-Length. Should
I/we just ignore this and expect people to be sane and upload
reasonable data?

Revised API
===========

I was about to write a huge document that described the API in here.
But then I realized I might as well write it in restructuredtext. Then
I realized I might as well make it formatted like a Django document.
And well...you can see the outcome at:
   http://orodruin.axiak.net/upload_handling.html

Let me know if you have any comments on the API. (Things like how it
deals with multiple handlers could be up for discussion.)

Addendum
========

One last note: In order to test the API and everything, I created a
google code project 'django-uploadutils' [2] which I hope will become
a collection of upload handlers for doing common tasks. If anyone is
interested in helping out with this, I'd be happy to add you as a
contributor.


Regards,
Mike


1: http://code.djangoproject.com/ticket/2070
2: http://code.google.com/p/django-uploadutils/

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-developers@...
To unsubscribe from this group, send email to django-developers-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---

[Ivan Sagalaev]

Mike Axiak wrote:
>  1. Leave forms code alone, causing (a) DeprecationWarnings and (b)
> the files to be read into memory when saving.
>  2. Modify the form code to access the attributes of the
> UploadedFile, but on AttributeError use the old dict-style interface
> and emit a DeprecationWarning.
>  3. Modify form code to use the new UploadedFile object, but just
> break on the old dictionary access as it's largely an internal API
> anyway.

I don't like #1 because there's no point to keep deprecated code in
Django where we can fix it. And #3 is indeed broken because there is
much code in the wild that uses request.FILES directly. It's a public
API after all.

#2 looks reasonable to me.

> I realized that I just want people to have a list interface.
> Therefore, I decided to just leave it as a plain old list. Thus, to
> add an upload handler, you'd just write::
>
>    request.upload_handlers.append(some_upload_handler)
>
> And to replace them all::
>
>    request.upload_handlers = [some_upload_handler]
>
> I've made a few efforts to ensure that it will raise an error if the
> upload has already been handled. I know this isn't as simple as the
> .set_upload_handler() interface, but I think it's the simplest way we
> can support the list modification/replacement in a useful fashion.
> What do people think about this?

It would be good to invent a declarative setting for this but since it's
a per-view thing it would be hard. So may be indeed a list would be enough.

> Currently when you try uploading Large files
> (~2GB and greater), you will get a weird Content-Length header (less
> than zero, overflowing).
> ...
> Should
> I/we just ignore this and expect people to be sane and upload
> reasonable data?

If Content-Length header is crewed then neither we, nor users can do
anything about it. When these file volumes become more widespread I
believe browsers will fix themselves to send correct Content-Length.


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-developers@...
To unsubscribe from this group, send email to django-developers-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---

[Mike Axiak]

Hey,

On Mar 25, 6:04 am, Ivan Sagalaev <man...@...> wrote:
> I don't like #1 because there's no point to keep deprecated code in
> Django where we can fix it. And #3 is indeed broken because there is
> much code in the wild that uses request.FILES directly. It's a public
> API after all.
I thought this would be confused. Currently when you want to upload
something to a form, you would do something along the lines of::

    f = SomeForm(request.POST, request.FILES)

This actually invokes two separate interfaces: the objects inside
request.FILES, and the way the form deals with it. Currently, the form
assumes request.FILES contains dictionaries, and so (like the
documentation) you can feed it dictionaries of data and it will work.
How the form treats this data is the specific interface I'm talking
about, not what request.FILES contains.

Anyway, I'll modify the patch to do #2 anyway. Thanks for the input.

-Mike
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-developers@...
To unsubscribe from this group, send email to django-developers-unsubscribe@...
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---