quetz-mod_python-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jim Gallacher <...@jgassociates.ca>
Subject Re: 3.2.9 release blocker? API breakage for mod_python.util.Field()
Date Thu, 22 Jun 2006 20:00:34 GMT
Max Bowsher wrote:
> MODPYTHON-93, r387693, backported in r393781, changes the API of
> mod_python.util.Field().
> I think that it would be a very bad thing to change an API in an
> incompatible way in a patch release - whilst people are likely to
> understand that things may break going from 3.2.x to 3.3.x, they are
> quite likely to expect an upgrade *within* the 3.2.x series to be
> relatively painless.
> Amongst the applications broken by this, are the 0.9.x current series of
> Trac releases.
> I suggest un-backporting the API-breaking change from the 3.2.x
> maintenance branch.

I agree that we should not make *public* API changes. However quoting 
from the docs for util.Field:

4.6.2 Field class

class Field()

This class is used internally by FieldStorage and is not meant to be 
instantiated by the user.

The parameters passed in the constructor are not documented so I don't 
think users should assume that the api will be invariant. If an 
application such as Trac chooses to ignore the documentation they do so 
at their own peril.

(That was the bad cop speaking. Now over to the good cop.)

On the other hand, to keep Trac and other such apps from breaking we can 
always find a compromise. The changes from 3.2.8 to branches/3.2.x 
currently look like this:
@@ -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."""

So what if we back out of this, but re-factor __init__?
-    def __init__(self, name):
+    def __init__(self, name, file=None, ctype=None, type_options=None,
+               disp=None, disp_options=None, headers = {}):

That change should be compatible with the changes in FieldStorage, but 
should keep the likes of Trac happy. Ain't python grand? :)

Beyond that, the Trac people should either stop using Field directly, or 
advocate that it be designated for public use. I don't personally care 
either way, as long as we have a consistent policy. If something in 
mod_python is marked for internal use, it really does mean it is for 
internal use only.

And of course if the breakage is completely different from what I've 
detailed above, feel free to call me an idiot. ;)


View raw message