You are viewing a plain text version of this content. The canonical link for it is here.
Posted to mod_python-dev@quetz.apache.org by Jim Gallacher <jp...@jgassociates.ca> on 2005/11/15 05:56:27 UTC
Developer Guidelines (was Re: More efficient FieldStorage, StringField
and Field classes)
Mike Looijmans wrote:
> Inspired by our FieldStorage work with file uploads, I also made some
> implementation changes to Field and StringField in util.py. In essense,
> that will make things like using form['key'] multiple times much more
> efficient. Also, FieldStorage will return the same object if called twice.
>
> I'd like some feedback from the group on these changes, and also some
> directions on what I'd need to do to get them integrated in the
> mod_python beta. I have no experience whatsoever in contributing to open
> source projects.
I think it's too late in the beta cycle to be rewriting code unless it's
to fix a specific, critical bug. We really, really, *really* need to
deliver 3.2 final, and I don't want to mess with working code at this
point. The changes you are suggesting are likely more appropriate for
3.3, or possibly a 3.2 bugfix release.
As far as contributing to mod_python, well, I think you already have the
right idea: propose changes; discuss on python-dev; seek consensus; wait
for your changes to be committed by myself or Nicolas. Bug fixes and
code improvements are much more likely to be accepted than major new
features. We want mod_python to be tightly focused on the core
functionality of providing a python interface to apache. The fancy bells
and whistles are best left to frameworks that utilize mod_python further
up the application stack.
For your specific changes, I would strongly suggest that you make your
changes against the mod_python development branch, ie trunk. Also people
are much more likely to test your changes if you submit a patch rather
than offering a completely new file. This makes it easier to see what
code is being changed and avoids problems with regressions.
I've put together a "Developer's Guide" draft that I'd like to
eventually see included in either the documentation or on the website.
This document doesn't represent any offical position of the mod_python
developers, but rather just what I see as best practices and some
resources for contributors. Everyone should feel free to offer
suggestions to improve this guide. The original is in svn at:
http://svn.apache.org/repos/asf/httpd/mod_python/branches/jgallacher/docs/developers-guide.rst
And no, I have not spell checked it yet. :)
Regards,
Jim
Re: More efficient StringField and Field classes
Posted by Nicolas Lehuen <ni...@gmail.com>.
I've ran the unit test on Windows with your patch and everything seems fine.
Regards,
Nicolas
2005/11/25, Mike Looijmans <nl...@natlab.research.philips.com>:
> Is there a way that I can run the unit tests on a Windows systems? I
> have plenty of Linux boxes here, but I'd have to build and install
> apache in a "private" corner just to run the tests.
>
> Anyway, I see where the problem is, in tests.py it does:
>
> def util_fieldstorage(req):
> from mod_python import util
> req.write(`util.FieldStorage(req).list`)
> return apache.OK
>
> So I'll just add a __repr__ function to the StringField class and the
> test should pass then.
>
> I have attached a patch (on the 3.2.5b version of util.py) to include
> this change. That should fix the unit test, and it did not break any of
> my own code.
>
>
> Jim Gallacher wrote:
> > Hi Mike,
> >
> > I don't have time to dig into this tonight, but your patch causes one of
> > the unit tests to fail.
> >
> > FAIL: test_util_fieldstorage (__main__.PerRequestTestCase)
> > ----------------------------------------------------------------------
> > Traceback (most recent call last):
> > File "test.py", line 1117, in test_util_fieldstorage
> > self.fail(`rsp`)
> > File "/usr/lib/python2.3/unittest.py", line 270, in fail
> > raise self.failureException, msg
> > AssertionError: "['1', '2', '3', '4']"
> >
> >
> > Jim
> >
>
>
> --
> Mike Looijmans
> Philips Natlab / Topic Automation
>
>
> Index: util.py
> ===================================================================
> --- util.py (revision 348746)
> +++ util.py (working copy)
> @@ -48,19 +48,8 @@
> """
>
> class Field:
> - def __init__(self, name, file, ctype, type_options,
> - disp, disp_options, headers = {}):
> + def __init__(self, name):
> self.name = name
> - self.file = file
> - self.type = ctype
> - self.type_options = type_options
> - self.disposition = disp
> - self.disposition_options = disp_options
> - if disp_options.has_key("filename"):
> - self.filename = disp_options["filename"]
> - else:
> - self.filename = None
> - self.headers = headers
>
> def __repr__(self):
> """Return printable representation."""
> @@ -81,13 +70,34 @@
> self.file.close()
>
> class StringField(str):
> - """ This class is basically a string with
> - a value attribute for compatibility with std lib cgi.py
> - """
> + """ This class is basically a string with
> + added attributes for compatibility with std lib cgi.py. Basically, this
> + works the opposite of Field, as it stores its data in a string, but creates
> + a file on demand. Field creates a value on demand and stores data in a file.
> + """
> + filename = None
> + headers = {}
> + ctype = "text/plain"
> + type_options = {}
> + disposition = None
> + disp_options = None
> +
> + # I wanted __init__(name, value) but that does not work (apparently, you
> + # cannot subclass str with a constructor that takes >1 argument)
> + def __init__(self, value):
> + '''Create StringField instance. You'll have to set name yourself.'''
> + str.__init__(self, value)
> + self.value = value
>
> - def __init__(self, str=""):
> - str.__init__(self, str)
> - self.value = self.__str__()
> + def __getattr__(self, name):
> + if name != 'file':
> + raise AttributeError, name
> + self.file = cStringIO.StringIO(self.value)
> + return self.file
> +
> + def __repr__(self):
> + """Return printable representation (to pass unit tests)."""
> + return "Field(%s, %s)" % (`self.name`, `self.value`)
>
> class FieldStorage:
>
> @@ -103,8 +113,7 @@
> if req.args:
> pairs = parse_qsl(req.args, keep_blank_values)
> for pair in pairs:
> - file = cStringIO.StringIO(pair[1])
> - self.list.append(Field(pair[0], file, "text/plain", {}, None, {}))
> + self.add_field(pair[0], pair[1])
>
> if req.method != "POST":
> return
> @@ -123,9 +132,7 @@
> if ctype.startswith("application/x-www-form-urlencoded"):
> pairs = parse_qsl(req.read(clen), keep_blank_values)
> for pair in pairs:
> - # TODO : isn't this a bit heavyweight just for form fields ?
> - file = cStringIO.StringIO(pair[1])
> - self.list.append(Field(pair[0], file, "text/plain", {}, None, {}))
> + self.add_field(pair[0], pair[1])
> return
>
> if not ctype.startswith("multipart/"):
> @@ -205,33 +212,44 @@
> else:
> name = None
>
> + # create a file object
> # is this a file?
> if disp_options.has_key("filename"):
> if file_callback and callable(file_callback):
> file = file_callback(disp_options["filename"])
> else:
> - file = self.make_file()
> + file = tempfile.TemporaryFile("w+b")
> else:
> if field_callback and callable(field_callback):
> file = field_callback()
> else:
> - file = self.make_field()
> + file = cStringIO.StringIO()
>
> # read it in
> - end_of_stream = self.read_to_boundary(req, boundary, file)
> + self.read_to_boundary(req, boundary, file)
> file.seek(0)
> -
> +
> # make a Field
> - field = Field(name, file, ctype, type_options, disp, disp_options, headers)
> -
> + if disp_options.has_key("filename"):
> + field = Field(name)
> + field.filename = disp_options["filename"]
> + else:
> + field = StringField(file.read())
> + field.name = name
> + field.file = file
> + field.type = ctype
> + field.type_options = type_options
> + field.disposition = disp
> + field.disposition_options = disp_options
> + field.headers = headers
> self.list.append(field)
>
> - def make_file(self):
> - return tempfile.TemporaryFile("w+b")
> + def add_field(self, key, value):
> + """Insert a field as key/value pair"""
> + item = StringField(value)
> + item.name = key
> + self.list.append(item)
>
> - def make_field(self):
> - return cStringIO.StringIO()
> -
> def read_to_boundary(self, req, boundary, file):
> previous_delimiter = None
> while True:
> @@ -288,11 +306,7 @@
> found = []
> for item in self.list:
> if item.name == key:
> - if isinstance(item.file, FileType) or \
> - isinstance(getattr(item.file, 'file', None), FileType):
> - found.append(item)
> - else:
> - found.append(StringField(item.value))
> + found.append(item)
> if not found:
> raise KeyError, key
> if len(found) == 1:
> @@ -333,11 +347,7 @@
> """ return the first value received """
> for item in self.list:
> if item.name == key:
> - if isinstance(item.file, FileType) or \
> - isinstance(getattr(item.file, 'file', None), FileType):
> - return item
> - else:
> - return StringField(item.value)
> + return item
> return default
>
> def getlist(self, key):
> @@ -347,11 +357,7 @@
> found = []
> for item in self.list:
> if item.name == key:
> - if isinstance(item.file, FileType) or \
> - isinstance(getattr(item.file, 'file', None), FileType):
> - found.append(item)
> - else:
> - found.append(StringField(item.value))
> + found.append(item)
> return found
>
> def parse_header(line):
>
>
>
Re: More efficient StringField and Field classes
Posted by Mike Looijmans <nl...@natlab.research.philips.com>.
Is there a way that I can run the unit tests on a Windows systems? I
have plenty of Linux boxes here, but I'd have to build and install
apache in a "private" corner just to run the tests.
Anyway, I see where the problem is, in tests.py it does:
def util_fieldstorage(req):
from mod_python import util
req.write(`util.FieldStorage(req).list`)
return apache.OK
So I'll just add a __repr__ function to the StringField class and the
test should pass then.
I have attached a patch (on the 3.2.5b version of util.py) to include
this change. That should fix the unit test, and it did not break any of
my own code.
Jim Gallacher wrote:
> Hi Mike,
>
> I don't have time to dig into this tonight, but your patch causes one of
> the unit tests to fail.
>
> FAIL: test_util_fieldstorage (__main__.PerRequestTestCase)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
> File "test.py", line 1117, in test_util_fieldstorage
> self.fail(`rsp`)
> File "/usr/lib/python2.3/unittest.py", line 270, in fail
> raise self.failureException, msg
> AssertionError: "['1', '2', '3', '4']"
>
>
> Jim
>
--
Mike Looijmans
Philips Natlab / Topic Automation
Re: More efficient StringField and Field classes
Posted by Jim Gallacher <jp...@jgassociates.ca>.
Hi Mike,
I don't have time to dig into this tonight, but your patch causes one of
the unit tests to fail.
FAIL: test_util_fieldstorage (__main__.PerRequestTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
File "test.py", line 1117, in test_util_fieldstorage
self.fail(`rsp`)
File "/usr/lib/python2.3/unittest.py", line 270, in fail
raise self.failureException, msg
AssertionError: "['1', '2', '3', '4']"
Jim
Re: More efficient StringField and Field classes
Posted by Mike Looijmans <nl...@natlab.research.philips.com>.
It has been awhile, but I finally made the patch.
Advice and feedback is welcome.
What it does:
- Simplifies the creation of StringField objects. This was already
marked as a TODO in the 3.2.5b code.
- Does not create a file object (cStringIO) for each and every field.
- FieldStorage.get() will always return the same instance(s) for any
given name.
- FieldStorage.get() is very cheap now (does not create new objects,
which is expensive in Python)
It may break:
- Code that creates an instance of Field or StringField directly. This
can be fixed by extending the __init__() routine, but I don't think it's
needed.
It works:
- Perfect on all of my projects.
Attached:
- Patch created by SVN
--
Mike Looijmans
Philips Natlab / Topic Automation
Re: Developer Guidelines (was Re: More efficient FieldStorage,
StringField and Field classes)
Posted by "Gregory (Grisha) Trubetskoy" <gr...@apache.org>.
On Tue, 15 Nov 2005, Mike Looijmans wrote:
> Then I get:
>
> error: Python was built with version 7.1 of Visual Studio, and extensions
> need to be built with the same version of the compiler, but it isn't
> installed.
>
> I don't have a VS license - that would mean I have to build my own Python in
> order to get any further. Oh dear...
I seem to remember long ago that if you find the code in distutils that
produces this message and comment out that check, it still works even with
incompatible VS's.
Grisha
Re: Developer Guidelines (was Re: More efficient FieldStorage, StringField
and Field classes)
Posted by David Fraser <da...@sjsoft.com>.
Mike Looijmans wrote:
> David Fraser wrote:
>
> > See my other mail - basically you do
> > cd dist
> > set APACHESRC=c:\path\to\apache (where this is the Apache2 dir in a
> > standard installation, now need for apache source)
> > build_installer
>
> Okay, first I downloaded the .NET SDK. (Our internet connection is
> great here)
>
> Then I get:
>
> error: Python was built with version 7.1 of Visual Studio, and
> extensions need to be built with the same version of the compiler, but
> it isn't installed.
>
> I don't have a VS license - that would mean I have to build my own
> Python in order to get any further. Oh dear...
Strange, I have VS.NET 2003 installed here, and it doesn't give errors.
I think I got mod_python compiling at one stage with mingw so that is
the other route you could go...
> I was wondering - since i won't be hackin' the C code (yet), would it
> be sufficient to just install the binary, and use the .py files as
> obtained from the SVN repository?
>
Yes you should be able to install the binary and run the tests from the
repository (if you can get past the issues I had running them ...)
> The 3.2.5 beta seems to run just fine here. Cannot run the automated
> tests though, because of the issues mentioned above.
Cheers
David
Re: Developer Guidelines (was Re: More efficient FieldStorage, StringField
and Field classes)
Posted by Mike Looijmans <nl...@natlab.research.philips.com>.
David Fraser wrote:
> See my other mail - basically you do
> cd dist
> set APACHESRC=c:\path\to\apache (where this is the Apache2 dir in a
> standard installation, now need for apache source)
> build_installer
Okay, first I downloaded the .NET SDK. (Our internet connection is great
here)
Then I get:
error: Python was built with version 7.1 of Visual Studio, and
extensions need to be built with the same version of the compiler, but
it isn't installed.
I don't have a VS license - that would mean I have to build my own
Python in order to get any further. Oh dear...
I was wondering - since i won't be hackin' the C code (yet), would it be
sufficient to just install the binary, and use the .py files as obtained
from the SVN repository?
The 3.2.5 beta seems to run just fine here. Cannot run the automated
tests though, because of the issues mentioned above.
--
Mike Looijmans
Philips Natlab / Topic Automation
Re: Developer Guidelines (was Re: More efficient FieldStorage, StringField
and Field classes)
Posted by David Fraser <da...@sjsoft.com>.
Mike Looijmans wrote:
> Ok, following Jim's advice, an having read his guide, I have so far:
> - Fetched the trunk version with anonymous SVN (no problem)
>
> I've already installed apache 2.0.55 and Python 2.4.1 on this Windows
> machine (the lazy way, with MSI packages).
>
> I've also installed the latest Cygwin (all "default" packages).
>
> However, I'm not getting anywhere in compiling mod_python. It requires
> the apxs tool which is apparently not part of any Apache windows
> distribution. So I grabbed the apache 2.0.55 source for windows, but
> the configure script is now stuck for over half an hour on:
>
> Configuring Apache Portable Runtime Utility library...
>
> checking for APR-util... reconfig
> configuring package in srclib/apr-util now
>
> Nothing appears to happen.
>
> Anyone here who can help me out getting this up and running on Windows
> XP?
See my other mail - basically you do
cd dist
set APACHESRC=c:\path\to\apache (where this is the Apache2 dir in a
standard installation, now need for apache source)
build_installer
Unfortunately I haven't got the tests working yet, but this produces an
installer in dist/dist/
I wonder if it would be a good idea to have a wiki for mod_python, it
can be a nice way of collecting documentation...
David
>
> I can also use linux machines for compile and tests (I'm not root on
> any), but I do most development on this Windows system, so I need a
> windows executable anyway.
>
> PS: Haven't found any speling errors in your document...
>
> Jim Gallacher wrote:
>
>> Mike Looijmans wrote:
>>
>>> Inspired by our FieldStorage work with file uploads, I also made
>>> some implementation changes to Field and StringField in util.py. In
>>> essense, that will make things like using form['key'] multiple times
>>> much more efficient. Also, FieldStorage will return the same object
>>> if called twice.
>>>
>>> I'd like some feedback from the group on these changes, and also
>>> some directions on what I'd need to do to get them integrated in the
>>> mod_python beta. I have no experience whatsoever in contributing to
>>> open source projects.
>>
>>
>>
>> I think it's too late in the beta cycle to be rewriting code unless
>> it's to fix a specific, critical bug. We really, really, *really*
>> need to deliver 3.2 final, and I don't want to mess with working code
>> at this point. The changes you are suggesting are likely more
>> appropriate for 3.3, or possibly a 3.2 bugfix release.
>>
>> As far as contributing to mod_python, well, I think you already have
>> the right idea: propose changes; discuss on python-dev; seek
>> consensus; wait for your changes to be committed by myself or
>> Nicolas. Bug fixes and code improvements are much more likely to be
>> accepted than major new features. We want mod_python to be tightly
>> focused on the core functionality of providing a python interface to
>> apache. The fancy bells and whistles are best left to frameworks that
>> utilize mod_python further up the application stack.
>>
>> For your specific changes, I would strongly suggest that you make your
>> changes against the mod_python development branch, ie trunk. Also people
>> are much more likely to test your changes if you submit a patch rather
>> than offering a completely new file. This makes it easier to see what
>> code is being changed and avoids problems with regressions.
>>
>> I've put together a "Developer's Guide" draft that I'd like to
>> eventually see included in either the documentation or on the
>> website. This document doesn't represent any offical position of the
>> mod_python developers, but rather just what I see as best practices
>> and some resources for contributors. Everyone should feel free to
>> offer suggestions to improve this guide. The original is in svn at:
>> http://svn.apache.org/repos/asf/httpd/mod_python/branches/jgallacher/docs/developers-guide.rst
>>
>>
>> And no, I have not spell checked it yet. :)
>
>
Re: Developer Guidelines (was Re: More efficient FieldStorage, StringField
and Field classes)
Posted by Mike Looijmans <nl...@natlab.research.philips.com>.
Ok, following Jim's advice, an having read his guide, I have so far:
- Fetched the trunk version with anonymous SVN (no problem)
I've already installed apache 2.0.55 and Python 2.4.1 on this Windows
machine (the lazy way, with MSI packages).
I've also installed the latest Cygwin (all "default" packages).
However, I'm not getting anywhere in compiling mod_python. It requires
the apxs tool which is apparently not part of any Apache windows
distribution. So I grabbed the apache 2.0.55 source for windows, but the
configure script is now stuck for over half an hour on:
Configuring Apache Portable Runtime Utility library...
checking for APR-util... reconfig
configuring package in srclib/apr-util now
Nothing appears to happen.
Anyone here who can help me out getting this up and running on Windows XP?
I can also use linux machines for compile and tests (I'm not root on
any), but I do most development on this Windows system, so I need a
windows executable anyway.
PS: Haven't found any speling errors in your document...
Jim Gallacher wrote:
> Mike Looijmans wrote:
>
>> Inspired by our FieldStorage work with file uploads, I also made some
>> implementation changes to Field and StringField in util.py. In
>> essense, that will make things like using form['key'] multiple times
>> much more efficient. Also, FieldStorage will return the same object if
>> called twice.
>>
>> I'd like some feedback from the group on these changes, and also some
>> directions on what I'd need to do to get them integrated in the
>> mod_python beta. I have no experience whatsoever in contributing to
>> open source projects.
>
>
> I think it's too late in the beta cycle to be rewriting code unless it's
> to fix a specific, critical bug. We really, really, *really* need to
> deliver 3.2 final, and I don't want to mess with working code at this
> point. The changes you are suggesting are likely more appropriate for
> 3.3, or possibly a 3.2 bugfix release.
>
> As far as contributing to mod_python, well, I think you already have the
> right idea: propose changes; discuss on python-dev; seek consensus; wait
> for your changes to be committed by myself or Nicolas. Bug fixes and
> code improvements are much more likely to be accepted than major new
> features. We want mod_python to be tightly focused on the core
> functionality of providing a python interface to apache. The fancy bells
> and whistles are best left to frameworks that utilize mod_python further
> up the application stack.
>
> For your specific changes, I would strongly suggest that you make your
> changes against the mod_python development branch, ie trunk. Also people
> are much more likely to test your changes if you submit a patch rather
> than offering a completely new file. This makes it easier to see what
> code is being changed and avoids problems with regressions.
>
> I've put together a "Developer's Guide" draft that I'd like to
> eventually see included in either the documentation or on the website.
> This document doesn't represent any offical position of the mod_python
> developers, but rather just what I see as best practices and some
> resources for contributors. Everyone should feel free to offer
> suggestions to improve this guide. The original is in svn at:
> http://svn.apache.org/repos/asf/httpd/mod_python/branches/jgallacher/docs/developers-guide.rst
>
>
> And no, I have not spell checked it yet. :)
--
Mike Looijmans
Philips Natlab / Topic Automation