Merge lp:~elachuni/txstatsd/straight into lp:txstatsd

[Anthony Lenton]

Made it possible to use the non-twisted client code even if twisted is unavailable.

This branch makes it possible to import the non-twisted client code without having twisted installed.

It also adds a test that ensures this can be installed, by marking everything twisted-ish as unavailable and then reloading the txstats client and metrics modules.

The code works as intended afaict, and the test fails if you add a random import twisted.something anywhere in txstatsd.client or txstatsd.metrics, but I'm still unsure if it's the best way to test. I tried a couple of permutations with import hooks, but that was even messier.

[Sidnei da Silva (sidnei)]

[1] Please add the license header to protocol.py

[2] The restore_modules cleanup should reload() the modules again after restoring the unloaded twisted modules.

Once you fix those, set a commit message and flip the MP to approved, and tarmac will take care of landing it.

Thanks!

review: Approve

[Anthony Lenton (elachuni)]

Thanks Sidnei!
Fixes pushed up in rev. 100, marking as approved

[Ubuntu One Server Tarmac Bot (ubuntuone-server-tarmac)]

The attempt to merge lp:~elachuni/txstatsd/straight into lp:txstatsd failed. Below is the output from the failed tests.

txstatsd.tests.metrics.test_distinct
  TestDistinct
    test_all ... [OK]
  TestDistinctMetricReporter
    test_reports ... [OK]
  TestHash
    test_chi_square ... [SKIPPED]
    test_hash_chars ... [OK]
  TestPlugin
    test_factory ... [OK]
  TestZeros
    test_zeros ... [OK]
txstatsd.tests.metrics.test_histogrammetric
  TestHistogramReporterMetric
    test_histogram_histogram ... [OK]
    test_histogram_of_numbers_1_through_10000 ... [OK]
    test_histogram_with_zero_recorded_values ... [OK]
txstatsd.tests.metrics.test_metermetric
  TestDeriveMetricReporter
    test_fastpoll ... [OK]
    test_interface ... [OK]
txstatsd.tests.metrics.test_sli
  TestConditions
    test_above ... [OK]
    test_above_linear ... [OK]
    test_below ... [OK]
    test_below_linear ... [OK]
    test_between ... [OK]
  TestFactory
    test_configure ... [OK]
    test_configure_linear ... [OK]
  TestMetric
    test_clear ... [OK]
    test_count_all ... [OK]
    test_count_error ... [OK]
    test_count_threshold ... [OK]
    test_reports ... [OK]
  TestMetricLinear
    test_count_threshold ... [OK]
  TestParsing
    test_parse ... [OK]
txstatsd.tests.metrics.test_timermetric
  TestBlankTimerMetric
    test_count ... [OK]
    test_max ... [OK]
    test_mean ... [OK]
    test_min ... [OK]
    test_no_values ... [OK]
    test_percentiles ... [OK]
    test_rate ... [OK]
    test_std_dev ... [OK]
  TestTim...

[lp:~elachuni/txstatsd/straight]

101. By Anthony Lenton on 2012-07-13

Avoid using assertIsNone in tests, as the trial version on tarmac doesn't have that available.

=== modified file 'txstatsd/client.py'
--- txstatsd/client.py	2012-06-28 17:29:26 +0000
+++ txstatsd/client.py	2012-07-13 18:01:22 +0000
@@ -21,118 +21,20 @@
 
 import socket
 
-from twisted.internet.defer import inlineCallbacks, returnValue
-from twisted.internet.protocol import DatagramProtocol
-from twisted.python import log
-
+try:
+    import twisted
+except ImportError:
+    # If twisted is missing, still provide the non-twisted client
+    pass
+else:
+    from txstatsd.protocol import (
+        StatsDClientProtocol,
+        TwistedStatsDClient,
+    )
+        
 from txstatsd.hashing import ConsistentHashRing
 
 
