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. ;)

Jim

Reply via email to