Merge lp:~matt-goodall/python-oops-datedir-repo/close-oops-tmp into lp:python-oops-datedir-repo

Proposed by Matt Goodall
Status: Merged
Merged at revision: 53
Proposed branch: lp:~matt-goodall/python-oops-datedir-repo/close-oops-tmp
Merge into: lp:python-oops-datedir-repo
Diff against target: 39 lines (+20/-1)
2 files modified
oops_datedir_repo/repository.py (+2/-1)
oops_datedir_repo/tests/test_repository.py (+18/-0)
To merge this branch: bzr merge lp:~matt-goodall/python-oops-datedir-repo/close-oops-tmp
Reviewer Review Type Date Requested Status
James Westby (community) Approve
Review via email: mp+279098@code.launchpad.net

Commit message

Ensure OOPS*.tmp file is closed before it's moved.

Description of the change

The oops is written to `{filename}.tmp` before it's moved to `{filename}`.

This branch ensures the .tmp file is closed before the move, just in case it's the cause of https://pastebin.canonical.com/145077/.

To post a comment you must log in.
Revision history for this message
James Westby (james-w) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'oops_datedir_repo/repository.py'
2--- oops_datedir_repo/repository.py 2015-10-27 16:24:57 +0000
3+++ oops_datedir_repo/repository.py 2015-12-01 12:00:38 +0000
4@@ -122,7 +122,8 @@
5 if self.inherit_id:
6 oopsid = report.get('id') or oopsid
7 report['id'] = oopsid
8- self.serializer.write(report, open(filename + '.tmp', 'wb'))
9+ with open(filename + '.tmp', 'wb') as f:
10+ self.serializer.write(report, f)
11 os.rename(filename + '.tmp', filename)
12 if self.stash_path:
13 original_report['datedir_repo_filepath'] = filename
14
15=== modified file 'oops_datedir_repo/tests/test_repository.py'
16--- oops_datedir_repo/tests/test_repository.py 2015-10-27 16:24:57 +0000
17+++ oops_datedir_repo/tests/test_repository.py 2015-12-01 12:00:38 +0000
18@@ -408,3 +408,21 @@
19 repo = DateDirRepo(self.useFixture(TempDir()).path)
20 repo.publish({'id': '1'})
21 repo.publish({'id': '2'})
22+
23+ def test_oops_tmp_is_closed(self):
24+
25+ # Collect opened OOPS-*.tmp files.
26+ oops_tmp = []
27+ open_real = open
28+ def open_intercept(path, *a, **k):
29+ f = open_real(path, *a, **k)
30+ name = os.path.basename(path)
31+ if name.startswith('OOPS-') and name.endswith('.tmp'):
32+ oops_tmp.append(f)
33+ return f
34+ self.useFixture(MonkeyPatch('__builtin__.open', open_intercept))
35+
36+ repo = DateDirRepo(self.useFixture(TempDir()).path)
37+ repo.publish({'id': '1'})
38+ self.assertEqual(1, len(oops_tmp))
39+ self.assertTrue(oops_tmp[0].closed)

Subscribers

People subscribed via source and target branches

to all changes: