Merge lp:~free.ekanayaka/storm/catpure-tracer into lp:storm

[Free Ekanayaka]

This branch adds a storm.tracer.capture function that supports capturing SQL statements into an in-memory log.

[Jamu Kakar (jkakar)]

Nice work, +1!

review: Approve

[lp:~free.ekanayaka/storm/catpure-tracer]

421. By Free Ekanayaka on 2011-10-26

Add a CaptureFixture as testing convenience

422. By Free Ekanayaka on 2011-10-26

Rename class

423. By Free Ekanayaka on 2011-10-26

Rename test

424. By Free Ekanayaka on 2011-10-26

Add a queries property

[Gavin Panella (allenap)]

This looks good, though it's quite complicated. I'm not sure I
understand the need for being able to close a log other than to stop
tracing. Why does add_query need to raise a RuntimeError if it's
closed for example?

The add_query(), is_closed(), count() and queries() methods just seem
a little unnecessary. I don't think is_closed() or the copying in
queries() is really needed (we're all adults, etc.), so a plain list
would suffice.

In that vein, here's my go at something that does a similar job but
with fewer bells and whistles:

    class CaptureTracer(Fixture, BaseStatementTracer):

        def __init__(self):
            super(CaptureTracer, self).__init__()
            self.queries = []

        def setUp(self):
            super(CaptureTracer, self).setUp()
            install_tracer(self)
            self.addCleanup(remove_tracer, self)

        def _expanded_raw_execute(self, conn, raw_cursor, statement):
            self.queries.append(statement)

    # Fixtures can be used as context managers.
    with CaptureTracer() as capture:
         ...
    self.assertEqual([...], capture.queries)

No tests ;)

So, Needs Information for now, and over to you to argue for the
features you've created :)

[1]

+def remove_tracer(tracer):
+ _tracers.remove(tracer)

If tracer is not in _tracers then this will blow up with
ValueError. Perhaps it's more friendly to suppress that?

review: Needs Information

[lp:~free.ekanayaka/storm/catpure-tracer]

425. By Free Ekanayaka on 2011-10-27

Make CaptureTracer a Fixture

[Free Ekanayaka (free.ekanayaka)]

Thanks for reviewing Gavin.

Yeah, I initially made all those classes separate because I thought one might want to use one but not the others, e.g. for not depending on python-fixtures. As my use case are tests and I do want to use python-fixture, I'm fine with what you suggest and changed the code accordingly. If somebody needs support for other use cases it can be added.

[1]

Fixed as you suggest.

[lp:~free.ekanayaka/storm/catpure-tracer]

426. By Free Ekanayaka on 2011-10-27

Update NEWS file

[Gavin Panella (allenap)]

The changes look great.

review: Approve

[lp:~free.ekanayaka/storm/catpure-tracer]

427. By Free Ekanayaka on 2011-10-27

Add missing test

[Free Ekanayaka (free.ekanayaka)]

Merged. Thanks Jamu too, wherever you are :)

=== modified file 'NEWS'
--- NEWS	2011-10-17 18:30:27 +0000
+++ NEWS	2011-10-27 13:27:23 +0000
@@ -4,6 +4,16 @@
 Improvements
 ------------
 
+- A new CaptureTracer has been added to storm.tracer, for capturing all SQL
+  statements executed by Storm. It can be used like this:
+
+    with CaptureTracer() as tracer:
+      # Run queries
+      pass
+    print tracer.queries  # Print all queries run in the context manager block
+
+  You will need the python-fixtures package in order to use this feature.
+
 Bug fixes
 ---------
 

=== modified file 'storm/tracer.py'
--- storm/tracer.py	2011-09-13 07:07:09 +0000
+++ storm/tracer.py	2011-10-27 13:27:23 +0000
@@ -8,6 +8,12 @@
 from storm.exceptions import TimeoutError
 from storm.expr import Variable
 
+try:
+    Fixture = object
+    from fixtures import Fixture
+except ImportError:
+    pass
+
 
 class DebugTracer(object):
 
@@ -16,7 +22,8 @@
             stream = sys.stderr
         self._stream = stream
 
