[jira] Commented: (MODPYTHON-93) Improve util.FieldStorage efficiency

["Jim Gallacher (JIRA)" <ji...@apache.org>]

    [ http://issues.apache.org/jira/browse/MODPYTHON-93?page=comments#action_12370996 ] 

Jim Gallacher commented on MODPYTHON-93:
----------------------------------------

I was checking the documentation with respect to this patch and I noticed a couple of things.

The FieldStorage constructor has 2 undocumented parameters, file_callback=None, field_callback=None. These were added in r160259 as part of  Barry Pearce's fix for MODPYTHON-40. In the same patch, the make_file and make_field methods were added. These allow FieldStorage to be subclassed and provide an alternative to using the callbacks.

In Mike's modpython325_util_py_dict.patch, the make_field and make_file methods have be removed, with the reasoning that the callbacks should be sufficient for controlling the upload.

We never had a discussion of the merits of the 2 approaches - subclassing vs passing the callbacks in the FieldStorage constructor. Should we perhaps allow both, and reinstate make_field and make_file, or are the callbacks sufficient?

Further infomation on the callbacks, including some nice examples, can be found  in the python-dev archives. See http://article.gmane.org/gmane.comp.apache.mod-python.devel/760

I'll update the docs as part of the fix for this issue.

> Improve util.FieldStorage efficiency
> ------------------------------------
>
>          Key: MODPYTHON-93
>          URL: http://issues.apache.org/jira/browse/MODPYTHON-93
>      Project: mod_python
>         Type: Improvement
>   Components: core
>     Versions: 3.2.7
>     Reporter: Jim Gallacher
>     Assignee: Jim Gallacher
>     Priority: Minor
>      Fix For: 3.3
>  Attachments: modpython325_util_py_dict.patch
>
> Form fields are saved as a list in a FieldStorage class instance. The class implements a __getitem__ method to provide dict-like behaviour.  This method iterates over the complete list for every call to __getitem__. Applications that need to access all the fields when processing the form will show O(n^2) behaviour where n == the number of form fields. This overhead could be avoided by creating a dict (to use as an index) when the FieldStorage instance is created.
> Mike Looijmans has been investigating StringField and Field as well. It is probably reasonable to include information on his work in this issue as well, so that we can consider all of these efficiency issues in toto.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira

Re: [jira] Commented: (MODPYTHON-93) Improve util.FieldStorage efficiency

[Mike Looijmans <nl...@natlab.research.philips.com>]

> I was checking the documentation with respect to this patch and I noticed a couple of things.
> 
> The FieldStorage constructor has 2 undocumented parameters, file_callback=None, field_callback=None. These were added in r160259 as part of  Barry Pearce's fix for MODPYTHON-40. In the same patch, the make_file and make_field methods were added. These allow FieldStorage to be subclassed and provide an alternative to using the callbacks.
> 
> In Mike's modpython325_util_py_dict.patch, the make_field and make_file methods have be removed, with the reasoning that the callbacks should be sufficient for controlling the upload.
> 
> We never had a discussion of the merits of the 2 approaches - subclassing vs passing the callbacks in the FieldStorage constructor. Should we perhaps allow both, and reinstate make_field and make_file, or are the callbacks sufficient?

The funny thing is that subclassing was my first way of implementing it. 
  Someone beat me to it, so I adopted the callbacks. The structure of 
FieldStorage requires either or both methods. The __init__ constructor 
is doing all the parsing already, so something Pythonic like injecting 
the methods later will not work:
form = util.FieldStorage()
form.make_file = myMakeFileFunction
form.parse(req)

Because of backward compatibility, my vote would be on the callbacks. 
Overriding looks more organized, more structured programming, but when 
you start using the FieldStorage in multiple handlers, providing 
callback functions is much handier for everyday use (otherwise, you'll 
end up with a dozen different FieldStorage classes or mixins for which 
you have to invent names).

> Further infomation on the callbacks, including some nice examples, can be found  in the python-dev archives. See http://article.gmane.org/gmane.comp.apache.mod-python.devel/760
> 
> I'll update the docs as part of the fix for this issue.

Good idea - and a good page too, thanks Barry.