Code review comment for lp:~cjwatson/launchpad/refactor-cron-germinate

Revision history for this message
Colin Watson (cjwatson) wrote :

I think you're right that the file shouldn't be put in place if the
context exits with an exception. If I were writing this without context
managers, then rather than:

    with AtomicFile('foo') as handle:
        print >>handle, 'bar'

... I'd probably write:

    handle = open('foo.new', 'w')
    try:
        print >>handle, 'bar'
    finally:
        handle.close()
    os.rename('foo.new', 'foo')

And that's equivalent to the natural expression in (good) shell too:

    set -e
    echo bar >foo.new
    mv foo.new foo

That all suggests to me that skipping the rename on exception, and
leaving the .new file in place so that somebody with access can look at
it and find out how far the script got before it fell over, is a good
thing. I'll change the code to do that.

« Back to merge proposal