Merge lp:~lifeless/python-oops-datedir-repo/bug-960775 into lp:python-oops-datedir-repo

Proposed by Robert Collins
Status: Merged
Merged at revision: 37
Proposed branch: lp:~lifeless/python-oops-datedir-repo/bug-960775
Merge into: lp:python-oops-datedir-repo
Diff against target: 123 lines (+57/-5)
5 files modified
NEWS (+7/-0)
oops_datedir_repo/__init__.py (+1/-1)
oops_datedir_repo/repository.py (+10/-3)
oops_datedir_repo/tests/test_repository.py (+38/-0)
setup.py (+1/-1)
To merge this branch: bzr merge lp:~lifeless/python-oops-datedir-repo/bug-960775
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+122580@code.launchpad.net

Description of the change

Handle zero length reports coming in from datedir repos: Some of u1 still use the old generating code that doesn't use .tmp files, so is nonatomic. When their appservers die without flushing - we see 0 length files.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2012-03-01 21:08:51 +0000
3+++ NEWS 2012-09-03 21:21:20 +0000
4@@ -6,6 +6,13 @@
5 NEXT
6 ----
7
8+0.0.18
9+------
10+
11+* DateDirRepo.republish will now ignore empty reports. They are not produced
12+ by the current stack, but old or third party implementations may create them.
13+ (Robert Collins, #960775)
14+
15 0.0.17
16 ------
17
18
19=== modified file 'oops_datedir_repo/__init__.py'
20--- oops_datedir_repo/__init__.py 2012-03-01 21:08:51 +0000
21+++ oops_datedir_repo/__init__.py 2012-09-03 21:21:20 +0000
22@@ -25,7 +25,7 @@
23 # established at this point, and setup.py will use a version of next-$(revno).
24 # If the releaselevel is 'final', then the tarball will be major.minor.micro.
25 # Otherwise it is major.minor.micro~$(revno).
26-__version__ = (0, 0, 17, 'beta', 0)
27+__version__ = (0, 0, 18, 'final', 0)
28
29 __all__ = [
30 'DateDirRepo',
31
32=== modified file 'oops_datedir_repo/repository.py'
33--- oops_datedir_repo/repository.py 2012-02-10 19:24:56 +0000
34+++ oops_datedir_repo/repository.py 2012-09-03 21:21:20 +0000
35@@ -181,9 +181,16 @@
36 os.unlink(candidate)
37 continue
38 with file(candidate, 'rb') as report_file:
39- report = serializer.read(report_file)
40- oopsid = publisher(report)
41- if oopsid:
42+ try:
43+ report = serializer.read(report_file)
44+ except IOError, e:
45+ if e.args[0] == 'Empty OOPS Report':
46+ report = None
47+ else:
48+ raise
49+ if report is not None:
50+ oopsid = publisher(report)
51+ if (report is None and prune) or (report is not None and oopsid):
52 os.unlink(candidate)
53
54 def _datedirs(self):
55
56=== modified file 'oops_datedir_repo/tests/test_repository.py'
57--- oops_datedir_repo/tests/test_repository.py 2012-02-10 19:24:56 +0000
58+++ oops_datedir_repo/tests/test_repository.py 2012-09-03 21:21:20 +0000
59@@ -240,6 +240,23 @@
60 self.assertTrue(os.path.isfile(inprogress_path))
61 self.assertEqual([], oopses)
62
63+ def test_republish_ignores_empty_files(self):
64+ # 0 length files are generated by old versions of oops libraries or
65+ # third party implementations that don't use .tmp staging, and we
66+ # should skip over them..
67+ repo = DateDirRepo(self.useFixture(TempDir()).path, stash_path=True)
68+ report = {}
69+ repo.publish(report)
70+ finished_path = report['datedir_repo_filepath']
71+ # Make the file zero-length.
72+ with open(finished_path, 'wb') as report_file:
73+ os.ftruncate(report_file.fileno(), 0)
74+ oopses = []
75+ publisher = oopses.append
76+ repo.republish(publisher)
77+ self.assertTrue(os.path.isfile(finished_path))
78+ self.assertEqual([], oopses)
79+
80 def test_republish_republishes_and_removes(self):
81 # When a report is republished it is then removed from disk.
82 repo = DateDirRepo(self.useFixture(TempDir()).path, stash_path=True)
83@@ -280,6 +297,27 @@
84 self.assertFalse(os.path.isfile(inprogress_path))
85 self.assertEqual([], oopses)
86
87+ def test_republish_removes_old_empty_files(self):
88+ # 0 length files are generated by old versions of oops libraries or
89+ # third party implementations that don't use .tmp staging, and they
90+ # are unlikely to ever get fleshed out when more than 24 hours old,
91+ # so we prune them.
92+ repo = DateDirRepo(self.useFixture(TempDir()).path)
93+ now = datetime.datetime(2006, 04, 01, 00, 30, 00, tzinfo=utc)
94+ report = {'time': now}
95+ repo.publish(report, now)
96+ dir = repo.root + '/2006-04-01/'
97+ files = os.listdir(dir)
98+ finished_path = dir + files[0]
99+ # Make the file zero-length.
100+ with open(finished_path, 'wb') as report_file:
101+ os.ftruncate(report_file.fileno(), 0)
102+ oopses = []
103+ publisher = oopses.append
104+ repo.republish(publisher)
105+ self.assertFalse(os.path.isfile(finished_path))
106+ self.assertEqual([], oopses)
107+
108 def test_republish_no_error_non_datedir(self):
109 # The present of a non datedir directory in a datedir repo doesn't
110 # break things.
111
112=== modified file 'setup.py'
113--- setup.py 2012-03-01 21:08:51 +0000
114+++ setup.py 2012-09-03 21:21:20 +0000
115@@ -22,7 +22,7 @@
116 description = file(os.path.join(os.path.dirname(__file__), 'README'), 'rb').read()
117
118 setup(name="oops_datedir_repo",
119- version="0.0.17",
120+ version="0.0.18",
121 description="OOPS disk serialisation and repository management.",
122 long_description=description,
123 maintainer="Launchpad Developers",

Subscribers

People subscribed via source and target branches

to all changes: