launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

Robert Collins has proposed merging lp:~lifeless/python-oops-datedir-repo/bug-960775 into lp:python-oops-datedir-repo.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~lifeless/python-oops-datedir-repo/bug-960775/+merge/122580

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.
-- 
https://code.launchpad.net/~lifeless/python-oops-datedir-repo/bug-960775/+merge/122580
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/python-oops-datedir-repo/bug-960775 into lp:python-oops-datedir-repo.

=== modified file 'NEWS'
--- NEWS	2012-03-01 21:08:51 +0000
+++ NEWS	2012-09-03 21:21:20 +0000
@@ -6,6 +6,13 @@
 NEXT
 ----
 
+0.0.18
+------
+
+* DateDirRepo.republish will now ignore empty reports. They are not produced
+  by the current stack, but old or third party implementations may create them.
+  (Robert Collins, #960775)
+
 0.0.17
 ------
 

=== modified file 'oops_datedir_repo/__init__.py'
--- oops_datedir_repo/__init__.py	2012-03-01 21:08:51 +0000
+++ oops_datedir_repo/__init__.py	2012-09-03 21:21:20 +0000
@@ -25,7 +25,7 @@
 # established at this point, and setup.py will use a version of next-$(revno).
 # If the releaselevel is 'final', then the tarball will be major.minor.micro.
 # Otherwise it is major.minor.micro~$(revno).
-__version__ = (0, 0, 17, 'beta', 0)
+__version__ = (0, 0, 18, 'final', 0)
 
 __all__ = [
     'DateDirRepo',

=== modified file 'oops_datedir_repo/repository.py'
--- oops_datedir_repo/repository.py	2012-02-10 19:24:56 +0000
+++ oops_datedir_repo/repository.py	2012-09-03 21:21:20 +0000
@@ -181,9 +181,16 @@
                         os.unlink(candidate)
                     continue
                 with file(candidate, 'rb') as report_file:
-                    report = serializer.read(report_file)
-                oopsid = publisher(report)
-                if oopsid:
+                    try:
+                        report = serializer.read(report_file)
+                    except IOError, e:
+                        if e.args[0] == 'Empty OOPS Report':
+                            report = None
+                        else:
+                            raise
+                if report is not None:
+                    oopsid = publisher(report)
+                if (report is None and prune) or (report is not None and oopsid):
                     os.unlink(candidate)
 
     def _datedirs(self):

=== modified file 'oops_datedir_repo/tests/test_repository.py'
--- oops_datedir_repo/tests/test_repository.py	2012-02-10 19:24:56 +0000
+++ oops_datedir_repo/tests/test_repository.py	2012-09-03 21:21:20 +0000
@@ -240,6 +240,23 @@
         self.assertTrue(os.path.isfile(inprogress_path))
         self.assertEqual([], oopses)
 
+    def test_republish_ignores_empty_files(self):
+        # 0 length files are generated by old versions of oops libraries or
+        # third party implementations that don't use .tmp staging, and we
+        # should skip over them..
+        repo = DateDirRepo(self.useFixture(TempDir()).path, stash_path=True)
+        report = {}
+        repo.publish(report)
+        finished_path = report['datedir_repo_filepath']
+        # Make the file zero-length.
+        with open(finished_path, 'wb') as report_file:
+            os.ftruncate(report_file.fileno(), 0)
+        oopses = []
+        publisher = oopses.append
+        repo.republish(publisher)
+        self.assertTrue(os.path.isfile(finished_path))
+        self.assertEqual([], oopses)
+
     def test_republish_republishes_and_removes(self):
         # When a report is republished it is then removed from disk.
         repo = DateDirRepo(self.useFixture(TempDir()).path, stash_path=True)
@@ -280,6 +297,27 @@
         self.assertFalse(os.path.isfile(inprogress_path))
         self.assertEqual([], oopses)
 
+    def test_republish_removes_old_empty_files(self):
+        # 0 length files are generated by old versions of oops libraries or
+        # third party implementations that don't use .tmp staging, and they
+        # are unlikely to ever get fleshed out when more than 24 hours old,
+        # so we prune them.
+        repo = DateDirRepo(self.useFixture(TempDir()).path)
+        now = datetime.datetime(2006, 04, 01, 00, 30, 00, tzinfo=utc)
+        report = {'time': now}
+        repo.publish(report, now)
+        dir = repo.root + '/2006-04-01/'
+        files = os.listdir(dir)
+        finished_path = dir + files[0]
+        # Make the file zero-length.
+        with open(finished_path, 'wb') as report_file:
+            os.ftruncate(report_file.fileno(), 0)
+        oopses = []
+        publisher = oopses.append
+        repo.republish(publisher)
+        self.assertFalse(os.path.isfile(finished_path))
+        self.assertEqual([], oopses)
+
     def test_republish_no_error_non_datedir(self):
         # The present of a non datedir directory in a datedir repo doesn't
         # break things.

=== modified file 'setup.py'
--- setup.py	2012-03-01 21:08:51 +0000
+++ setup.py	2012-09-03 21:21:20 +0000
@@ -22,7 +22,7 @@
 description = file(os.path.join(os.path.dirname(__file__), 'README'), 'rb').read()
 
 setup(name="oops_datedir_repo",
-      version="0.0.17",
+      version="0.0.18",
       description="OOPS disk serialisation and repository management.",
       long_description=description,
       maintainer="Launchpad Developers",