Launchpad itself

Merge lp:~salgado/launchpad/bug-495544 into lp:launchpad/db-devel

  1. bug-495544
  2. Merge into db-devel
Proposed by Guilherme Salgado
Status: Merged
Approved by: Данило Шеган
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~salgado/launchpad/bug-495544
Merge into: lp:launchpad/db-devel
Diff against target: 90 lines (+26/-17)
2 files modified
lib/canonical/database/sqlbase.py (+5/-15)
lib/canonical/database/tests/test_zopeless_transaction_manager.py (+21/-2)
To merge this branch: bzr merge lp:~salgado/launchpad/bug-495544
Reviewer Review Type Date Requested Status
Данило Шеган (community) release-critical Approve
Henning Eggers (community) code Approve
Review via email: mp+16120@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Guilherme Salgado (salgado) wrote :

Make ZopelessTransactionManager._reset_stores only reset active stores (e.g. those returned by ZStorm.iterstores) instead of always trying to reset the main-master and auth-master stores.

Revision history for this message
Henning Eggers (henninge) wrote :

Thanks for fixing this!

As discussed on IRC, please use assertContentEqual where appropriate and order assert parameters as "expected, observed" (this reviewer likes that better ;). Also, please rename the two test cases in the file as TestZopelessTransactionManagerNoLayer and TestZopelessTransactionManager.

Cheers,
Henning

review: Approve (code)
Revision history for this message
Данило Шеган (danilo) wrote :

Looks good, we'll want to QA a few scripts (at least the Soyuz ones which failed before due to previous bug fix).

review: Approve (release-critical)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/database/sqlbase.py'
2--- lib/canonical/database/sqlbase.py 2009-12-02 11:57:11 +0000
3+++ lib/canonical/database/sqlbase.py 2009-12-14 21:29:12 +0000
4@@ -341,27 +341,17 @@
5 cls._dbhost = dbhost
6 cls._dbuser = dbuser
7 cls._isolation = isolation
8- cls._reset_store()
9+ cls._reset_stores()
10 cls._installed = cls
11 return cls._installed
12
13 @staticmethod
14- def _reset_store():
15- """Reset the MAIN DEFAULT store.
16+ def _reset_stores():
17+ """Reset the active stores.
18
19 This is required for connection setting changes to be made visible.
20-
21- Other stores do not need to be reset, as code using other stores
22- or explicit flavors isn't using this compatibility layer.
23 """
24- main_store = _get_sqlobject_store()
25- # XXX: salgado, 2009-11-06, bug=253542: The import is here to work
26- # around a particularly convoluted circular import.
27- from canonical.launchpad.webapp.interfaces import (
28- IStoreSelector, AUTH_STORE, DEFAULT_FLAVOR)
29- auth_store = getUtility(IStoreSelector).get(
30- AUTH_STORE, DEFAULT_FLAVOR)
31- for store in [main_store, auth_store]:
32+ for name, store in getUtility(IZStorm).iterstores():
33 connection = store._connection
34 if connection._state == storm.database.STATE_CONNECTED:
35 if connection._raw_connection is not None:
36@@ -392,7 +382,7 @@
37 assert cls._installed is not None, (
38 "ZopelessTransactionManager not installed")
39 config.pop(cls._CONFIG_OVERLAY_NAME)
40- cls._reset_store()
41+ cls._reset_stores()
42 cls._installed = None
43
44 @classmethod
45
46=== modified file 'lib/canonical/database/tests/test_zopeless_transaction_manager.py'
47--- lib/canonical/database/tests/test_zopeless_transaction_manager.py 2009-11-06 21:30:29 +0000
48+++ lib/canonical/database/tests/test_zopeless_transaction_manager.py 2009-12-14 21:29:12 +0000
49@@ -4,14 +4,17 @@
50 from textwrap import dedent
51 import unittest
52
53-import psycopg2
54+from zope.component import getUtility
55+
56+from storm.zope.interfaces import IZStorm
57
58 from canonical.config import config
59 from canonical.database.sqlbase import ZopelessTransactionManager
60+from canonical.testing.layers import LaunchpadZopelessLayer
61 from lp.testing import TestCase
62
63
64-class TestZopelessTransactionManager(TestCase):
65+class TestZopelessTransactionManagerNoLayer(TestCase):
66
67 def test_initZopeless_connects_to_auth_master_db(self):
68 # Some scripts might create EmailAddress and Account entries, so
69@@ -39,5 +42,21 @@
70 # Clean up the configuration
71 config.pop('new-db')
72
73+
74+class TestZopelessTransactionManager(TestCase):
75+ layer = LaunchpadZopelessLayer
76+
77+ def test_reset_stores_only_does_so_on_active_stores(self):
78+ active_stores = [item[0] for item in getUtility(IZStorm).iterstores()]
79+ self.assertContentEqual(
80+ ['launchpad-main-master', 'session'], active_stores)
81+ ZopelessTransactionManager._reset_stores()
82+ # If any other stores had been reset, they'd be activated and would
83+ # then be returned by ZStorm.iterstores().
84+ new_active_stores = [
85+ item[0] for item in getUtility(IZStorm).iterstores()]
86+ self.assertContentEqual(active_stores, new_active_stores)
87+
88+
89 def test_suite():
90 return unittest.TestLoader().loadTestsFromName(__name__)

Subscribers

People subscribed via source and target branches

to status/vote changes: