Code review comment for lp:~gary/z3c.recipe.filetemplate/support-system-python

Revision history for this message
Francis J. Lacoste (flacoste) wrote :

On July 13, 2009, Gary Poster wrote:
> In general, the necessity is caused by the mtime precision. On my
> Mac, the precision is 1 second. Here's an example run of a slightly
> modified version of the helper (adding a print), in the interactive
> prompt.
>
> >>> def write_and_wait(dir, *args):
> ... path = os.path.join(dir, *(args[:-1]))
> ... original = os.stat(path).st_mtime
> ... while os.stat(path).st_mtime == original:
> ... print original
> ... f = open(path, 'w')
> ... f.write(args[-1])
> ... f.flush()
> ... os.fsync(f.fileno())
> ... f.close()
> ...
> >>> f = open('/Users/gary/dev/deleteme', 'w')
> >>> f.write('test')
> >>> f.close()
> >>> write_and_wait('/Users/gary/dev/deleteme', 'test')
> 1247536625.0
> >>> write_and_wait('/Users/gary/dev/deleteme', 'test')
> 1247536627.0
> >>> write_and_wait('/Users/gary/dev/deleteme', 'test')
> 1247536628.0
> >>> write_and_wait('/Users/gary/dev/deleteme', 'test')
> 1247536629.0
> 1247536629.0
> 1247536629.0
> [...there are 299 of these...]
> >>>
>
> So, the precision is the point. I tried a variety of things to
> trigger the mtime, but writing an empty string didn't work, and
> directly writing the mtime is OS-specific/dependent, and I didn't
> really want to go there. This is a test harness. Good enough, was my
> thinking.
>
> As far as all the flushing and so on, that's what Jim had for his
> ``write`` helper function, so I suspect there's a point there too. It
> could probably be somewhat refactored but...it's a test harness.

Simply add a docstring explaining that we use this helper to update files to
make sure that the mtime of the file is updated so that buildout pick up the
changes, with a note that the resolution of mtime is system dependant. (And
possibly rename it to update_file() or something like that?)

And instead of busy-waiting, why don't you add a time.sleep(0.5) in there?

Cheers

  review approve

--
Francis J. Lacoste
<email address hidden>

review: Approve

« Back to merge proposal