Launchpad itself

Merge lp:~stub/launchpad/librarian-gc into lp:launchpad

  1. librarian-gc
  2. Merge into devel
Proposed by Stuart Bishop
Status: Merged
Approved by: Stuart Bishop
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~stub/launchpad/librarian-gc
Merge into: lp:launchpad
Diff against target: 93 lines
2 files modified
lib/canonical/librarian/ftests/test_gc.py (+11/-6)
lib/canonical/librarian/librariangc.py (+6/-4)
To merge this branch: bzr merge lp:~stub/launchpad/librarian-gc
Reviewer Review Type Date Requested Status
Michael Hudson-Doyle Approve
Review via email: mp+13048@code.launchpad.net

Commit message

Ignore well known 'lost+found' directory silently and lower the threat level of some errors from ERROR to WARNING.

To post a comment you must log in.
Revision history for this message
Stuart Bishop (stub) wrote :

The Librarian garbage collection job currently reports an ERROR as it doesn't know how to handle the directory 'lost+found' that exists in the storage area. This directory isn't going away, so this branch makes the job just ignore it - we know it is benign.

This branch also reduces the severity of some errors to warnings.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Looks fine, but I cringe at the sight of yet another MockLogger in the diff :(

review: Approve
Revision history for this message
Stuart Bishop (stub) wrote :

> Looks fine, but I cringe at the sight of yet another MockLogger in the diff :(

This was probably the original MockLogger. All the others are poor imitations :-)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/librarian/ftests/test_gc.py'
--- lib/canonical/librarian/ftests/test_gc.py 2009-10-01 08:02:29 +0000
+++ lib/canonical/librarian/ftests/test_gc.py 2009-10-09 05:06:13 +0000
@@ -31,13 +31,18 @@
3131
3232
33class MockLogger:33class MockLogger:
34 def __init__(self, fail_on_error=True):34 def __init__(self, fail_on_error=True, fail_on_warning=True):
35 self.fail_on_error = fail_on_error35 self.fail_on_error = fail_on_error
36 self.fail_on_warning = fail_on_warning
3637
37 def error(self, *args, **kw):38 def error(self, *args, **kw):
38 if self.fail_on_error:39 if self.fail_on_error:
39 raise RuntimeError("An error was indicated: %r %r" % (args, kw))40 raise RuntimeError("An error was indicated: %r %r" % (args, kw))
4041
42 def warning(self, *args, **kw):
43 if self.fail_on_warning:
44 raise RuntimeError("A warning was indicated: %r %r" % (args, kw))
45
41 def debug(self, *args, **kw):46 def debug(self, *args, **kw):
42 #print '%r %r' % (args, kw)47 #print '%r %r' % (args, kw)
43 pass48 pass
@@ -517,10 +522,10 @@
517 self.failUnless(os.path.exists(path))522 self.failUnless(os.path.exists(path))
518523
519 def test_deleteUnwantedFilesIgnoresNoise(self):524 def test_deleteUnwantedFilesIgnoresNoise(self):
520 # Directories with invalid names in the storage area are ignored.525 # Directories with invalid names in the storage area are
521 # They are reported as errors though, so don't let errors fail526 # ignored. They are reported as warnings though, so don't let
522 # this test.527 # warnings fail this test.
523 librariangc.log = MockLogger(fail_on_error=False)528 librariangc.log = MockLogger(fail_on_warning=False)
524529
525 # Not a hexidecimal number.530 # Not a hexidecimal number.
526 noisedir1_path = os.path.join(config.librarian_server.root, 'zz')531 noisedir1_path = os.path.join(config.librarian_server.root, 'zz')
@@ -574,7 +579,7 @@
574 # There was a bug where delete_unwanted_files() would die579 # There was a bug where delete_unwanted_files() would die
575 # if the last file found on disk was unwanted.580 # if the last file found on disk was unwanted.
576 self.layer.switchDbUser(dbuser='testadmin')581 self.layer.switchDbUser(dbuser='testadmin')
577 content='foo'582 content = 'foo'
578 self.client.addFile(583 self.client.addFile(
579 'foo.txt', len(content), StringIO(content), 'text/plain')584 'foo.txt', len(content), StringIO(content), 'text/plain')
580 # Roll back the database changes, leaving the file on disk.585 # Roll back the database changes, leaving the file on disk.
581586
=== modified file 'lib/canonical/librarian/librariangc.py'
--- lib/canonical/librarian/librariangc.py 2009-09-30 10:46:38 +0000
+++ lib/canonical/librarian/librariangc.py 2009-10-09 05:06:13 +0000
@@ -534,20 +534,22 @@
534 # Ignore known and harmless noise in the Librarian storage area.534 # Ignore known and harmless noise in the Librarian storage area.
535 if 'incoming' in dirnames:535 if 'incoming' in dirnames:
536 dirnames.remove('incoming')536 dirnames.remove('incoming')
537 if 'lost+found' in dirnames:
538 dirnames.remove('lost+found')
537 if 'librarian.pid' in filenames:539 if 'librarian.pid' in filenames:
538 filenames.remove('librarian.pid')540 filenames.remove('librarian.pid')
539541
540 for dirname in dirnames[:]:542 for dirname in dirnames[:]:
541 if len(dirname) != 2:543 if len(dirname) != 2:
542 dirnames.remove(dirname)544 dirnames.remove(dirname)
543 log.error(545 log.warning(
544 "Ignoring directory %s that shouldn't be here" % dirname)546 "Ignoring directory %s that shouldn't be here" % dirname)
545 continue547 continue
546 try:548 try:
547 int(dirname, 16)549 int(dirname, 16)
548 except ValueError:550 except ValueError:
549 dirnames.remove(dirname)551 dirnames.remove(dirname)
550 log.error("Ignoring invalid directory %s" % dirname)552 log.warning("Ignoring invalid directory %s" % dirname)
551553
552 # We need everything in order to ensure we visit files in the554 # We need everything in order to ensure we visit files in the
553 # same order we retrieve wanted files from the database.555 # same order we retrieve wanted files from the database.
@@ -557,7 +559,7 @@
557 # Noise in the storage area, or maybe we are looking at the wrong559 # Noise in the storage area, or maybe we are looking at the wrong
558 # path?560 # path?
559 if dirnames and filenames:561 if dirnames and filenames:
560 log.error(562 log.warning(
561 "%s contains both files %r and subdirectories %r. Skipping."563 "%s contains both files %r and subdirectories %r. Skipping."
562 % (dirpath, filenames, dirnames))564 % (dirpath, filenames, dirnames))
563 continue565 continue
@@ -566,7 +568,7 @@
566 path = os.path.join(dirpath, filename)568 path = os.path.join(dirpath, filename)
567 hex_content_id = ''.join(path.split(os.sep)[-4:])569 hex_content_id = ''.join(path.split(os.sep)[-4:])
568 if hex_content_id_re.search(hex_content_id) is None:570 if hex_content_id_re.search(hex_content_id) is None:
569 log.error(571 log.warning(
570 "Ignoring invalid path %s" % path)572 "Ignoring invalid path %s" % path)
571 continue573 continue
572574