Merge lp:~lucio.torre/txstatsd/distinct-plugin-fix-http into lp:~txstatsd-dev/txstatsd/distinct-plugin

Proposed by Lucio Torre
Status: Merged
Approved by: Sidnei da Silva
Approved revision: 12
Merged at revision: 12
Proposed branch: lp:~lucio.torre/txstatsd/distinct-plugin-fix-http
Merge into: lp:~txstatsd-dev/txstatsd/distinct-plugin
Diff against target: 294 lines (+132/-15)
6 files modified
bin/start-database.sh (+2/-1)
bin/stop-database.sh (+2/-0)
distinctdb/distinctmetric.py (+9/-5)
distinctdb/tests/test_distinct.py (+114/-4)
distinctdb/version.py (+1/-1)
setup.py (+4/-4)
To merge this branch: bzr merge lp:~lucio.torre/txstatsd/distinct-plugin-fix-http
Reviewer Review Type Date Requested Status
Sidnei da Silva Approve
Review via email: mp+103747@code.launchpad.net

Commit message

1- makes it compatible with pg9.1 (the one in P)
2- fixes setup.py and friends, its now installable
3- fixes resource http argument parsing so we get an int out of the query args
4- fixes the get_top query so we get the correct number of hits per user
5- returns http:500 on errors

Description of the change

1- makes it compatible with pg9.1 (the one in P)
2- fixes setup.py and friends, its now installable
3- fixes resource http argument parsing so we get an int out of the query args
4- fixes the get_top query so we get the correct number of hits per user
5- returns http:500 on errors

Sorry about doing big integration tests, but it helped a lot to find the issues.

To post a comment you must log in.
Revision history for this message
Sidnei da Silva (sidnei) wrote :

Looks good.

review: Approve
Revision history for this message
Ubuntu One Server Tarmac Bot (ubuntuone-server-tarmac) wrote :

The attempt to merge lp:~lucio.torre/txstatsd/distinct-plugin-fix-http into lp:~txstatsd-dev/txstatsd/distinct-plugin failed. Below is the output from the failed tests.

./bin/start-database.sh
## Starting postgres in /mnt/tarmac/cache/txstatsd/distinct-plugin/tmp/db1 ##
The files belonging to this database system will be owned by user "tarmac".
This user must also own the server process.

The database cluster will be initialized with locale C.
The default text search configuration will be set to "english".

fixing permissions on existing directory /mnt/tarmac/cache/txstatsd/distinct-plugin/tmp/db1/data ... ok
creating subdirectories ... ok
selecting default max_connections ... 100
selecting default shared_buffers ... 28MB
creating configuration files ... ok
creating template1 database in /mnt/tarmac/cache/txstatsd/distinct-plugin/tmp/db1/data/base/1 ... ok
initializing pg_authid ... ok
initializing dependencies ... ok
creating system views ... ok
loading system objects' descriptions ... ok
creating conversions ... ok
creating dictionaries ... ok
setting privileges on built-in objects ... ok
creating information schema ... ok
vacuuming database template1 ... ok
copying template1 to template0 ... ok
copying template1 to postgres ... ok

Success. You can now start the database server using:

    /usr/lib/postgresql/8.4/bin/postgres -D /mnt/tarmac/cache/txstatsd/distinct-plugin/tmp/db1/data
or
    /usr/lib/postgresql/8.4/bin/pg_ctl -D /mnt/tarmac/cache/txstatsd/distinct-plugin/tmp/db1/data -l logfile start

waiting for server to start.... done
server started
CREATE ROLE
To set your environment so psql will connect to this DB instance type:
    export PGHOST=/mnt/tarmac/cache/txstatsd/distinct-plugin/tmp/db1
## Done. ##
psql -h `pwd`/tmp/db1 -d distinct < bin/schema.sql
CREATE TABLE
CREATE TABLE
CREATE INDEX
./bin/start-redis.sh

WARNING: enabling "trust" authentication for local connections
You can change this by editing pg_hba.conf or using the -A option the
next time you run initdb.
ERROR: role "client" already exists
NOTICE: CREATE TABLE will create implicit sequence "paths_id_seq" for serial column "paths.id"
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "paths_pkey" for table "paths"
NOTICE: CREATE TABLE / UNIQUE will create implicit index "paths_path_key" for table "paths"
/sbin/start-stop-daemon: stat /usr/bin/redis-server: No such file or directory (No such file or directory)
make: *** [start-redis] Error 2