-    def connection_raw_execute(self, connection, raw_cursor, statement, params):
+    def connection_raw_execute(self, connection, raw_cursor, statement,
+                               params):
         time = datetime.now().isoformat()[11:]
         raw_params = []
         for param in params:
@@ -158,7 +165,7 @@
 
     For more information on timelines see the module at
     http://pypi.python.org/pypi/timeline.
-    
+
     The timeline to use is obtained by calling the timeline_factory supplied to
     the constructor. This simple function takes no parameters and returns a
     timeline to use. If it returns None, the tracer is bypassed.
@@ -190,7 +197,7 @@
 
     def connection_raw_execute_success(self, connection, raw_cursor,
                                        statement, params):
-                                       
+
         # action may be None if the tracer was installed after the statement
         # was submitted.
         action = getattr(self.threadinfo, 'action', None)
@@ -205,32 +212,71 @@
             connection, raw_cursor, statement, params)
 
 
+class CaptureTracer(BaseStatementTracer, Fixture):
+    """Trace SQL statements appending them to a C{list}.
+
+    Example:
+
+        with CaptureTracer() as tracer:
+            # Run queries
+        print tracer.queries  # Print the queries that have been run
+
+    @note: This class requires the fixtures package to be available.
+    """
+
+    def __init__(self):
+        super(CaptureTracer, self).__init__()
+        self.queries = []
+
+    def setUp(self):
+        super(CaptureTracer, self).setUp()
+        install_tracer(self)
+        self.addCleanup(remove_tracer, self)
+
+    def _expanded_raw_execute(self, conn, raw_cursor, statement):
+        """Save the statement to the log."""
+        self.queries.append(statement)
+
+
 _tracers = []
 
+
 def trace(name, *args, **kwargs):
     for tracer in _tracers:
         attr = getattr(tracer, name, None)
         if attr:
             attr(*args, **kwargs)
 
+
 def install_tracer(tracer):
     _tracers.append(tracer)
 
+
 def get_tracers():
     return _tracers[:]
 
+
 def remove_all_tracers():
     del _tracers[:]
 
+
+def remove_tracer(tracer):
+    try:
+        _tracers.remove(tracer)
+    except ValueError:
+        pass  # The tracer is not installed, succeed gracefully
+
+
 def remove_tracer_type(tracer_type):
-    for i in range(len(_tracers)-1, -1, -1):
+    for i in range(len(_tracers) - 1, -1, -1):
         if type(_tracers[i]) is tracer_type:
             del _tracers[i]
 
+
 def debug(flag, stream=None):
     remove_tracer_type(DebugTracer)
     if flag:
         install_tracer(DebugTracer(stream=stream))
 
-# Deal with circular import.        
+# Deal with circular import.
 from storm.database import convert_param_marks

=== modified file 'tests/tracer.py'
--- tests/tracer.py	2011-09-13 07:07:09 +0000
+++ tests/tracer.py	2011-10-27 13:27:23 +0000
@@ -3,15 +3,26 @@
 from unittest import TestCase
 
 try:
+    # Optional dependency, if missing Fixture tests are skipped.
+    TestWithFixtures = object
+    from fixtures.testcase import TestWithFixtures
+except ImportError:
+    has_fixtures = False
+else:
+    has_fixtures = True
+
+try:
     # Optional dependency, if missing TimelineTracer tests are skipped.
     import timeline
+    has_timeline = True
 except ImportError:
-    timeline = None
+    has_timeline = False
 
-from storm.tracer import (trace, install_tracer, get_tracers,
+from storm.tracer import (trace, install_tracer, get_tracers, remove_tracer,
                           remove_tracer_type, remove_all_tracers, debug,
                           BaseStatementTracer, DebugTracer, TimeoutTracer,
-                          TimelineTracer, TimeoutError, _tracers)
+                          TimelineTracer, TimeoutError, CaptureTracer,
+                          _tracers)
 from storm.database import Connection
 from storm.expr import Variable
 
@@ -36,9 +47,28 @@
         remove_all_tracers()
         self.assertEquals(get_tracers(), [])
 
