lib/python/Products/PageTemplateFile.py, line 110, method _cook_check try: mtime=os.stat(self.filename)[8] except: mtime=0 I've just spent an hour or so tracking down an awkward bug in some unit-tests. The true error was being hidden by this bare except. I'd change it, and commit the change, except that I don't know what it is supposed to be catching. Any hints? Actually, I'd rather just remove the try: except: and let os.stat fail if it is going to fail. Any objections? (of course there will be... even bare excepts were put there for some reason...) -- Steve Alexander
Steve Alexander wrote:
lib/python/Products/PageTemplateFile.py, line 110, method _cook_check
try: mtime=os.stat(self.filename)[8] except: mtime=0
I've just spent an hour or so tracking down an awkward bug in some unit-tests. The true error was being hidden by this bare except.
I'd change it, and commit the change, except that I don't know what it is supposed to be catching. Any hints?
os.stat() raises OSError if the file is not found, in which case mtime should be set to 0. There are several ways to fix this; use your judgment. :-) BTW this little idiom is found all over the place, including CMF. Shane
Shane Hathaway wrote:
os.stat() raises OSError if the file is not found, in which case mtime should be set to 0.
Surely if the file is not found, that's an error because the PageTemplateFile is pointing at a source file that doesn't exist. I cannot think of any reason I'd want not to be informed that the source file for a PageTemplateFile isn't there. That's clearly a programming error. So, I suggest removing the try: except: clause entirely, and letting os.stat() raise its error. Or, am I missing something here? -- Steve Alexander
Steve Alexander wrote:
Shane Hathaway wrote:
os.stat() raises OSError if the file is not found, in which case mtime should be set to 0.
Surely if the file is not found, that's an error because the PageTemplateFile is pointing at a source file that doesn't exist.
I cannot think of any reason I'd want not to be informed that the source file for a PageTemplateFile isn't there. That's clearly a programming error.
So, I suggest removing the try: except: clause entirely, and letting os.stat() raise its error.
Or, am I missing something here?
I vaguely recall having a similar discussion with someone regarding DTMLFile, and we decided it had to ignore missing files, but I don't remember why. Also, the open() call just below that line will raise an equivalent exception. Use your judgment--mine is clouded. :-) Shane
Shane Hathaway wrote:
I vaguely recall having a similar discussion with someone regarding DTMLFile, and we decided it had to ignore missing files, but I don't remember why.
Darn... that'll be just the reason I'm looking for!
Also, the open() call just below that line will raise an equivalent exception.
For some reason, the code was not getting there when I came across this problem. I guess this 'if' expression must have evaluated true. if hasattr(self, '_v_program') and mtime == self._v_last_read: return I'm not sure why it would have done so though. -- Steve Alexander
Steve Alexander wrote:
Shane Hathaway wrote:
I vaguely recall having a similar discussion with someone regarding DTMLFile, and we decided it had to ignore missing files, but I don't remember why.
Darn... that'll be just the reason I'm looking for!
Also, the open() call just below that line will raise an equivalent exception.
For some reason, the code was not getting there when I came across this problem. I guess this 'if' expression must have evaluated true.
if hasattr(self, '_v_program') and mtime == self._v_last_read: return
I'm not sure why it would have done so though.
Now I know. _v_program is defined in the PageTemplate base class as None. So, the if statement should be if self._v_program and mtime == self._v_last_read: return If I make that change, then I get an OSError as expected. So, I'll change the bare except: to catch OSError, and change the if statement as described. Great. -- Steve Alexander
Shane Hathaway wrote:
os.stat() raises OSError if the file is not found, in which case mtime should be set to 0. There are several ways to fix this; use your judgment. :-)
The problem is that I've had different exceptions raised on Windows, on ocassion, so I can see why whoever wrote that did it that way :-( cheers, Chris
My guess would be that it would get an OSError possible IndexError, but I'm not sure the logic in just setting the mtime to null and continuing. especially since it will likely just try and fail to open the file a few lines later... This could actually be simplified to: try: mtime = os.path.getmtime(self.filename) except OSError: mtime = 0 -Casey On Wednesday 17 July 2002 07:08 pm, Steve Alexander wrote:
lib/python/Products/PageTemplateFile.py, line 110, method _cook_check
try: mtime=os.stat(self.filename)[8] except: mtime=0
I've just spent an hour or so tracking down an awkward bug in some unit-tests. The true error was being hidden by this bare except.
I'd change it, and commit the change, except that I don't know what it is supposed to be catching. Any hints?
Actually, I'd rather just remove the try: except: and let os.stat fail if it is going to fail. Any objections?
(of course there will be... even bare excepts were put there for some reason...)
-- Steve Alexander
Casey Duncan wrote:
My guess would be that it would get an OSError possible IndexError,
Sounds about right.
but I'm not sure the logic in just setting the mtime to null and continuing.
Right -- this is the part I don't understand. I would prefer os.path.getmtime(self.filename) (or whatever) raised the error, so that I know something isn't set up correctly. Otherwise, with the current behaviour, I just get the page template rendering as empty, which is very confusing. If there is a good reason for the current behaviour, then I want to make the except: clause log a warning. Does anyone own the PageTemplate product at the moment? Oh, and any idea why messages aren't reaching the zope-dev mailing list? According to this page, none of the recent messages have arrived yet. http://lists.zope.org/pipermail/zope-dev/2002-July/date.html -- Steve Alexander
participants (4)
-
Casey Duncan -
Chris Withers -
Shane Hathaway -
Steve Alexander