Merge lp:~lucio.torre/txstatsd/distinct-plugin-fix-http into lp:~txstatsd-dev/txstatsd/distinct-plugin
- distinct-plugin-fix-http
- Merge into distinct-plugin
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 |
Related bugs: |
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.
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-
## Starting postgres in /mnt/tarmac/
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/
creating subdirectories ... ok
selecting default max_connections ... 100
selecting default shared_buffers ... 28MB
creating configuration files ... ok
creating template1 database in /mnt/tarmac/
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/
or
/usr/
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=
## Done. ##
psql -h `pwd`/tmp/db1 -d distinct < bin/schema.sql
CREATE TABLE
CREATE TABLE
CREATE INDEX
./bin/start-
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-
make: *** [start-redis] Error 2
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-
## Starting postgres in /mnt/tarmac/
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/
creating subdirectories ... ok
selecting default max_connections ... 100
selecting default shared_buffers ... 28MB
creating configuration files ... ok
creating template1 database in /mnt/tarmac/
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/
or
/usr/
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=
## Done. ##
psql -h `pwd`/tmp/db1 -d distinct < bin/schema.sql
CREATE TABLE
CREATE TABLE
CREATE INDEX
./bin/start-
trial distinctdb/
=======
[ERROR]: distinctdb.
Traceback (most recent call last):
File "/usr/lib/
module = modinfo.load()
File "/usr/lib/
return self.pathEntry.
File "/usr/lib/
topLevelPackage = _importAndCheck
File "/mnt/tarmac/
from twisted.plugins import distinctdbplugin
File "/mnt/tarmac/
from txstatsd.itxstatsd import IMetricFactory
exceptions.
-------
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...
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-
## Starting postgres in /mnt/tarmac/
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/
creating subdirectories ... ok
selecting default max_connections ... 100
selecting default shared_buffers ... 28MB
creating configuration files ... ok
creating template1 database in /mnt/tarmac/
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/
or
/usr/
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=
## Done. ##
psql -h `pwd`/tmp/db1 -d distinct < bin/schema.sql
CREATE TABLE
CREATE TABLE
CREATE INDEX
./bin/start-
trial distinctdb/
distinctdb.
TestDatabaseM
test_connect ... [OK]
test_
test_
test_
test_
test_
test_
TestDistinctM
test_configure ... [OK]
test_
test_max ... [OK]
test_reports ... [OK]
TestDistinctR
test_
test_
TestDistinctR
test_count ... [OK]
test_top ... ...
Preview Diff
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", |
Looks good.