Revision history for this message
Ubuntu One Server Tarmac Bot (ubuntuone-server-tarmac) wrote :
Download full text (4.6 KiB)

The attempt to merge lp:~lucio.torre/txstatsd/distinct-plugin-fix-http into lp:~txstatsd-dev/txstatsd/distinct-plugin failed. Below is the output from the failed tests.

./bin/start-database.sh
## Starting postgres in /mnt/tarmac/cache/txstatsd/distinct-plugin/tmp/db1 ##
The files belonging to this database system will be owned by user "tarmac".
This user must also own the server process.

The database cluster will be initialized with locale C.
The default text search configuration will be set to "english".

fixing permissions on existing directory /mnt/tarmac/cache/txstatsd/distinct-plugin/tmp/db1/data ... ok
creating subdirectories ... ok
selecting default max_connections ... 100
selecting default shared_buffers ... 28MB
creating configuration files ... ok
creating template1 database in /mnt/tarmac/cache/txstatsd/distinct-plugin/tmp/db1/data/base/1 ... ok
initializing pg_authid ... ok
initializing dependencies ... ok
creating system views ... ok
loading system objects' descriptions ... ok
creating conversions ... ok
creating dictionaries ... ok
setting privileges on built-in objects ... ok
creating information schema ... ok
vacuuming database template1 ... ok
copying template1 to template0 ... ok
copying template1 to postgres ... ok

Success. You can now start the database server using:

    /usr/lib/postgresql/8.4/bin/postgres -D /mnt/tarmac/cache/txstatsd/distinct-plugin/tmp/db1/data
or
    /usr/lib/postgresql/8.4/bin/pg_ctl -D /mnt/tarmac/cache/txstatsd/distinct-plugin/tmp/db1/data -l logfile start

waiting for server to start.... done
server started
CREATE ROLE
To set your environment so psql will connect to this DB instance type:
    export PGHOST=/mnt/tarmac/cache/txstatsd/distinct-plugin/tmp/db1
## Done. ##
psql -h `pwd`/tmp/db1 -d distinct < bin/schema.sql
CREATE TABLE
CREATE TABLE
CREATE INDEX
./bin/start-redis.sh
trial distinctdb/
                                                                        [ERROR]

===============================================================================
[ERROR]: distinctdb.tests.test_distinct

Traceback (most recent call last):
  File "/usr/lib/python2.6/dist-packages/twisted/trial/runner.py", line 563, in loadPackage
    module = modinfo.load()
  File "/usr/lib/python2.6/dist-packages/twisted/python/modules.py", line 381, in load
    return self.pathEntry.pythonPath.moduleLoader(self.name)
  File "/usr/lib/python2.6/dist-packages/twisted/python/reflect.py", line 464, in namedAny
    topLevelPackage = _importAndCheckStack(trialname)
  File "/mnt/tarmac/cache/txstatsd/distinct-plugin/distinctdb/tests/test_distinct.py", line 25, in <module>
    from twisted.plugins import distinctdbplugin
  File "/mnt/tarmac/cache/txstatsd/distinct-plugin/twisted/plugins/distinctdbplugin.py", line 4, in <module>
    from txstatsd.itxstatsd import IMetricFactory
exceptions.ImportError: No module named txstatsd.itxstatsd
-------------------------------------------------------------------------------

FAILED (errors=1)

WARNING: enabling "trust" authentication for local connections
You can change this by editing pg_hba.conf or using the -A option the
next time you run initdb.
ERROR: role "client" already exists
NOT...

Read more...

Revision history for this message
Ubuntu One Server Tarmac Bot (ubuntuone-server-tarmac) wrote :
Download full text (4.8 KiB)

The attempt to merge lp:~lucio.torre/txstatsd/distinct-plugin-fix-http into lp:~txstatsd-dev/txstatsd/distinct-plugin failed. Below is the output from the failed tests.

./bin/start-database.sh
## Starting postgres in /mnt/tarmac/cache/txstatsd/distinct-plugin/tmp/db1 ##
The files belonging to this database system will be owned by user "tarmac".
This user must also own the server process.

The database cluster will be initialized with locale C.
The default text search configuration will be set to "english".

fixing permissions on existing directory /mnt/tarmac/cache/txstatsd/distinct-plugin/tmp/db1/data ... ok
creating subdirectories ... ok
selecting default max_connections ... 100
selecting default shared_buffers ... 28MB
creating configuration files ... ok
creating template1 database in /mnt/tarmac/cache/txstatsd/distinct-plugin/tmp/db1/data/base/1 ... ok
initializing pg_authid ... ok
initializing dependencies ... ok
creating system views ... ok
loading system objects' descriptions ... ok
creating conversions ... ok
creating dictionaries ... ok
setting privileges on built-in objects ... ok
creating information schema ... ok
vacuuming database template1 ... ok
copying template1 to template0 ... ok
copying template1 to postgres ... ok

