Merge lp:~canonical-isd-hackers/canonical-identity-provider/716494-readonly-set-db into lp:canonical-identity-provider/release

[Ricardo Kirkner]

fixed bug in selecting the proper database for failover

This branch fixes a bug introduced by moving from django 1.0.2 to django 1.1, because of the way the connection settings are retrieved in the latter.

Now the DatabaseWrapper.settings_dict also gets updated when the new database is selected.

[David Owen (dsowen)]

Nice fix for a subtle bug!

review: Approve

=== modified file 'identityprovider/readonly.py'
--- identityprovider/readonly.py	2010-10-14 17:30:00 +0000
+++ identityprovider/readonly.py	2011-02-10 20:09:58 +0000
@@ -85,8 +85,12 @@
             to set the database connection
         """
         for attname in ['HOST', 'PORT', 'NAME', 'USER', 'PASSWORD', 'ID']:
-            attvalue = db.get('DATABASE_' + attname, '')
-            setattr(settings, 'DATABASE_' + attname, attvalue)
+            setting_name = "DATABASE_%s" % attname
+            attvalue = db.get(setting_name, '')
+            setattr(settings, setting_name, attvalue)
+            # need to update the DatabaseWrapper cached settings_dict
+            # to keep up to date with the new db setting
+            connection.settings_dict[setting_name] = attvalue
 
     def mark_current_failed(self, automatic=False):
         """Mark the current DB connection as failed.

=== modified file 'identityprovider/tests/test_readonly.py'
--- identityprovider/tests/test_readonly.py	2010-10-14 17:30:00 +0000
+++ identityprovider/tests/test_readonly.py	2011-02-10 20:09:58 +0000
@@ -73,6 +73,7 @@
 
         settings.DATABASE_ID = old_DATABASE_ID
 
+
 class RemoteRequestTest(SQLCachedTestCase):
     msg = 'hello'
     host = 'myhost'
@@ -141,12 +142,18 @@
         settings.READONLY_SECRET = 'testsecret'
         self.orig_db_connections = settings.DB_CONNECTIONS
         settings.DB_CONNECTIONS = [{'DATABASE_ID': 'master',
-                                    'DATABASE_HOST': 'dbhost1',
-                                    'DATABASE_NAME': 'masterdb',
+                                    'DATABASE_HOST': settings.DATABASE_HOST,
+                                    'DATABASE_NAME': settings.DATABASE_NAME,
+                                    'DATABASE_USER': settings.DATABASE_USER,
+                                    'DATABASE_PORT': settings.DATABASE_PORT,
+                                    'DATABASE_PASSWORD': settings.DATABASE_PASSWORD,
                                    },
                                    {'DATABASE_ID': 'slave',
-                                    'DATABASE_HOST': 'dbhost2',
-                                    'DATABASE_NAME': 'slavedb',
+                                    'DATABASE_HOST': settings.DATABASE_HOST,
+                                    'DATABASE_NAME': settings.DATABASE_NAME,
+                                    'DATABASE_USER': settings.DATABASE_USER,
+                                    'DATABASE_PORT': settings.DATABASE_PORT,
+                                    'DATABASE_PASSWORD': settings.DATABASE_PASSWORD,
                                    }]
         self._DBFAILOVER_FLAG_DIR = getattr(settings, 'DBFAILOVER_FLAG_DIR',
                                             None)
@@ -162,6 +169,24 @@
         self.rm.set_db(settings.DB_CONNECTIONS[0])
 
 
+class ReadOnlyManagerTestCase(ReadOnlyBackUp):
+    def test_set_db(self):
+        db = {
+            'DATABASE_ID': 'foo',
+            'DATABASE_HOST': 'foo_host',
+            'DATABASE_PORT': 'foo_port',
+            'DATABASE_NAME': 'foo_name',
+            'DATABASE_USER': 'foo_user',
+            'DATABASE_PASSWORD': 'foo_password',
+        }
+        self.rm.set_db(db)
+
+        from django.db import connection
+        for setting in db:
+            self.assertEqual(getattr(settings, setting), db[setting])
+            self.assertEqual(connection.settings_dict[setting], db[setting])
+
+
 class ReadOnlyRecoveryTestCase(ReadOnlyBackUp):
     def setUp(self):
         super(ReadOnlyRecoveryTestCase, self).setUp()
@@ -233,6 +258,7 @@
     def mock_ping_current_connection(self):
         return False
 
+
 class ReadOnlyFlagFilesTest(ReadOnlyBackUp):
     def test_flag_files_in_right_directory(self):
         self.rm.set_readonly()