launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04541
[Merge] lp:~lifeless/launchpad/useoops into lp:launchpad
Robert Collins has proposed merging lp:~lifeless/launchpad/useoops into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~lifeless/launchpad/useoops/+merge/70995
Delete lp.services.log.uniquefileallocator and the ErrorReport.read method body - both are now in the lp:python-oops project.
To keep from having a --mega-- branch at the end I'm doing a bunch of small point releases as this work progresses.
--
https://code.launchpad.net/~lifeless/launchpad/useoops/+merge/70995
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/useoops into lp:launchpad.
=== modified file 'lib/canonical/launchpad/scripts/ftests/test_oops_prune.py'
--- lib/canonical/launchpad/scripts/ftests/test_oops_prune.py 2010-10-04 19:50:45 +0000
+++ lib/canonical/launchpad/scripts/ftests/test_oops_prune.py 2011-08-10 06:56:09 +0000
@@ -23,6 +23,7 @@
import tempfile
import unittest
+from oops import uniquefileallocator
from pytz import UTC
import transaction
@@ -36,7 +37,6 @@
unwanted_oops_files,
)
from canonical.testing.layers import LaunchpadZopelessLayer
-from lp.services.log import uniquefileallocator
class TestOopsPrune(unittest.TestCase):
=== modified file 'lib/canonical/launchpad/scripts/oops.py'
--- lib/canonical/launchpad/scripts/oops.py 2010-10-26 15:47:24 +0000
+++ lib/canonical/launchpad/scripts/oops.py 2011-08-10 06:56:09 +0000
@@ -18,12 +18,12 @@
import os
import re
+from ...oops import uniquefileallocator
from pytz import utc
from canonical.database.sqlbase import cursor
from canonical.launchpad.webapp.dbpolicy import SlaveOnlyDatabasePolicy
from lp.app.browser.stringformatter import FormattersAPI
-from lp.services.log import uniquefileallocator
def referenced_oops():
=== modified file 'lib/canonical/launchpad/webapp/errorlog.py'
--- lib/canonical/launchpad/webapp/errorlog.py 2011-06-21 13:54:37 +0000
+++ lib/canonical/launchpad/webapp/errorlog.py 2011-08-10 06:56:09 +0000
@@ -20,6 +20,8 @@
import urlparse
from lazr.restful.utils import get_current_browser_request
+from oops.uniquefileallocator import UniqueFileAllocator
+import oops.serializer_rfc822
import pytz
from zope.component.interfaces import ObjectEvent
from zope.error.interfaces import IErrorReportingUtility
@@ -45,7 +47,6 @@
from canonical.launchpad.webapp.vhosts import allvhosts
from canonical.lazr.utils import safe_hasattr
from lp.app import versioninfo
-from lp.services.log.uniquefileallocator import UniqueFileAllocator
from lp.services.timeline.requesttimeline import get_request_timeline
@@ -128,20 +129,6 @@
return True
-def parse_iso8601_date(datestring):
- """Parses a standard ISO 8601 format date, ignoring time zones.
-
- Performs no validation whatsoever. It just plucks up to the first
- 7 numbers from the string and passes them to `datetime.datetime`,
- so would in fact parse any string containing reasonable numbers.
-
- This function can be replaced with `datetime.datetime.strptime()`
- once we move to Python 2.5.
- """
- return datetime.datetime(
- *(int(elem) for elem in re.findall('[0-9]+', datestring)[:7]))
-
-
class ErrorReportEvent(ObjectEvent):
"""A new error report has been created."""
implements(IErrorReportEvent)
@@ -204,54 +191,9 @@
@classmethod
def read(cls, fp):
- msg = rfc822.Message(fp)
- id = msg.getheader('oops-id')
- exc_type = msg.getheader('exception-type')
- exc_value = msg.getheader('exception-value')
- date = parse_iso8601_date(msg.getheader('date'))
- pageid = msg.getheader('page-id')
- username = msg.getheader('user')
- url = msg.getheader('url')
- duration = int(float(msg.getheader('duration', '-1')))
- informational = msg.getheader('informational')
-
- # Explicitly use an iterator so we can process the file
- # sequentially. In most instances the iterator will actually
- # be the file object passed in because file objects should
- # support iteration.
- lines = iter(msg.fp)
-
- # Request variables until the first blank line.
- req_vars = []
- for line in lines:
- line = line.strip()
- if line == '':
- break
- key, value = line.split('=', 1)
- req_vars.append((urllib.unquote(key), urllib.unquote(value)))
-
- # Statements until the next blank line.
- statements = []
- for line in lines:
- line = line.strip()
- if line == '':
- break
- match = re.match(
- r'^(\d+)-(\d+)(?:@([\w-]+))?\s+(.*)', line)
- assert match is not None, (
- "Unable to interpret oops line: %s" % line)
- start, end, db_id, statement = match.groups()
- if db_id is not None:
- db_id = intern(db_id) # This string is repeated lots.
- statements.append(
- (int(start), int(end), db_id, statement))
-
- # The rest is traceback.
- tb_text = ''.join(lines)
-
- return cls(id, exc_type, exc_value, date, pageid, tb_text,
- username, url, duration, req_vars, statements,
- informational)
+ # Deprecated: use the oops module directly now, when possible.
+ report = oops.serializer_rfc822.read(fp)
+ return cls(**report)
class ErrorReportingUtility:
=== modified file 'lib/canonical/launchpad/webapp/tests/test_errorlog.py'
--- lib/canonical/launchpad/webapp/tests/test_errorlog.py 2011-06-20 18:08:45 +0000
+++ lib/canonical/launchpad/webapp/tests/test_errorlog.py 2011-08-10 06:56:09 +0000
@@ -20,6 +20,7 @@
from lazr.batchnavigator.interfaces import InvalidBatchSizeError
from lazr.restful.declarations import error_status
+from oops.uniquefileallocator import UniqueFileAllocator
import pytz
import testtools
from zope.app.publication.tests.test_zopepublication import (
@@ -51,13 +52,12 @@
GoneError,
TranslationUnavailable,
)
-from lp.services.log.uniquefileallocator import UniqueFileAllocator
from lp.services.osutils import remove_tree
from lp.testing import TestCase
from lp_sitecustomize import customize_get_converter
-UTC = pytz.timezone('UTC')
+UTC = pytz.utc
class ArbitraryException(Exception):
@@ -141,6 +141,7 @@
def test_read(self):
"""Test ErrorReport.read()."""
+ # Note: this exists to test the compatibility thunk only.
fp = StringIO.StringIO(dedent("""\
Oops-Id: OOPS-A0001
Exception-Type: NotFound
@@ -163,7 +164,8 @@
self.assertEqual(entry.id, 'OOPS-A0001')
self.assertEqual(entry.type, 'NotFound')
self.assertEqual(entry.value, 'error message')
- self.assertEqual(entry.time, datetime.datetime(2005, 4, 1))
+ self.assertEqual(
+ entry.time, datetime.datetime(2005, 4, 1, tzinfo=UTC))
self.assertEqual(entry.pageid, 'IFoo:+foo-template')
self.assertEqual(entry.tb_text, 'traceback-text')
self.assertEqual(entry.username, 'Sample User')
@@ -183,46 +185,6 @@
entry.db_statements[1],
(5, 10, 'store_b', 'SELECT 2'))
- def test_read_no_store_id(self):
- """Test ErrorReport.read() for old logs with no store_id."""
- fp = StringIO.StringIO(dedent("""\
- Oops-Id: OOPS-A0001
- Exception-Type: NotFound
- Exception-Value: error message
- Date: 2005-04-01T00:00:00+00:00
- Page-Id: IFoo:+foo-template
- User: Sample User
- URL: http://localhost:9000/foo
- Duration: 42
-
- HTTP_USER_AGENT=Mozilla/5.0
- HTTP_REFERER=http://localhost:9000/
- name%3Dfoo=hello%0Aworld
-
- 00001-00005 SELECT 1
- 00005-00010 SELECT 2
-
- traceback-text"""))
- entry = ErrorReport.read(fp)
- self.assertEqual(entry.id, 'OOPS-A0001')
- self.assertEqual(entry.type, 'NotFound')
- self.assertEqual(entry.value, 'error message')
- self.assertEqual(entry.time, datetime.datetime(2005, 4, 1))
- self.assertEqual(entry.pageid, 'IFoo:+foo-template')
- self.assertEqual(entry.tb_text, 'traceback-text')
- self.assertEqual(entry.username, 'Sample User')
- self.assertEqual(entry.url, 'http://localhost:9000/foo')
- self.assertEqual(entry.duration, 42)
- self.assertEqual(len(entry.req_vars), 3)
- self.assertEqual(entry.req_vars[0], ('HTTP_USER_AGENT',
- 'Mozilla/5.0'))
- self.assertEqual(entry.req_vars[1], ('HTTP_REFERER',
- 'http://localhost:9000/'))
- self.assertEqual(entry.req_vars[2], ('name=foo', 'hello\nworld'))
- self.assertEqual(len(entry.db_statements), 2)
- self.assertEqual(entry.db_statements[0], (1, 5, None, 'SELECT 1'))
- self.assertEqual(entry.db_statements[1], (5, 10, None, 'SELECT 2'))
-
class TestErrorReportingUtility(testtools.TestCase):
=== removed file 'lib/lp/services/log/tests/test_uniquefileallocator.py'
--- lib/lp/services/log/tests/test_uniquefileallocator.py 2010-10-26 15:47:24 +0000
+++ lib/lp/services/log/tests/test_uniquefileallocator.py 1970-01-01 00:00:00 +0000
@@ -1,156 +0,0 @@
-# Copyright 2010 Canonical Ltd. This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Tests for the unique log naming facility."""
-
-__metaclass__ = type
-
-import datetime
-import os
-import shutil
-import stat
-import tempfile
-import unittest
-
-import pytz
-import testtools
-
-from lp.services.log.uniquefileallocator import UniqueFileAllocator
-
-
-UTC = pytz.timezone('UTC')
-
-
-class TestUniqueFileAllocator(testtools.TestCase):
-
- def setUp(self):
- super(TestUniqueFileAllocator, self).setUp()
- tempdir = tempfile.mkdtemp()
- self._tempdir = tempdir
- self.addCleanup(shutil.rmtree, tempdir, ignore_errors=True)
-
- def test_setToken(self):
- namer = UniqueFileAllocator("/any-old/path/", 'OOPS', 'T')
- self.assertEqual('T', namer.get_log_infix())
-
- # Some scripts will append a string token to the prefix.
- namer.setToken('CW')
- self.assertEqual('TCW', namer.get_log_infix())
-
- # Some scripts run multiple processes and append a string number
- # to the prefix.
- namer.setToken('1')
- self.assertEqual('T1', namer.get_log_infix())
-
- def assertUniqueFileAllocator(self, namer, now, expected_id,
- expected_last_id, expected_suffix, expected_lastdir):
- logid, filename = namer.newId(now)
- self.assertEqual(logid, expected_id)
- self.assertEqual(filename,
- os.path.join(namer._output_root, expected_suffix))
- self.assertEqual(namer._last_serial, expected_last_id)
- self.assertEqual(namer._last_output_dir,
- os.path.join(namer._output_root, expected_lastdir))
-
- def test_newId(self):
- # TODO: This should return an id, fileobj instead of a file name, to
- # reduce races with threads that are slow to use what they asked for,
- # when combined with configuration changes causing disk scans. That
- # would also permit using a completely stubbed out file system,
- # reducing IO in tests that use UniqueFileAllocator (such as all the
- # pagetests in Launchpad. At that point an interface to obtain a
- # factory of UniqueFileAllocator's would be useful to parameterise the
- # entire test suite.
- namer = UniqueFileAllocator(self._tempdir, 'OOPS', 'T')
- # first name of the day
- self.assertUniqueFileAllocator(namer,
- datetime.datetime(2006, 04, 01, 00, 30, 00, tzinfo=UTC),
- 'OOPS-91T1', 1, '2006-04-01/01800.T1', '2006-04-01')
- # second name of the day
- self.assertUniqueFileAllocator(namer,
- datetime.datetime(2006, 04, 01, 12, 00, 00, tzinfo=UTC),
- 'OOPS-91T2', 2, '2006-04-01/43200.T2', '2006-04-01')
-
- # first name of the following day sets a new dir and the id starts
- # over.
- self.assertUniqueFileAllocator(namer,
- datetime.datetime(2006, 04, 02, 00, 30, 00, tzinfo=UTC),
- 'OOPS-92T1', 1, '2006-04-02/01800.T1', '2006-04-02')
-
- # Setting a token inserts the token into the filename.
- namer.setToken('YYY')
- logid, filename = namer.newId(
- datetime.datetime(2006, 04, 02, 00, 30, 00, tzinfo=UTC))
- self.assertEqual(logid, 'OOPS-92TYYY2')
-
- # Setting a type controls the log id:
- namer.setToken('')
- namer._log_type = "PROFILE"
- logid, filename = namer.newId(
- datetime.datetime(2006, 04, 02, 00, 30, 00, tzinfo=UTC))
- self.assertEqual(logid, 'PROFILE-92T3')
-
- # Native timestamps are not permitted - UTC only.
- now = datetime.datetime(2006, 04, 02, 00, 30, 00)
- self.assertRaises(ValueError, namer.newId, now)
-
- def test_changeErrorDir(self):
- """Test changing the log output dir."""
- namer = UniqueFileAllocator(self._tempdir, 'OOPS', 'T')
-
- # First an id in the original error directory.
- self.assertUniqueFileAllocator(namer,
- datetime.datetime(2006, 04, 01, 00, 30, 00, tzinfo=UTC),
- 'OOPS-91T1', 1, '2006-04-01/01800.T1', '2006-04-01')
-
- # UniqueFileAllocator uses the _output_root attribute to get the
- # current output directory.
- new_output_dir = tempfile.mkdtemp()
- self.addCleanup(shutil.rmtree, new_output_dir, ignore_errors=True)
- namer._output_root = new_output_dir
-
- # Now an id on the same day, in the new directory.
- now = datetime.datetime(2006, 04, 01, 12, 00, 00, tzinfo=UTC)
- log_id, filename = namer.newId(now)
-
- # Since it's a new directory, with no previous logs, the id is 1
- # again, rather than 2.
- self.assertEqual(log_id, 'OOPS-91T1')
- self.assertEqual(namer._last_serial, 1)
- self.assertEqual(namer._last_output_dir,
- os.path.join(new_output_dir, '2006-04-01'))
-
- def test_findHighestSerial(self):
- namer = UniqueFileAllocator(self._tempdir, "OOPS", "T")
- # Creates the dir using now as the timestamp.
- output_dir = namer.output_dir()
- # write some files, in non-serial order.
- open(os.path.join(output_dir, '12343.T1'), 'w').close()
- open(os.path.join(output_dir, '12342.T2'), 'w').close()
- open(os.path.join(output_dir, '12345.T3'), 'w').close()
- open(os.path.join(output_dir, '1234567.T0010'), 'w').close()
- open(os.path.join(output_dir, '12346.A42'), 'w').close()
- open(os.path.join(output_dir, '12346.B100'), 'w').close()
- # The namer should figure out the right highest serial.
- self.assertEqual(namer._findHighestSerial(output_dir), 10)
-
- def test_output_dir_permission(self):
- # Set up default dir creation mode to rwx------.
- umask_permission = stat.S_IRWXG | stat.S_IRWXO
- old_umask = os.umask(umask_permission)
- namer = UniqueFileAllocator(self._tempdir, "OOPS", "T")
- output_dir = namer.output_dir()
- st = os.stat(output_dir)
- # Permission we want here is: rwxr-xr-x
- wanted_permission = (
- stat.S_IRWXU | stat.S_IRGRP | stat.S_IXGRP | stat.S_IROTH |
- stat.S_IXOTH)
- # Get only the permission bits for this directory.
- dir_permission = stat.S_IMODE(st.st_mode)
- self.assertEqual(dir_permission, wanted_permission)
- # Restore the umask to the original value.
- ignored = os.umask(old_umask)
-
-
-def test_suite():
- return unittest.TestLoader().loadTestsFromName(__name__)
=== modified file 'setup.py'
--- setup.py 2011-08-10 01:00:53 +0000
+++ setup.py 2011-08-10 06:56:09 +0000
@@ -56,6 +56,7 @@
'mercurial',
'mocker',
'oauth',
+ 'oops',
'paramiko',
'psycopg2',
'python-memcached',
=== modified file 'versions.cfg'
--- versions.cfg 2011-08-10 01:00:53 +0000
+++ versions.cfg 2011-08-10 06:56:09 +0000
@@ -25,6 +25,7 @@
grokcore.component = 1.6
httplib2 = 0.6.0
ipython = 0.9.1
+iso8601 = 0.1.4
Jinja2 = 2.2
keyring = 0.6.2
launchpadlib = 1.9.9
@@ -48,6 +49,7 @@
mocker = 0.10.1
mozrunner = 1.3.4
oauth = 1.0
+oops = 0.0.1
paramiko = 1.7.4
Paste = 1.7.2
PasteDeploy = 1.3.3
Follow ups
