Merge lp:~allenap/storm/oneiric-admin-shutdown-bug-871596 into lp:storm
- oneiric-admin-shutdown-bug-871596
- Merge into trunk
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Gavin Panella | ||||
Approved revision: | 447 | ||||
Merged at revision: | 421 | ||||
Proposed branch: | lp:~allenap/storm/oneiric-admin-shutdown-bug-871596 | ||||
Merge into: | lp:storm | ||||
Prerequisite: | lp:~allenap/storm/go-setuptools | ||||
Diff against target: |
407 lines (+212/-32) 9 files modified
MANIFEST.in (+1/-1) NEWS (+12/-1) README (+11/-4) setup.py (+4/-0) storm/databases/postgres.py (+46/-15) test (+2/-3) tests/__init__.py (+9/-0) tests/databases/postgres.py (+120/-1) tests/tracer.py (+7/-7) |
||||
To merge this branch: | bzr merge lp:~allenap/storm/oneiric-admin-shutdown-bug-871596 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gavin Panella | Abstain | ||
Raphaël Badin | code | Approve | |
Stuart Bishop | Pending | ||
Review via email: mp+79889@code.launchpad.net |
This proposal supersedes a proposal from 2011-10-17.
Commit message
Description of the change
This branch adds three tests for disconnection errors:
- When the connection is terminated with pg_terminate_
- When the connection is terminated with pg_terminate_
connection goes via pgbouncer.
- When the connection is terminated because pgbouncer is terminated.
It then updates is_disconnectio
Additionally, under Oneiric, some disconnection tests were failing
anyway, so is_disconnectio
This branch adds two optional test dependencies: python-fixtures and
python-pgbouncer. If these are not available the tests will not be
attempted.
Stuart Bishop (stub) wrote : Posted in a previous version of this proposal | # |
Gavin Panella (allenap) wrote : Posted in a previous version of this proposal | # |
> Thanks for working on this!
>
> So the only time you ever saw the 57P01 error was with pgbouncer? That is
> interesting as in the past we often never saw the error codes at all (hence
> the string matching).
Yeah, I don't fully understand what was happening there.
>
> The code seems good. Just a few suggestions and points for discussion:
>
> - Rather than check just for 57P01, I think we might as well check for the
> following list:
>
> pg_connection_
> '08006', # CONNECTION FAILURE
> '08001', # SQLCLIENT UNABLE TO ESTABLISH SQLCONNECTION
> '08004', # SQLSERVER REJECTED ESTABLISHMENT OF SQLCONNECTION
> '53300', # TOO MANY CONNECTIONS
> '57000', # OPERATOR INTERVENTION
> '57014', # QUERY CANCELED
> '57P01', # ADMIN SHUTDOWN
> '57P02', # CRASH SHUTDOWN
> '57P03', # CANNOT CONNECT NOW
> ])
Tip top, done.
>
> - Should we document installing python-pgbouncer using the Python egg from
> pypi rather than checking out the Bazaar branch and using a symlink hack?
>
> - Should python-fixtures be an optional dependency? Maybe if it gets used
> elsewhere.
>
> - Do we want python-pgbouncer and fixtures as optional dependencies for
> running the PostgreSQL tests? I understand not needing the environment setup
> for testing every database backend, but there seems little point for allowing
> people to run a subset of the tests for a particular DB backend.
I've switched everything to use setuptools, and have defined all the
test dependencies in setup.py. I've switched make check over to use
setuptools' testing support, and I've also made the ./test script use
the eggs that setuptools downloads. There's also an additional develop
Makefile target that downloads the test dependencies without running
the tests.
This means you can do make check from a fresh branch of Storm and run
*all* the tests without further intervention (other than doing the
one-off package installations and database set-up). This has already
shown me that there's another Django disconnection test failure that I
need to address.
Robert Collins (lifeless) wrote : Posted in a previous version of this proposal | # |
van.pg can do postgresql cluster setup as a test fixture, if you want :)
Gavin Panella (allenap) wrote : Posted in a previous version of this proposal | # |
> van.pg can do postgresql cluster setup as a test fixture, if you
> want :)
Indeed, I do want! I did some work on van.pg last week or the week
before, but it was a little more complicated than I had imagined... or
I was misunderstanding. I won't attempt it again in this branch, but I
feel like I might have a much better chance of success with the things
I've learnt doing this.
Stuart Bishop (stub) wrote : Posted in a previous version of this proposal | # |
On Wed, Oct 19, 2011 at 3:42 PM, Gavin Panella
<email address hidden> wrote:
>> van.pg can do postgresql cluster setup as a test fixture, if you
>> want :)
>
> Indeed, I do want! I did some work on van.pg last week or the week
> before, but it was a little more complicated than I had imagined... or
> I was misunderstanding. I won't attempt it again in this branch, but I
> feel like I might have a much better chance of success with the things
> I've learnt doing this.
Yer, separate branch for that or this will never land :)
--
Stuart Bishop <email address hidden>
http://
Gavin Panella (allenap) wrote : Posted in a previous version of this proposal | # |
I'm going to split this branch into two, to help review.
- 443. By Gavin Panella
-
Include ez_setup.py in tarballs.
- 444. By Gavin Panella
-
Rely upon pgbouncer 0.0.6 or later, owing to unreliable shutdown problems in earlier versions.
- 445. By Gavin Panella
-
Deal with a weird ProgrammingError when connecting via pgbouncer.
- 446. By Gavin Panella
-
QUERY CANCELED is not a disconnection.
- 447. By Gavin Panella
-
Merged go-setuptools into oneiric-
admin-shutdown-bug-871596, resolving conflicts.
Raphaël Badin (rvb) wrote : | # |
I'm getting weird Segmentation fault errors when running the test suite but I know this is not unheard of when dealing with storm :/.
test_
I suppose you have all the tests passing so it must be something with my setup.
Apart from that, this looks good. (I confess I'd like to know more about pgbouncer but right now http://
[0]
8 -include MANIFEST.in LICENSE README TODO NEWS Makefile setup.cfg test
9 +include MANIFEST.in LICENSE README TODO NEWS Makefile setup.cfg test ez_setup.py
This should probably go in the prereq branch but since you're going to land them together I guess that's ok ;)
[1]
317 +if has_fixtures:
318 + # Upgrade to full test case class with fixtures.
319 + from fixtures import TestWithFixtures
320 + class PostgresDisconn
321 + PostgresDisconn
322 + TestWithFixtures, TestHelper): pass
I don't think that's very readable but I have no better idea atm…
Gavin Panella (allenap) wrote : | # |
Stuart reviewed an earlier version of this branch. I have not made
huge changes since then so I'm going to assume that his +1 still
stands.
Gavin Panella (allenap) wrote : | # |
> I'm getting weird Segmentation fault errors when running the test suite but I
> know this is not unheard of when dealing with storm :/.
>
> test_terminated
> (tests.
> ... Segmentation fault
>
> I suppose you have all the tests passing so it must be something with my
> setup.
Grargh, there are two outstanding tests failing which I ought to
address:
test_
in tests.django.
test_
in tests.django.
> Apart from that, this looks good. (I confess I'd like to know more about
> pgbouncer but right now http://
> "PgFoundry Could Not Connect to Database:").
man 1 pgbouncer is where I read about it.
> [0]
>
> 8 -include MANIFEST.in LICENSE README TODO NEWS Makefile setup.cfg test
> 9 +include MANIFEST.in LICENSE README TODO NEWS Makefile setup.cfg test
> ez_setup.py
>
> This should probably go in the prereq branch but since you're going to land
> them together I guess that's ok ;)
Doh, of course. Oops.
>
> [1]
>
> 317 +if has_fixtures:
> 318 + # Upgrade to full test case class with fixtures.
> 319 + from fixtures import TestWithFixtures
> 320 + class PostgresDisconn
> 321 + PostgresDisconn
> 322 + TestWithFixtures, TestHelper): pass
>
> I don't think that's very readable but I have no better idea atm…
No, me neither :-/
- 448. By Gavin Panella
-
Merged go-setuptools into oneiric-
admin-shutdown-bug-871596, resolving conflicts. - 449. By Gavin Panella
-
Merged go-setuptools into oneiric-
admin-shutdown-bug-871596. - 450. By Gavin Panella
-
Use has_fixtures from tests/__init__.py.
- 451. By Gavin Panella
-
Update NEWS.
- 452. By Gavin Panella
-
pgbouncer 0.0.7 is needed.
- 453. By Gavin Panella
-
Add eggs to path before importing tests.
- 454. By Gavin Panella
-
Depend on testtools >= 0.9.8; newer versions of python-fixtures needs it.
Preview Diff
1 | === modified file 'MANIFEST.in' |
2 | --- MANIFEST.in 2008-01-28 18:17:33 +0000 |
3 | +++ MANIFEST.in 2011-10-28 14:51:59 +0000 |
4 | @@ -1,4 +1,4 @@ |
5 | recursive-include storm *.py *.c *.zcml |
6 | recursive-include tests *.py *.txt |
7 | |
8 | -include MANIFEST.in LICENSE README TODO NEWS Makefile setup.cfg test |
9 | +include MANIFEST.in LICENSE README TODO NEWS Makefile setup.cfg test ez_setup.py |
10 | |
11 | === modified file 'NEWS' |
12 | --- NEWS 2011-10-27 13:19:00 +0000 |
13 | +++ NEWS 2011-10-28 14:51:59 +0000 |
14 | @@ -14,10 +14,21 @@ |
15 | |
16 | You will need the python-fixtures package in order to use this feature. |
17 | |
18 | +- Setuptools <http://pypi.python.org/pypi/setuptools> is now required |
19 | + to run setup.py. This makes it much easier to install the majority |
20 | + of the dependencies required to run the test suite in its entirety. |
21 | + |
22 | +- Disconnection errors arising from PostgreSQL are now more reliably |
23 | + detected, especially with regard to recent libpq updates in Ubuntu. |
24 | + There are also disconnection tests that simulate sudden termination |
25 | + of pgbouncer <http://pgfoundry.org/projects/pgbouncer/>. |
26 | + |
27 | Bug fixes |
28 | --------- |
29 | |
30 | -- When a SQLObjectResultSet object was sliced with slice.start and slice.end both zero (sqlobject[0:0]), the full, unsliced resultset was returned (bug #872086). |
31 | +- When a SQLObjectResultSet object was sliced with slice.start and |
32 | + slice.end both zero (sqlobject[0:0]), the full, unsliced resultset |
33 | + was returned (bug #872086). |
34 | |
35 | 0.19 (2011-10-03) |
36 | ================= |
37 | |
38 | === modified file 'README' |
39 | --- README 2011-10-28 14:51:59 +0000 |
40 | +++ README 2011-10-28 14:51:59 +0000 |
41 | @@ -100,7 +100,8 @@ |
42 | |
43 | $ sudo apt-get install \ |
44 | python-mysqldb mysql-server \ |
45 | - postgresql build-essential |
46 | + postgresql pgbouncer \ |
47 | + build-essential |
48 | |
49 | These will take a few minutes to download (its a bit under 200MB all |
50 | together). Once the download is complete, a screen called |
51 | @@ -115,9 +116,15 @@ |
52 | apt-get: |
53 | |
54 | $ apt-get install \ |
55 | - python-django python-psycopg2 python-testresources \ |
56 | - python-transaction python-twisted python-zope.component \ |
57 | - python-zope.security |
58 | + python-django python-fixtures python-psycopg2 \ |
59 | + python-testresources python-transaction python-twisted \ |
60 | + python-zope.component python-zope.security |
61 | + |
62 | +Two modules - pgbouncer and timeline - are not yet packaged in |
63 | +Ubuntu. These can be installed from PyPI: |
64 | + |
65 | + http://pypi.python.org/pypi/pgbouncer |
66 | + http://pypi.python.org/pypi/timeline |
67 | |
68 | Alternatively, dependencies can be downloaded as eggs into the current |
69 | directory with: |
70 | |
71 | === modified file 'setup.py' |
72 | --- setup.py 2011-10-28 14:51:59 +0000 |
73 | +++ setup.py 2011-10-28 14:51:59 +0000 |
74 | @@ -54,8 +54,12 @@ |
75 | tests_require=[ |
76 | # Versions based on Lucid, where packaged. |
77 | "django >= 1.1.1", |
78 | + "fixtures >= 0.3.5", |
79 | + # pgbouncer (the Python module) is not yet packaged in Ubuntu. |
80 | + "pgbouncer >= 0.0.7", |
81 | "psycopg2 >= 2.0.13", |
82 | "testresources >= 0.2.4", |
83 | + "testtools >= 0.9.8", |
84 | # timeline is not yet packaged in Ubuntu. |
85 | "timeline >= 0.0.2", |
86 | "transaction >= 1.0.0", |
87 | |
88 | === modified file 'storm/databases/postgres.py' |
89 | --- storm/databases/postgres.py 2011-09-22 09:16:49 +0000 |
90 | +++ storm/databases/postgres.py 2011-10-28 14:51:59 +0000 |
91 | @@ -47,7 +47,7 @@ |
92 | from storm.database import Database, Connection, Result |
93 | from storm.exceptions import ( |
94 | install_exceptions, DatabaseError, DatabaseModuleError, InterfaceError, |
95 | - OperationalError, ProgrammingError, TimeoutError) |
96 | + OperationalError, ProgrammingError, TimeoutError, Error) |
97 | from storm.tracer import TimeoutTracer |
98 | |
99 | |
100 | @@ -217,6 +217,18 @@ |
101 | return And(*equals) |
102 | |
103 | |
104 | +pg_connection_failure_codes = frozenset([ |
105 | + '08006', # CONNECTION FAILURE |
106 | + '08001', # SQLCLIENT UNABLE TO ESTABLISH SQLCONNECTION |
107 | + '08004', # SQLSERVER REJECTED ESTABLISHMENT OF SQLCONNECTION |
108 | + '53300', # TOO MANY CONNECTIONS |
109 | + '57000', # OPERATOR INTERVENTION |
110 | + '57P01', # ADMIN SHUTDOWN |
111 | + '57P02', # CRASH SHUTDOWN |
112 | + '57P03', # CANNOT CONNECT NOW |
113 | + ]) |
114 | + |
115 | + |
116 | class PostgresConnection(Connection): |
117 | |
118 | result_factory = PostgresResult |
119 | @@ -277,22 +289,41 @@ |
120 | yield param |
121 | |
122 | def is_disconnection_error(self, exc, extra_disconnection_errors=()): |
123 | - if not isinstance(exc, (InterfaceError, OperationalError, |
124 | - ProgrammingError, extra_disconnection_errors)): |
125 | - return False |
126 | - |
127 | - # XXX: 2007-09-17 jamesh |
128 | - # The last message is for the benefit of old versions of |
129 | - # psycopg2 (prior to 2.0.7) which have a bug related to |
130 | - # stripping the error severity from the message. |
131 | - msg = str(exc) |
132 | - return ("server closed the connection unexpectedly" in msg or |
133 | + # Attempt to use pgcode to determine the nature of the error. This is |
134 | + # more reliable than string matching because it is not affected by |
135 | + # locale settings. Fall through if pgcode is not available. |
136 | + if isinstance(exc, Error): |
137 | + pgcode = getattr(exc, "pgcode", None) |
138 | + if pgcode in pg_connection_failure_codes: |
139 | + return True |
140 | + |
141 | + disconnection_errors = ( |
142 | + InterfaceError, OperationalError, ProgrammingError, |
143 | + extra_disconnection_errors) |
144 | + |
145 | + if isinstance(exc, disconnection_errors): |
146 | + # When the connection is closed by a termination of pgbouncer, a |
147 | + # ProgrammingError is raised. If the raw connection is closed we |
148 | + # assume it's actually a disconnection. |
149 | + if isinstance(exc, ProgrammingError): |
150 | + if self._raw_connection.closed: |
151 | + return True |
152 | + msg = str(exc) |
153 | + return ( |
154 | + "connection already closed" in msg or |
155 | + "connection not open" in msg or |
156 | "could not connect to server" in msg or |
157 | + "could not receive data from server" in msg or |
158 | + "losed the connection unexpectedly" in msg or |
159 | "no connection to the server" in msg or |
160 | - "connection not open" in msg or |
161 | - "connection already closed" in msg or |
162 | - "losed the connection unexpectedly" in msg or |
163 | - "could not receive data from server" in msg) |
164 | + "terminating connection due to administrator" in msg) |
165 | + elif isinstance(exc, DatabaseError): |
166 | + msg = str(exc) |
167 | + return ( |
168 | + "EOF detected" in msg or |
169 | + "server closed the connection unexpectedly" in msg) |
170 | + else: |
171 | + return False |
172 | |
173 | |
174 | class Postgres(Database): |
175 | |
176 | === modified file 'test' |
177 | --- test 2011-10-28 14:51:59 +0000 |
178 | +++ test 2011-10-28 14:51:59 +0000 |
179 | @@ -26,8 +26,6 @@ |
180 | import sys |
181 | import os |
182 | |
183 | -import tests |
184 | - |
185 | |
186 | def add_eggs_to_path(): |
187 | here = os.path.dirname(__file__) |
188 | @@ -50,7 +48,8 @@ |
189 | # top directory, so we add them to sys.path here for convenience. |
190 | add_eggs_to_path() |
191 | |
192 | - suite = tests.find_tests(args) |
193 | + from tests import find_tests |
194 | + suite = find_tests(args) |
195 | result = runner.run(suite) |
196 | return not result.wasSuccessful() |
197 | |
198 | |
199 | === modified file 'tests/__init__.py' |
200 | --- tests/__init__.py 2011-10-28 14:51:59 +0000 |
201 | +++ tests/__init__.py 2011-10-28 14:51:59 +0000 |
202 | @@ -21,12 +21,21 @@ |
203 | |
204 | __all__ = [ |
205 | 'find_tests', |
206 | + 'has_fixtures', |
207 | ] |
208 | |
209 | import doctest |
210 | import os |
211 | import unittest |
212 | |
213 | +try: |
214 | + import fixtures |
215 | + fixtures # Silence lint. |
216 | +except ImportError: |
217 | + has_fixtures = False |
218 | +else: |
219 | + has_fixtures = True |
220 | + |
221 | |
222 | def find_tests(testpaths=()): |
223 | """Find all test paths, or test paths contained in the provided sequence. |
224 | |
225 | === modified file 'tests/databases/postgres.py' |
226 | --- tests/databases/postgres.py 2011-02-25 10:26:26 +0000 |
227 | +++ tests/databases/postgres.py 2011-10-28 14:51:59 +0000 |
228 | @@ -22,26 +22,53 @@ |
229 | import os |
230 | |
231 | from storm.databases.postgres import ( |
232 | - Postgres, compile, currval, Returning, PostgresTimeoutTracer) |
233 | + Postgres, compile, currval, Returning, PostgresTimeoutTracer, make_dsn) |
234 | from storm.database import create_database |
235 | from storm.exceptions import InterfaceError, ProgrammingError |
236 | from storm.variables import DateTimeVariable, RawStrVariable |
237 | from storm.variables import ListVariable, IntVariable, Variable |
238 | from storm.properties import Int |
239 | +from storm.exceptions import DisconnectionError |
240 | from storm.expr import (Union, Select, Insert, Alias, SQLRaw, State, |
241 | Sequence, Like, Column, COLUMN) |
242 | from storm.tracer import install_tracer, TimeoutError |
243 | +from storm.uri import URI |
244 | |
245 | # We need the info to register the 'type' compiler. In normal |
246 | # circumstances this is naturally imported. |
247 | import storm.info |
248 | +storm # Silence lint. |
249 | |
250 | +from tests import has_fixtures |
251 | from tests.databases.base import ( |
252 | DatabaseTest, DatabaseDisconnectionTest, UnsupportedDatabaseTest) |
253 | from tests.expr import column1, column2, column3, elem1, table1, TrackContext |
254 | from tests.tracer import TimeoutTracerTestBase |
255 | from tests.helper import TestHelper |
256 | |
257 | +try: |
258 | + import pgbouncer |
259 | +except ImportError: |
260 | + has_pgbouncer = False |
261 | +else: |
262 | + has_pgbouncer = True |
263 | + |
264 | + |
265 | +def terminate_other_backends(connection): |
266 | + """Terminate all connections to the database except the one given.""" |
267 | + connection.execute( |
268 | + "SELECT pg_terminate_backend(procpid)" |
269 | + " FROM pg_stat_activity" |
270 | + " WHERE datname = current_database()" |
271 | + " AND procpid != pg_backend_pid()") |
272 | + |
273 | + |
274 | +def terminate_all_backends(database): |
275 | + """Terminate all connections to the given database.""" |
276 | + connection = database.connect() |
277 | + terminate_other_backends(connection) |
278 | + connection.close() |
279 | + |
280 | |
281 | class PostgresTest(DatabaseTest, TestHelper): |
282 | |
283 | @@ -542,6 +569,98 @@ |
284 | self.fail('Exception should have been swallowed: %s' % repr(exc)) |
285 | |
286 | |
287 | +class PostgresDisconnectionTestWithoutProxyBase(object): |
288 | + # DatabaseDisconnectionTest uses a socket proxy to simulate broken |
289 | + # connections. This class tests some other causes of disconnection. |
290 | + |
291 | + database_uri = None |
292 | + |
293 | + def is_supported(self): |
294 | + return bool(self.database_uri) |
295 | + |
296 | + def setUp(self): |
297 | + super(PostgresDisconnectionTestWithoutProxyBase, self).setUp() |
298 | + self.database = create_database(self.database_uri) |
299 | + |
300 | + def test_terminated_backend(self): |
301 | + # The error raised when trying to use a connection that has been |
302 | + # terminated at the server is considered a disconnection error. |
303 | + connection = self.database.connect() |
304 | + terminate_all_backends(self.database) |
305 | + self.assertRaises( |
306 | + DisconnectionError, connection.execute, |
307 | + "SELECT current_database()") |
308 | + |
309 | + |
310 | +class PostgresDisconnectionTestWithoutProxyUnixSockets( |
311 | + PostgresDisconnectionTestWithoutProxyBase, TestHelper): |
312 | + """Disconnection tests using Unix sockets.""" |
313 | + |
314 | + database_uri = os.environ.get("STORM_POSTGRES_URI") |
315 | + |
316 | + |
317 | +class PostgresDisconnectionTestWithoutProxyTCPSockets( |
318 | + PostgresDisconnectionTestWithoutProxyBase, TestHelper): |
319 | + """Disconnection tests using TCP sockets.""" |
320 | + |
321 | + database_uri = os.environ.get("STORM_POSTGRES_HOST_URI") |
322 | + |
323 | + |
324 | +class PostgresDisconnectionTestWithPGBouncerBase(object): |
325 | + # Connecting via pgbouncer <http://pgfoundry.org/projects/pgbouncer> |
326 | + # introduces new possible causes of disconnections. |
327 | + |
328 | + def is_supported(self): |
329 | + return ( |
330 | + has_fixtures and has_pgbouncer and |
331 | + bool(os.environ.get("STORM_POSTGRES_HOST_URI"))) |
332 | + |
333 | + def setUp(self): |
334 | + super(PostgresDisconnectionTestWithPGBouncerBase, self).setUp() |
335 | + database_uri = URI(os.environ["STORM_POSTGRES_HOST_URI"]) |
336 | + database_user = database_uri.username or os.environ['USER'] |
337 | + database_dsn = make_dsn(database_uri) |
338 | + # Create a pgbouncer fixture. |
339 | + self.pgbouncer = pgbouncer.fixture.PGBouncerFixture() |
340 | + self.pgbouncer.databases[database_uri.database] = database_dsn |
341 | + self.pgbouncer.users[database_user] = "trusted" |
342 | + self.pgbouncer.admin_users = [database_user] |
343 | + self.useFixture(self.pgbouncer) |
344 | + # Create a Database that uses pgbouncer. |
345 | + pgbouncer_uri = database_uri.copy() |
346 | + pgbouncer_uri.host = self.pgbouncer.host |
347 | + pgbouncer_uri.port = self.pgbouncer.port |
348 | + self.database = create_database(pgbouncer_uri) |
349 | + |
350 | + def test_terminated_backend(self): |
351 | + # The error raised when trying to use a connection through pgbouncer |
352 | + # that has been terminated at the server is considered a disconnection |
353 | + # error. |
354 | + connection = self.database.connect() |
355 | + terminate_all_backends(self.database) |
356 | + self.assertRaises( |
357 | + DisconnectionError, connection.execute, |
358 | + "SELECT current_database()") |
359 | + |
360 | + def test_pgbouncer_stopped(self): |
361 | + # The error raised from a connection that is no longer connected |
362 | + # because pgbouncer has been immediately shutdown (via SIGTERM; see |
363 | + # man 1 pgbouncer) is considered a disconnection error. |
364 | + connection = self.database.connect() |
365 | + self.pgbouncer.stop() |
366 | + self.assertRaises( |
367 | + DisconnectionError, connection.execute, |
368 | + "SELECT current_database()") |
369 | + |
370 | + |
371 | +if has_fixtures: |
372 | + # Upgrade to full test case class with fixtures. |
373 | + from fixtures import TestWithFixtures |
374 | + class PostgresDisconnectionTestWithPGBouncer( |
375 | + PostgresDisconnectionTestWithPGBouncerBase, |
376 | + TestWithFixtures, TestHelper): pass |
377 | + |
378 | + |
379 | class PostgresTimeoutTracerTest(TimeoutTracerTestBase): |
380 | |
381 | tracer_class = PostgresTimeoutTracer |
382 | |
383 | === modified file 'tests/tracer.py' |
384 | --- tests/tracer.py 2011-10-27 13:22:32 +0000 |
385 | +++ tests/tracer.py 2011-10-28 14:51:59 +0000 |
386 | @@ -2,14 +2,14 @@ |
387 | import sys |
388 | from unittest import TestCase |
389 | |
390 | -try: |
391 | - # Optional dependency, if missing Fixture tests are skipped. |
392 | +from tests import has_fixtures |
393 | + |
394 | +# Optional dependency. If missing, Fixture tests are skipped. |
395 | +if has_fixtures: |
396 | + import fixtures.testcase |
397 | + TestWithFixtures = fixtures.testcase.TestWithFixtures |
398 | +else: |
399 | TestWithFixtures = object |
400 | - from fixtures.testcase import TestWithFixtures |
401 | -except ImportError: |
402 | - has_fixtures = False |
403 | -else: |
404 | - has_fixtures = True |
405 | |
406 | try: |
407 | # Optional dependency, if missing TimelineTracer tests are skipped. |
Thanks for working on this!
So the only time you ever saw the 57P01 error was with pgbouncer? That is interesting as in the past we often never saw the error codes at all (hence the string matching).
The code seems good. Just a few suggestions and points for discussion:
- Rather than check just for 57P01, I think we might as well check for the following list:
pg_connection_ failure_ codes = frozenset([
'08006', # CONNECTION FAILURE
'08001', # SQLCLIENT UNABLE TO ESTABLISH SQLCONNECTION
'08004', # SQLSERVER REJECTED ESTABLISHMENT OF SQLCONNECTION
'53300', # TOO MANY CONNECTIONS
'57000', # OPERATOR INTERVENTION
'57014', # QUERY CANCELED
'57P01', # ADMIN SHUTDOWN
'57P02', # CRASH SHUTDOWN
'57P03', # CANNOT CONNECT NOW
])
- Should we document installing python-pgbouncer using the Python egg from pypi rather than checking out the Bazaar branch and using a symlink hack?
- Should python-fixtures be an optional dependency? Maybe if it gets used elsewhere.
- Do we want python-pgbouncer and fixtures as optional dependencies for running the PostgreSQL tests? I understand not needing the environment setup for testing every database backend, but there seems little point for allowing people to run a subset of the tests for a particular DB backend.