-class StatsDClientProtocol(DatagramProtocol):
-    """A Twisted-based implementation of the StatsD client protocol.
-
-    Data is sent via UDP to a StatsD server for aggregation.
-    """
-
-    def __init__(self, client):
-        self.client = client
-
-    def startProtocol(self):
-        """Connect to destination host."""
-        self.client.connect(self.transport)
-
-    def stopProtocol(self):
-        """Connection was lost."""
-        self.client.disconnect()
-
-
-class TwistedStatsDClient(object):
-
-    def __init__(self, host, port,
-                 connect_callback=None,
-                 disconnect_callback=None,
-                 resolver_errback=None):
-        """
-        Build a connection that reports to the endpoint (on C{host} and
-        C{port}) using UDP.
-
-        @param host: The StatsD server host.
-        @param port: The StatsD server port.
-        @param resolver_errback: The errback to invoke should
-            issues occur resolving the supplied C{host}.
-        @param connect_callback: The callback to invoke on connection.
-        @param disconnect_callback: The callback to invoke on disconnection.
-        """
-        from twisted.internet import reactor
-
-        self.reactor = reactor
-
-        @inlineCallbacks
-        def resolve(host):
-            self.host = yield reactor.resolve(host)
-            returnValue(self.host)
-
-        self.original_host = host
-        self.host = None
-        self.resolver = resolve(host)
-        if resolver_errback is None:
-            self.resolver.addErrback(log.err)
-        else:
-            self.resolver.addErrback(resolver_errback)
-
-        self.port = port
-        self.connect_callback = connect_callback
-        self.disconnect_callback = disconnect_callback
-
-        self.transport = None
-
-    def __str__(self):
-        return "%s:%d" % (self.original_host, self.port)
-
-    @inlineCallbacks
-    def connect(self, transport=None):
-        """Connect to the StatsD server."""
-        host = yield self.resolver
-        if host is not None:
-            self.transport = transport
-            if self.transport is not None:
-                if self.connect_callback is not None:
-                    self.connect_callback()
-
-    def disconnect(self):
-        """Disconnect from the StatsD server."""
-        if self.disconnect_callback is not None:
-            self.disconnect_callback()
-        self.transport = None
-
-    def write(self, data, callback=None):
-        """Send the metric to the StatsD server.
-
-        @param data: The data to be sent.
-        @param callback: The callback to which the result should be sent.
-            B{Note}: The C{callback} will be called in the C{reactor}
-            thread, and not in the thread of the original caller.
-        """
-        self.reactor.callFromThread(self._write, data, callback)
-
-    def _write(self, data, callback):
-        """Send the metric to the StatsD server.
-
-        @param data: The data to be sent.
-        @param callback: The callback to which the result should be sent.
-        @raise twisted.internet.error.MessageLengthError: If the size of data
-            is too large.
-        """
-        if self.host is not None and self.transport is not None:
-            try:
-                bytes_sent = self.transport.write(data, (self.host, self.port))
-                if callback is not None:
-                    callback(bytes_sent)
-            except (OverflowError, TypeError, socket.error, socket.gaierror):
-                if callback is not None:
-                    callback(None)
-
-
 class UdpStatsDClient(object):
 
     def __init__(self, host=None, port=None):