Success. You can now start the database server using:

    /usr/lib/postgresql/8.4/bin/postgres -D /mnt/tarmac/cache/txstatsd/distinct-plugin/tmp/db1/data
or
    /usr/lib/postgresql/8.4/bin/pg_ctl -D /mnt/tarmac/cache/txstatsd/distinct-plugin/tmp/db1/data -l logfile start

waiting for server to start.... done
server started
CREATE ROLE
To set your environment so psql will connect to this DB instance type:
    export PGHOST=/mnt/tarmac/cache/txstatsd/distinct-plugin/tmp/db1
## Done. ##
psql -h `pwd`/tmp/db1 -d distinct < bin/schema.sql
CREATE TABLE
CREATE TABLE
CREATE INDEX
./bin/start-redis.sh
trial distinctdb/
distinctdb.tests.test_distinct
  TestDatabaseMetricStorage
    test_connect ... [OK]
    test_create_metric_id ... [OK]
    test_find_saved_data ... [OK]
    test_get_distinct_count ... [OK]
    test_get_distinct_top_value ... [OK]
    test_load_metric_id ... [OK]
    test_save_bucket_twice ... [OK]
  TestDistinctMetricReporter
    test_configure ... [OK]
    test_get_bucket_no ... [OK]
    test_max ... [OK]
    test_reports ... [OK]
  TestDistinctResource
    test_render_count_resource ... [OK]
    test_render_top_resource ... [OK]
  TestDistinctResourceIntegration
    test_count ... [OK]
    test_top ... ...

