Re: mod_python 3.2.3b available for testing
Jim Gallacher wrote: Are you sure there is anything to correct? In both cases, the object has the same methods available for manipulating files (t.write('a'), for example). They are not the same type of object, so they have different dir() output, but don't they have the same functionality? What specifically gets broken in util.FieldStorage? No, I'm not sure. Now that I play around with it I'm not sure I understand the problem at all. Perhaps Nick could elaborate? [...] Other than the fact that "isinstance(t, FileType): returns False, I don't see the problem. Nick? There's 2 issues. I think a documentation change in mod_python where it states that a file object is returned to say a file-like object instead is one. That's easy. The other is that there isn't an easy way to tell whether what you've got in the field is a file or not, because no matter what you've got something that "looks" like a file. Under the current documentation you could have tried isinstance(field.file, file), but that's clearly not the right thing to do since TemporaryFile doesn't necessarily return an actual file object. I accept that, but there needs to be an easy way to know what type the Field object value is -- string or file. Checking filename or disposition is obtuse, and there may not necessarily be a file name even if you get a file. So that's my problem, or at least that's where the conversation has led me. Is there an easy way to figure out what you've got other than process of elimination? Thanks, Nick
Re: mod_python 3.2.3b available for testing
Jorey Bump wrote: Jim Gallacher wrote: Nick wrote: More info: python 2.4.2 on Linux: >>> import tempfile >>> t = tempfile.TemporaryFile() >>> t ', mode 'w+b' at 0xb7df07b8> >>> type(t) >>> dir(t) ['__class__', '__delattr__', '__doc__', '__getattribute__', '__hash__', '__init__', '__iter__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__str__', 'close', 'closed', 'encoding', 'fileno', 'flush', 'isatty', 'mode', 'name', 'newlines', 'next', 'read', 'readinto', 'readline', 'readlines', 'seek', 'softspace', 'tell', 'truncate', 'write', 'writelines', 'xreadlines'] python 2.4.1 on windows: >>> import tempfile >>> t = tempfile.TemporaryFile() >>> t ', mode 'w+b' at 0x0099FBA8> >>> type(t) >>> dir(t) ['__doc__', '__getattr__', '__init__', '__module__', '__repr__', 'close_called', 'file', 'name'] So this is an inconsistency within Python. Should mod_python attempt to correct it, or just claim a Python bug? I think we should correct it. I'm sure users don't care that we implement this with TemporaryFile. That being said, I wonder how many applications on Windows we may break by fixing this? Version 3.1.4 also used TemporaryFile, so this is not a new bug. Are you sure there is anything to correct? In both cases, the object has the same methods available for manipulating files (t.write('a'), for example). They are not the same type of object, so they have different dir() output, but don't they have the same functionality? What specifically gets broken in util.FieldStorage? No, I'm not sure. Now that I play around with it I'm not sure I understand the problem at all. Perhaps Nick could elaborate? Testing with python3.2.3 on Wine: >>> import tempfile >>> from types import * >>> t = tempfile.TemporaryFile() >>> t ', mode 'w+b' at 0x40D6A560> >>> t.file ', mode 'w+b' at 0x40D6A560> >>> t.write('stuff') >>> t.seek(0) >>> t.read() >>> isinstance(t, FileType) False Other than the fact that "isinstance(t, FileType): returns False, I don't see the problem. Nick? Jim
Re: mod_python 3.2.3b available for testing
Indrek Järve wrote: This behaviour has been with Python for quite a while, so claiming it's simply a Python bug will be the same as declaring we don't support Windows. Our company's software that runs on Windows and uses mod_python simply patches util.py with the following change: 227c227 < if isinstance(item.file, FileType): --- > if isinstance(item.file, FileType) or (hasattr(item.file, 'file') and isinstance(item.file.file, FileType)): I haven't tried this with mp32 yet (we're still running on Python 2.3 and I haven't had time to investigate how to compile mp on Windows), but on 3.0/3.1 it appears to work just fine for our customers. The relevant part of FieldStorage has changed in 3.2. isinstance(item.file, FileType) or \ isinstance(getattr(item.file, 'file', None), FileType): so no more patching for you! Now I just need to understand what Nick is on about. ;) Jim Nick wrote: More info: python 2.4.2 on Linux: >>> import tempfile >>> t = tempfile.TemporaryFile() >>> t ', mode 'w+b' at 0xb7df07b8> >>> type(t) >>> dir(t) ['__class__', '__delattr__', '__doc__', '__getattribute__', '__hash__', '__init__', '__iter__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__str__', 'close', 'closed', 'encoding', 'fileno', 'flush', 'isatty', 'mode', 'name', 'newlines', 'next', 'read', 'readinto', 'readline', 'readlines', 'seek', 'softspace', 'tell', 'truncate', 'write', 'writelines', 'xreadlines'] python 2.4.1 on windows: >>> import tempfile >>> t = tempfile.TemporaryFile() >>> t ', mode 'w+b' at 0x0099FBA8> >>> type(t) >>> dir(t) ['__doc__', '__getattr__', '__init__', '__module__', '__repr__', 'close_called', 'file', 'name'] So this is an inconsistency within Python. Should mod_python attempt to correct it, or just claim a Python bug? Nick Nick wrote: This may be a Python Windows thing, but it shows up in mod_python: When using util.FieldStorage on multipart/form-data encoded POST data containing a file, in Linux a field.file will yield a file object (actually a subclass of file), but in Windows you have to get the file object through field.file.file. This probably has something to do with the fact that Windows' implementation of tempfile.TemporaryFile is different from Linux, but it should be made consistent in the mod_python interface. Nick
Re: mod_python 3.2.3b available for testing
Jim Gallacher wrote: You may have misunderstood. I was not suggesting that tempfile.TemporaryFile was introduced in 3.1.4, only that it existed there. Looking at the svn repository I see it's used in 3.0.0-beta and 2.7.9, so this bug has been lurking for a while. ;) Yes, although the fact that the implementation of TemporaryFile changed in Python 2.3 may have something to do with it. I honestly don't remember what the previous behavior was, but it worked OK for me at one time :) Nick
Re: mod_python 3.2.3b available for testing
Nick wrote: Jim Gallacher wrote: So this is an inconsistency within Python. Should mod_python attempt to correct it, or just claim a Python bug? I think we should correct it. I'm sure users don't care that we implement this with TemporaryFile. That being said, I wonder how many applications on Windows we may break by fixing this? Version 3.1.4 also used TemporaryFile, so this is not a new bug. Yeah, I never noticed it either until someone pointed it out to me. I appreciated the change to TemporaryFile, but being primarily a Linux user I never noticed that this broke my code in Windows. In any case, I'm still gonna have to implement a workaround in my own code to catch people using the different versions of mod_python out there, so I can live with whatever decision you guys make. But here's +1 for making the interface consistent at least for mod_python users. As for code breakage, I would consider this a "bug" introduced in 3.1.4, which was the last official release of mod_python, which will be corrected in release 3.3. You may have misunderstood. I was not suggesting that tempfile.TemporaryFile was introduced in 3.1.4, only that it existed there. Looking at the svn repository I see it's used in 3.0.0-beta and 2.7.9, so this bug has been lurking for a while. ;) Regards, Jim
Re: mod_python 3.2.3b available for testing
Well, here's another alternative: provide some other attribute to Field, such as is_file, to determine whether or not the Field is an actual file upload or something else. Because as implemented, the file attribute will always return a file-type object. Nick Nick wrote: Jorey Bump wrote: Are you sure there is anything to correct? In both cases, the object has the same methods available for manipulating files (t.write('a'), for example). They are not the same type of object, so they have different dir() output, but don't they have the same functionality? What specifically gets broken in util.FieldStorage? At a minimum, the documentation: "This is a file object. For file uploads it points to a temporary file. For simple values, it is a StringIO object, so you can read simple string values via this attribute instead of using the value attribute as well." As I stated, it just may be enough to document here that it's a file -or- a file-type object. That's what the Python documentation for tempfile.TemporaryFile states, so maybe calling it a "bug" is wrong. But it's equally simple to return the actual file object, and in my opinion more convenient for programmers to do isinstance(field.file, file) to see if you have an uploaded file vs. some other kind of value. I don't know enough about Windows though to know what will happen if you garbage collect the tempfile._TemporaryFileWrapper object but keeping a reference to the file. It's unfortunate that it's not possible to subclass a builtin type and overload its methods (that I know of). Nick
Re: mod_python 3.2.3b available for testing
Jorey Bump wrote: Are you sure there is anything to correct? In both cases, the object has the same methods available for manipulating files (t.write('a'), for example). They are not the same type of object, so they have different dir() output, but don't they have the same functionality? What specifically gets broken in util.FieldStorage? At a minimum, the documentation: "This is a file object. For file uploads it points to a temporary file. For simple values, it is a StringIO object, so you can read simple string values via this attribute instead of using the value attribute as well." As I stated, it just may be enough to document here that it's a file -or- a file-type object. That's what the Python documentation for tempfile.TemporaryFile states, so maybe calling it a "bug" is wrong. But it's equally simple to return the actual file object, and in my opinion more convenient for programmers to do isinstance(field.file, file) to see if you have an uploaded file vs. some other kind of value. I don't know enough about Windows though to know what will happen if you garbage collect the tempfile._TemporaryFileWrapper object but keeping a reference to the file. It's unfortunate that it's not possible to subclass a builtin type and overload its methods (that I know of). Nick
Re: mod_python 3.2.3b available for testing
Jim Gallacher wrote: Nick wrote: More info: python 2.4.2 on Linux: >>> import tempfile >>> t = tempfile.TemporaryFile() >>> t ', mode 'w+b' at 0xb7df07b8> >>> type(t) >>> dir(t) ['__class__', '__delattr__', '__doc__', '__getattribute__', '__hash__', '__init__', '__iter__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__str__', 'close', 'closed', 'encoding', 'fileno', 'flush', 'isatty', 'mode', 'name', 'newlines', 'next', 'read', 'readinto', 'readline', 'readlines', 'seek', 'softspace', 'tell', 'truncate', 'write', 'writelines', 'xreadlines'] python 2.4.1 on windows: >>> import tempfile >>> t = tempfile.TemporaryFile() >>> t ', mode 'w+b' at 0x0099FBA8> >>> type(t) >>> dir(t) ['__doc__', '__getattr__', '__init__', '__module__', '__repr__', 'close_called', 'file', 'name'] So this is an inconsistency within Python. Should mod_python attempt to correct it, or just claim a Python bug? I think we should correct it. I'm sure users don't care that we implement this with TemporaryFile. That being said, I wonder how many applications on Windows we may break by fixing this? Version 3.1.4 also used TemporaryFile, so this is not a new bug. Are you sure there is anything to correct? In both cases, the object has the same methods available for manipulating files (t.write('a'), for example). They are not the same type of object, so they have different dir() output, but don't they have the same functionality? What specifically gets broken in util.FieldStorage?
Re: mod_python 3.2.3b available for testing
Jim Gallacher wrote: So this is an inconsistency within Python. Should mod_python attempt to correct it, or just claim a Python bug? I think we should correct it. I'm sure users don't care that we implement this with TemporaryFile. That being said, I wonder how many applications on Windows we may break by fixing this? Version 3.1.4 also used TemporaryFile, so this is not a new bug. Yeah, I never noticed it either until someone pointed it out to me. I appreciated the change to TemporaryFile, but being primarily a Linux user I never noticed that this broke my code in Windows. In any case, I'm still gonna have to implement a workaround in my own code to catch people using the different versions of mod_python out there, so I can live with whatever decision you guys make. But here's +1 for making the interface consistent at least for mod_python users. As for code breakage, I would consider this a "bug" introduced in 3.1.4, which was the last official release of mod_python, which will be corrected in release 3.3. Nick
Re: mod_python 3.2.3b available for testing
Nick wrote: More info: python 2.4.2 on Linux: >>> import tempfile >>> t = tempfile.TemporaryFile() >>> t ', mode 'w+b' at 0xb7df07b8> >>> type(t) >>> dir(t) ['__class__', '__delattr__', '__doc__', '__getattribute__', '__hash__', '__init__', '__iter__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__str__', 'close', 'closed', 'encoding', 'fileno', 'flush', 'isatty', 'mode', 'name', 'newlines', 'next', 'read', 'readinto', 'readline', 'readlines', 'seek', 'softspace', 'tell', 'truncate', 'write', 'writelines', 'xreadlines'] python 2.4.1 on windows: >>> import tempfile >>> t = tempfile.TemporaryFile() >>> t ', mode 'w+b' at 0x0099FBA8> >>> type(t) >>> dir(t) ['__doc__', '__getattr__', '__init__', '__module__', '__repr__', 'close_called', 'file', 'name'] So this is an inconsistency within Python. Should mod_python attempt to correct it, or just claim a Python bug? I think we should correct it. I'm sure users don't care that we implement this with TemporaryFile. That being said, I wonder how many applications on Windows we may break by fixing this? Version 3.1.4 also used TemporaryFile, so this is not a new bug. Jim Nick wrote: This may be a Python Windows thing, but it shows up in mod_python: When using util.FieldStorage on multipart/form-data encoded POST data containing a file, in Linux a field.file will yield a file object (actually a subclass of file), but in Windows you have to get the file object through field.file.file. This probably has something to do with the fact that Windows' implementation of tempfile.TemporaryFile is different from Linux, but it should be made consistent in the mod_python interface. Nick
Re: mod_python 3.2.3b available for testing
Right, that's exactly what I'm having to do; I was thinking though that mod_python should present a consistent interface, even if Python doesn't. And, it is a bug in Python, even if it's a documentation bug (which claims that the behavior of fdopen is to return a file object). I disagree that documenting this fact in mod_python without changing the code doesn't amount to saying you don't support Windows. It just means you support Windows to the extent that Python itself does. Nick Indrek Järve wrote: This behaviour has been with Python for quite a while, so claiming it's simply a Python bug will be the same as declaring we don't support Windows. Our company's software that runs on Windows and uses mod_python simply patches util.py with the following change: 227c227 < if isinstance(item.file, FileType): --- > if isinstance(item.file, FileType) or (hasattr(item.file, 'file') and isinstance(item.file.file, FileType)): I haven't tried this with mp32 yet (we're still running on Python 2.3 and I haven't had time to investigate how to compile mp on Windows), but on 3.0/3.1 it appears to work just fine for our customers. Best regards, Indrek Järve Inversion Software OÜ Nick wrote: More info: python 2.4.2 on Linux: >>> import tempfile >>> t = tempfile.TemporaryFile() >>> t ', mode 'w+b' at 0xb7df07b8> >>> type(t) >>> dir(t) ['__class__', '__delattr__', '__doc__', '__getattribute__', '__hash__', '__init__', '__iter__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__str__', 'close', 'closed', 'encoding', 'fileno', 'flush', 'isatty', 'mode', 'name', 'newlines', 'next', 'read', 'readinto', 'readline', 'readlines', 'seek', 'softspace', 'tell', 'truncate', 'write', 'writelines', 'xreadlines'] python 2.4.1 on windows: >>> import tempfile >>> t = tempfile.TemporaryFile() >>> t ', mode 'w+b' at 0x0099FBA8> >>> type(t) >>> dir(t) ['__doc__', '__getattr__', '__init__', '__module__', '__repr__', 'close_called', 'file', 'name'] So this is an inconsistency within Python. Should mod_python attempt to correct it, or just claim a Python bug? Nick Nick wrote: This may be a Python Windows thing, but it shows up in mod_python: When using util.FieldStorage on multipart/form-data encoded POST data containing a file, in Linux a field.file will yield a file object (actually a subclass of file), but in Windows you have to get the file object through field.file.file. This probably has something to do with the fact that Windows' implementation of tempfile.TemporaryFile is different from Linux, but it should be made consistent in the mod_python interface. Nick Jim Gallacher wrote: A new mod_python 3.2.3 beta tarball is now available for testing. A Windows binary is also available. This release is similar to 3.2.2b but fixes a couple a small issues where a non-threaded python is used. Here are the rules: In order for a file to be officially announced, it has to be tested by developers on the dev list. Anyone subscribed to this list can (and should feel obligated to :-) ) test it, and provide feedback *to _this_ list*! (Not the [EMAIL PROTECTED] list, and preferably not me personally). The files are (temporarily) available here: http://www.modpython.org/dist/ Please download it, then do the usual $ ./configure --with-apxs=/wherever/it/is $ make $ (su) # make install Then (as non-root user!) $ cd test $ python test.py And see if any tests fail. If they pass, send a +1 to the list, if they fail, send the details (the versions of OS, Python and Apache, the test output, and suggestions, if any). Thank you, Jim Gallacher