launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

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.

-- 
https://code.launchpad.net/~cjwatson/launchpad/refactor-cron-germinate/+merge/84624
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/refactor-cron-germinate into lp:launchpad.