Read more...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bin/start-database.sh'
2--- bin/start-database.sh 2011-12-08 21:08:38 +0000
3+++ bin/start-database.sh 2012-04-26 18:22:25 +0000
4@@ -23,6 +23,8 @@
5 export PGBINDIR=/usr/lib/postgresql/8.4/bin
6 elif [ -d /usr/lib/postgresql/8.3 ]; then
7 export PGBINDIR=/usr/lib/postgresql/8.3/bin
8+ elif [ -d /usr/lib/postgresql/9.1 ]; then
9+ export PGBINDIR=/usr/lib/postgresql/9.1/bin
10 else
11 echo "Cannot find valid parent for PGBINDIR"
12 fi
13@@ -34,7 +36,6 @@
14 (
15 cat <<EOF
16 search_path='\$user,public,ts2'
17-add_missing_from=false
18 log_statement='all'
19 log_line_prefix='[%m] %q%u@%d %c '
20 fsync = off
21
22=== modified file 'bin/stop-database.sh'
23--- bin/stop-database.sh 2011-12-08 21:08:38 +0000
24+++ bin/stop-database.sh 2012-04-26 18:22:25 +0000
25@@ -10,6 +10,8 @@
26 PGBINDIR=/usr/lib/postgresql/8.4/bin
27 elif [ -d /usr/lib/postgresql/8.3 ]; then
28 PGBINDIR=/usr/lib/postgresql/8.3/bin
29+elif [ -d /usr/lib/postgresql/9.1 ]; then
30+ PGBINDIR=/usr/lib/postgresql/9.1/bin
31 else
32 echo "Cannot find valid parent for PGBINDIR"
33 fi
34
35=== modified file 'distinctdb/distinctmetric.py'
36--- distinctdb/distinctmetric.py 2012-03-15 00:32:23 +0000
37+++ distinctdb/distinctmetric.py 2012-04-26 18:22:25 +0000
38@@ -26,6 +26,7 @@
39
40 def _render_json_error(self, request, result):
41 """Fetch the data, make the request return"""
42+ request.setResponseCode(500)
43 request.write(json.dumps({"error": result.getErrorMessage()}))
44 request.finish()
45
46@@ -36,7 +37,9 @@
47
48 def render_GET(self, request):
49 """Return the web request asynchronously."""
50- d = threads.deferToThread(getattr(self.target, self.name), **request.args)
51+ d = threads.deferToThread(
52+ getattr(self.target, self.name),
53+ **dict((k, float(v[0])) for k, v in request.args.items()))
54 d.addCallback(partial(self._render_json_result, request))
55 d.addErrback(partial(self._render_json_error, request))
56 return server.NOT_DONE_YET
57@@ -48,8 +51,10 @@
58 def __init__(self, reporter):
59 resource.Resource.__init__(self)
60 self.reporter = reporter
61- self.putChild("top", JSONMethodResource(self.reporter, "get_distinct_top_value"))
62- self.putChild("count", JSONMethodResource(self.reporter, "get_distinct_count"))
63+ self.putChild("top",
64+ JSONMethodResource(self.reporter, "get_distinct_top_value"))
65+ self.putChild("count",
66+ JSONMethodResource(self.reporter, "get_distinct_count"))
67
68
69 class DistinctMetricReporter(object):
70@@ -216,11 +221,10 @@
71 path = self.prefix + self.name
72 c = psycopg2.connect(self.dsn)
73 cr = c.cursor()
74- cr.execute("SELECT value, COUNT(value) AS cnt FROM points "
75+ cr.execute("SELECT value, SUM(count) AS cnt FROM points "
76 "INNER JOIN paths ON (paths.id = points.path_id) "
77 "WHERE paths.path = %s AND bucket BETWEEN %s AND %s"
78 "GROUP BY value ORDER BY cnt DESC LIMIT %s", (
79 path, since_bucket, until_bucket, how_many,))
80 rows = cr.fetchall()
81 return rows
82-
83
84=== modified file 'distinctdb/tests/test_distinct.py'
85--- distinctdb/tests/test_distinct.py 2012-03-15 00:32:23 +0000
86+++ distinctdb/tests/test_distinct.py 2012-04-26 18:22:25 +0000
87@@ -20,11 +20,15 @@
88 import redis
89
90 from twisted.trial.unittest import TestCase
91-from twisted.internet import reactor
92+from twisted.internet import reactor, protocol, defer
93 from twisted.plugin import getPlugins
94 from twisted.plugins import distinctdbplugin
95 from twisted.web.test.test_web import DummyRequest
96
97+from twisted.application import internet
98+from twisted.web import server
99+from twisted.web.client import Agent
100+
101 from txstatsd.itxstatsd import IMetricFactory
102 from txstatsd import service
103
104@@ -46,7 +50,7 @@
105
106 def get_distinct_top_value(self, since, until, how_many=20):
107 self.called.append(("get_distinct_top_value", (since, until, how_many)))
108- return [("one", 1), ("two", 1)]
109+ return [("one", 1), ("two", 1)]
110
111
112 class TestDistinctMetricReporter(TestCase):
113@@ -130,6 +134,7 @@
114 reporter = DummyReporter()
115 request = DummyRequest([])
116 resource = distinct.JSONMethodResource(reporter, "get_foo")
117+
118 def check(result):
119 self.assertEquals({"result": "foo"},
120 json.loads("".join(request.written)))
121@@ -145,9 +150,11 @@
122 def test_render_top_resource(self):
123 reporter = DummyReporter()
124 request = DummyRequest([])
125- request.args = {"since": time.time(), "until": time.time() + 1}
126+ request.args = {"since": [str(time.time())],
127+ "until": [str(time.time() + 1)]}
128 resource = distinct.DistinctResource(reporter)
129 child_resource = resource.getChildWithDefault("top", request)
130+
131 def check(result):
132 self.assertEquals(json.dumps({"result": [("one", 1), ("two", 1)]}),
133 "".join(request.written))
134@@ -159,9 +166,11 @@
135 def test_render_count_resource(self):
136 reporter = DummyReporter()
137 request = DummyRequest([])
138- request.args = {"since": time.time(), "until": time.time() + 1}
139+ request.args = {"since": [str(time.time())],
140+ "until": [str(time.time() + 1)]}
141 resource = distinct.DistinctResource(reporter)
142 child_resource = resource.getChildWithDefault("count", request)
143+
144 def check(result):
145 self.assertEquals(json.dumps({"result": 42}),
146 "".join(request.written))
147@@ -200,6 +209,107 @@
148 dmr._save_bucket(dmr.bucket, bucket_no)
149
150
151+class ResponseCollector(protocol.Protocol):
152+
153+ def __init__(self, finished):
154+ self.finished = finished
155+ self.data = []
156+
157+ def dataReceived(self, bytes):
158+ self.data.append(bytes)
159+
160+ def connectionLost(self, reason):
161+ self.finished.callback("".join(self.data))
162+
163+
164+def collect_response(response):
165+ d = defer.Deferred()
166+ c = ResponseCollector(d)
167+ response.deliverBody(c)
168+ return d
169+
170+
171+class ErrorDummyReporter(object):
172+
173+ def get_distinct_count(self, *args, **kweargs):
174+ raise Exception("die!")
175+
176+
177+class TestDistinctResourceIntegrationError(TestCase):
178+
179+ def setUp(self):
180+ dmr = ErrorDummyReporter()
181+ root = distinct.DistinctResource(dmr)
182+ site = server.Site(root)
183+ self.port = 12341
184+ self.service = internet.TCPServer(self.port, site)
185+ self.service.startService()
186+
187+ @defer.inlineCallbacks
188+ def test_count(self):
189+ agent = Agent(reactor)
190+
191+ result = yield agent.request('GET',
192+ 'http://localhost:%s/count?since=0&until=1000000'
193+ % (self.port,))
194+ self.assertEquals(500, result.code)
195+
196+ def tearDown(self):
197+ self.service.stopService()
198+
199+
200+class TestDistinctResourceIntegration(TestDatabase):
201+
202+ def setUp(self):
203+ TestDatabase.setUp(self)
204+ dmr = distinct.DistinctMetricReporter("test", dsn=self.dsn)
205+ root = dmr.getResource()
206+ site = server.Site(root)
207+ self.port = 12341
208+ self.service = internet.TCPServer(self.port, site)
209+ self.service.startService()
210+
211+ def setup_bucket(self):
212+ dmr = distinct.DistinctMetricReporter("test", dsn=self.dsn)
213+ dmr.update("one")
214+ dmr.update("two")
215+ dmr._save_bucket(dmr.bucket, 1)
216+ dmr.bucket = {}
217+ dmr.update("one")
218+ dmr.update("one")
219+ dmr._save_bucket(dmr.bucket, 2)
220+
221+ @defer.inlineCallbacks
222+ def test_count(self):
223+ self.setup_bucket()
224+
225+ agent = Agent(reactor)
226+
227+ result = yield agent.request('GET',
228+ 'http://localhost:%s/count?since=0&until=1000000'
229+ % (self.port,))
230+ self.assertEquals(200, result.code)
231+ data = yield collect_response(result)
232+ self.assertEquals({"result": 2}, json.loads(data))
233+
234+ @defer.inlineCallbacks
235+ def test_top(self):
236+ self.setup_bucket()
237+
238+ agent = Agent(reactor)
239+
240+ result = yield agent.request('GET',
241+ 'http://localhost:%s/top?since=0&until=1000000&how_many=1'
242+ % (self.port,))
243+ self.assertEquals(200, result.code)
244+ data = yield collect_response(result)
245+ self.assertEquals({"result": [['one', 3]]}, json.loads(data))
246+
247+ def tearDown(self):
248+ self.service.stopService()
249+ TestDatabase.tearDown(self)
250+
251+
252 class TestDatabaseMetricStorage(TestDatabase):
253
254 def test_connect(self):
255
256=== modified file 'distinctdb/version.py'
257--- distinctdb/version.py 2011-12-08 21:07:55 +0000
258+++ distinctdb/version.py 2012-04-26 18:22:25 +0000
259@@ -1,1 +1,1 @@
260-distinctplugin = "0.0.1"
261+distinctdb = "0.0.1"
262
263=== modified file 'setup.py'
264--- setup.py 2011-12-08 21:14:57 +0000
265+++ setup.py 2012-04-26 18:22:25 +0000
266@@ -4,7 +4,7 @@
267
268 from twisted.plugin import IPlugin, getPlugins
269
270-from distinctplugin import version
271+from distinctdb import version
272
273 # If setuptools is present, use it to find_packages(), and also
274 # declare our dependency on epsilon.
275@@ -20,7 +20,7 @@
276 Taken from storm setup.py.
277 """
278 packages = []
279- for directory, subdirectories, files in os.walk("distinctplugin"):
280+ for directory, subdirectories, files in os.walk("distinctdb"):
281 if '__init__.py' in files:
282 packages.append(directory.replace(os.sep, '.'))
283 return packages
284@@ -42,8 +42,8 @@
285
286 setup(
287 cmdclass={'install': TxPluginInstaller},
288- name="distinctplugin",
289- version=version.distinctplugin,
290+ name="distinctdb",
291+ version=version.distinctdb,
292 description="A txstatsd plugin for distinct counts",
293 author="txStatsD Developers",
294 url="https://launchpad.net/txstatsd",

Subscribers

People subscribed via source and target branches