+    def test_remove_tracer(self):
+        """The C{remote_tracer} function removes a specific tracer."""
+        tracer1 = object()
+        tracer2 = object()
+        install_tracer(tracer1)
+        install_tracer(tracer2)
+        remove_tracer(tracer1)
+        self.assertEquals(get_tracers(), [tracer2])
+
+    def test_remove_tracer_with_not_installed_tracer(self):
+        """C{remote_tracer} exits gracefully if the tracer is not installed."""
+        tracer = object()
+        remove_tracer(tracer)
+        self.assertEquals(get_tracers(), [])
+
     def test_remove_tracer_type(self):
-        class C(object): pass
-        class D(C): pass
+        class C(object):
+            pass
+
+        class D(C):
+            pass
+
         c = C()
         d1 = D()
         d2 = D()
@@ -69,9 +99,11 @@
 
     def test_trace(self):
         stash = []
+
         class Tracer(object):
             def m1(_, *args, **kwargs):
                 stash.extend(["m1", args, kwargs])
+
             def m2(_, *args, **kwargs):
                 stash.extend(["m2", args, kwargs])
 
@@ -82,7 +114,6 @@
         self.assertEquals(stash, ["m1", (1, 2), {"c": 3}, "m2", (), {}])
 
 
-
 class MockVariable(Variable):
 
     def __init__(self, value):
@@ -101,7 +132,7 @@
 
         datetime_mock = self.mocker.replace("datetime.datetime")
         datetime_mock.now()
-        self.mocker.result(datetime.datetime(1,2,3,4,5,6,7))
+        self.mocker.result(datetime.datetime(1, 2, 3, 4, 5, 6, 7))
         self.mocker.count(0, 1)
 
         self.variable = MockVariable("PARAM")
@@ -192,7 +223,9 @@
 
         # Some data is kept in the connection, so we use a proxy to
         # allow things we don't care about here to happen.
-        class Connection(object): pass
+        class Connection(object):
+            pass
+
         self.connection = self.mocker.proxy(Connection())
 
     def tearDown(self):
@@ -330,6 +363,14 @@
         self.execute()
 
 
+class StubConnection(Connection):
+
+    def __init__(self):
+        self._database = None
+        self._event = None
+        self._raw_connection = None
+
+
 class BaseStatementTracerTest(TestCase):
 
     class LoggingBaseStatementTracer(BaseStatementTracer):
@@ -337,13 +378,6 @@
             self.__dict__.setdefault('calls', []).append(
                 (connection, raw_cursor, statement))
 
-    class StubConnection(Connection):
-
-        def __init__(self):
-            self._database = None
-            self._event = None
-            self._raw_connection = None
-
     def test_no_params(self):
         """With no parameters the statement is passed through verbatim."""
         tracer = self.LoggingBaseStatementTracer()
@@ -352,7 +386,7 @@
 
     def test_params_substituted_pyformat(self):
         tracer = self.LoggingBaseStatementTracer()