=== added file 'txstatsd/protocol.py'
--- txstatsd/protocol.py	1970-01-01 00:00:00 +0000
+++ txstatsd/protocol.py	2012-07-13 18:01:22 +0000
@@ -0,0 +1,131 @@
+# Copyright (C) 2011-2012 Canonical Services Ltd
+#
+# Permission is hereby granted, free of charge, to any person obtaining
+# a copy of this software and associated documentation files (the
+# "Software"), to deal in the Software without restriction, including
+# without limitation the rights to use, copy, modify, merge, publish,
+# distribute, sublicense, and/or sell copies of the Software, and to
+# permit persons to whom the Software is furnished to do so, subject to
+# the following conditions:
+#
+# The above copyright notice and this permission notice shall be
+# included in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+# IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
+# CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+# TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+# SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+import socket
+
+from twisted.internet.defer import inlineCallbacks, returnValue
+from twisted.internet.protocol import DatagramProtocol
+from twisted.python import log
+
+
+class StatsDClientProtocol(DatagramProtocol):
+    """A Twisted-based implementation of the StatsD client protocol.
+
+    Data is sent via UDP to a StatsD server for aggregation.
+    """
+
+    def __init__(self, client):
+        self.client = client
+
+    def startProtocol(self):
+        """Connect to destination host."""
+        self.client.connect(self.transport)
+
+    def stopProtocol(self):
+        """Connection was lost."""
+        self.client.disconnect()
+
+
+class TwistedStatsDClient(object):
+
+    def __init__(self, host, port,
+                 connect_callback=None,
+                 disconnect_callback=None,
+                 resolver_errback=None):
+        """
+        Build a connection that reports to the endpoint (on C{host} and
+        C{port}) using UDP.
+
+        @param host: The StatsD server host.
+        @param port: The StatsD server port.
+        @param resolver_errback: The errback to invoke should
+            issues occur resolving the supplied C{host}.
+        @param connect_callback: The callback to invoke on connection.
+        @param disconnect_callback: The callback to invoke on disconnection.
+        """
+        from twisted.internet import reactor
+
+        self.reactor = reactor
+
+        @inlineCallbacks
+        def resolve(host):
+            self.host = yield reactor.resolve(host)
+            returnValue(self.host)
+
+        self.original_host = host
+        self.host = None
+        self.resolver = resolve(host)
+        if resolver_errback is None:
+            self.resolver.addErrback(log.err)
+        else:
+            self.resolver.addErrback(resolver_errback)
+
+        self.port = port
+        self.connect_callback = connect_callback
+        self.disconnect_callback = disconnect_callback
+
+        self.transport = None
+
+    def __str__(self):
+        return "%s:%d" % (self.original_host, self.port)
+
+    @inlineCallbacks
+    def connect(self, transport=None):
+        """Connect to the StatsD server."""
+        host = yield self.resolver
+        if host is not None:
+            self.transport = transport
+            if self.transport is not None:
+                if self.connect_callback is not None:
+                    self.connect_callback()
+
+    def disconnect(self):
+        """Disconnect from the StatsD server."""
+        if self.disconnect_callback is not None:
+            self.disconnect_callback()
+        self.transport = None
+
+    def write(self, data, callback=None):
+        """Send the metric to the StatsD server.
+
+        @param data: The data to be sent.
+        @param callback: The callback to which the result should be sent.
+            B{Note}: The C{callback} will be called in the C{reactor}
+            thread, and not in the thread of the original caller.
+        """
+        self.reactor.callFromThread(self._write, data, callback)
+
+    def _write(self, data, callback):
+        """Send the metric to the StatsD server.
+
+        @param data: The data to be sent.
+        @param callback: The callback to which the result should be sent.
+        @raise twisted.internet.error.MessageLengthError: If the size of data
+            is too large.
+        """
+        if self.host is not None and self.transport is not None:
+            try:
+                bytes_sent = self.transport.write(data, (self.host, self.port))
+                if callback is not None:
+                    callback(bytes_sent)
+            except (OverflowError, TypeError, socket.error, socket.gaierror):
+                if callback is not None:
+                    callback(None)

=== modified file 'txstatsd/tests/test_client.py'
--- txstatsd/tests/test_client.py	2012-06-28 17:29:26 +0000
+++ txstatsd/tests/test_client.py	2012-07-13 18:01:22 +0000
@@ -20,11 +20,15 @@
 # SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 """Tests for the various client classes."""
 
+import sys
 from twisted.internet import reactor
 from twisted.internet.defer import inlineCallbacks, Deferred
 from twisted.python import log
 from twisted.trial.unittest import TestCase
 
+import txstatsd.client
+import txstatsd.metrics.metric
+import txstatsd.metrics.metrics
 from txstatsd.metrics.metric import Metric
 from txstatsd.client import (
     StatsDClientProtocol, TwistedStatsDClient, UdpStatsDClient,
@@ -128,6 +132,29 @@
         # setblocking(0) is the same as settimeout(0.0).
         self.assertEqual(client.socket.gettimeout(), 0.0)
 
+    def test_udp_client_can_be_imported_without_twisted(self):
+        """Ensure that the twisted-less client can be used without twisted."""
+        unloaded = [(name, mod) for (name, mod) in sys.modules.items()
+                    if 'twisted' in name]
+        def restore_modules():
+            for name, mod in unloaded:
+                sys.modules[name] = mod
+            reload(txstatsd.client)
+            reload(txstatsd.metrics.metrics)
+            reload(txstatsd.metrics.metric)
+        self.addCleanup(restore_modules)
+
+        # Mark everything twistedish as unavailable
+        for name, mod in unloaded:
+            sys.modules[name] = None
+
+        reload(txstatsd.client)
+        reload(txstatsd.metrics.metrics)
+        reload(txstatsd.metrics.metric)
+        for mod in sys.modules:
+            if 'twisted' in mod:
+                self.assertTrue(sys.modules[mod] is None)
+
 
 class TestConsistentHashingClient(TestCase):