-        conn = self.StubConnection()
+        conn = StubConnection()
         conn.param_mark = '%s'
         var1 = MockVariable(u'VAR1')
         tracer.connection_raw_execute(
@@ -364,7 +398,7 @@
     def test_params_substituted_single_string(self):
         """String parameters are formatted as a single quoted string."""
         tracer = self.LoggingBaseStatementTracer()
-        conn = self.StubConnection()
+        conn = StubConnection()
         var1 = MockVariable(u'VAR1')
         tracer.connection_raw_execute(
             conn, 'cursor', 'SELECT * FROM person where name = ?', [var1])
@@ -375,7 +409,7 @@
     def test_qmark_percent_s_literal_preserved(self):
         """With ? parameters %s in the statement can be kept intact."""
         tracer = self.LoggingBaseStatementTracer()
-        conn = self.StubConnection()
+        conn = StubConnection()
         var1 = MockVariable(1)
         tracer.connection_raw_execute(
             conn, 'cursor',
@@ -388,7 +422,7 @@
     def test_int_variable_as_int(self):
         """Int parameters are formatted as an int literal."""
         tracer = self.LoggingBaseStatementTracer()
-        conn = self.StubConnection()
+        conn = StubConnection()
         var1 = MockVariable(1)
         tracer.connection_raw_execute(
             conn, 'cursor', "SELECT * FROM person where id = ?", [var1])
@@ -399,7 +433,7 @@
     def test_like_clause_preserved(self):
         """% operators in LIKE statements are preserved."""
         tracer = self.LoggingBaseStatementTracer()
-        conn = self.StubConnection()
+        conn = StubConnection()
         var1 = MockVariable(u'substring')
         tracer.connection_raw_execute(
             conn, 'cursor',
@@ -412,7 +446,7 @@
 
     def test_unformattable_statements_are_handled(self):
         tracer = self.LoggingBaseStatementTracer()
-        conn = self.StubConnection()
+        conn = StubConnection()
         var1 = MockVariable(u'substring')
         tracer.connection_raw_execute(
             conn, 'cursor', "%s %s",
@@ -426,14 +460,14 @@
 class TimelineTracerTest(TestHelper):
 
     def is_supported(self):
-        return timeline is not None
+        return has_timeline
 
     def factory(self):
         self.timeline = timeline.Timeline()
         return self.timeline
 
     def test_separate_tracers_own_state(self):
-        """"Check that multiple TimelineTracer's could be used at once."""
+        """Check that multiple TimelineTracer's could be used at once."""
         tracer1 = TimelineTracer(self.factory)
         tracer2 = TimelineTracer(self.factory)
         tracer1.threadinfo.action = 'foo'
@@ -457,8 +491,7 @@
 
     def test_finds_timeline_from_factory(self):
         factory_result = timeline.Timeline()
-        factory = lambda:factory_result
-        tracer = TimelineTracer(lambda:factory_result)
+        tracer = TimelineTracer(lambda: factory_result)
         tracer._expanded_raw_execute('conn', 'cursor', 'statement')
         self.assertEqual(1, len(factory_result.actions))
 
@@ -491,3 +524,73 @@
         tracer = TimelineTracer(self.factory)
         tracer._expanded_raw_execute('conn', 'cursor', 'statement')
         self.assertEqual('SQL-<unknown>', self.timeline.actions[-1].category)
+
+
+class CaptureTracerTest(TestHelper, TestWithFixtures):
+
+    def is_supported(self):
+        return has_fixtures
+
+    def tearDown(self):
+        super(CaptureTracerTest, self).tearDown()
+        del _tracers[:]
+
+    def test_capture(self):
+        """
+        Using the L{CaptureTracer} fixture starts capturing queries and stops
+        removes the tracer upon cleanup.
+        """
+        tracer = self.useFixture(CaptureTracer())
+        self.assertEqual([tracer], get_tracers())
+        conn = StubConnection()
+        conn.param_mark = '%s'
+        var = MockVariable(u"var")
+        tracer.connection_raw_execute(conn, "cursor", "select %s", [var])
+        self.assertEqual(["select 'var'"], tracer.queries)
+
+        def check():
+            self.assertEqual([], get_tracers())
+
+        self.addCleanup(check)
+
+    def test_capture_as_context_manager(self):
+        """{CaptureTracer}s can be used as context managers."""
+        conn = StubConnection()
+        with CaptureTracer() as tracer:
+            self.assertEqual([tracer], get_tracers())
+            tracer.connection_raw_execute(conn, "cursor", "select", [])
+        self.assertEqual([], get_tracers())
+        self.assertEqual(["select"], tracer.queries)
+
+    def test_capture_multiple(self):
+        """L{CaptureTracer}s can be used as nested context managers."""
+
+        conn = StubConnection()
+
+        def trace(statement):
+            for tracer in get_tracers():
+                tracer.connection_raw_execute(conn, "cursor", statement, [])
+
+        with CaptureTracer() as tracer1:
+            trace("one")
+            with CaptureTracer() as tracer2:
+                trace("two")
+            trace("three")
+
+        self.assertEqual([], get_tracers())
+        self.assertEqual(["one", "two", "three"], tracer1.queries)
+        self.assertEqual(["two"], tracer2.queries)
+
+    def test_capture_with_exception(self):
+        """
+        L{CaptureTracer}s re-raise any error when used as context managers.
+        """
+        errors = []
+        try:
+            with CaptureTracer():
+                raise RuntimeError("boom")
+        except RuntimeError, error:
+            errors.append(error)
+        [error] = errors
+        self.assertEqual("boom", str(error))
+        self.assertEqual([], get_tracers())