diff --git a/NEWS b/NEWS index 3753462c..2f3b8650 100644 --- a/NEWS +++ b/NEWS @@ -13,6 +13,8 @@ What's new in psycopg 2.4.6 Thanks to Manu Cupcic for the report (ticket #110). - 'register_hstore()', 'register_composite()', 'tpc_recover()' work with RealDictConnection and Cursor (ticket #114). + - connect() raises an exception instead of swallowing keyword arguments + when a connection string is specified as well (ticket #131). - 'errorcodes' map updated to PostgreSQL 9.2. diff --git a/lib/__init__.py b/lib/__init__.py index 0a8ed0f5..892d8038 100644 --- a/lib/__init__.py +++ b/lib/__init__.py @@ -146,37 +146,36 @@ def connect(dsn=None, Using *async*=True an asynchronous connection will be created. Any other keyword parameter will be passed to the underlying client - library: the list of supported parameter depends on the library version. + library: the list of supported parameters depends on the library version. """ + items = [] + if database is not None: + items.append(('dbname', database)) + if user is not None: + items.append(('user', user)) + if password is not None: + items.append(('password', password)) + if host is not None: + items.append(('host', host)) + if port is not None: + items.append(('port', port)) + + items.extend([(k, v) for (k, v) in kwargs.iteritems() if v is not None]) + + if dsn is not None and items: + raise TypeError( + "'%s' is an invalid keyword argument when the dsn is specified" + % items[0][0]) + if dsn is None: - # Note: reproducing the behaviour of the previous C implementation: - # keyword are silently swallowed if a DSN is specified. I would have - # raised an exception. File under "histerical raisins". - items = [] - if database is not None: - items.append(('dbname', database)) - if user is not None: - items.append(('user', user)) - if password is not None: - items.append(('password', password)) - if host is not None: - items.append(('host', host)) - # Reproducing the previous C implementation behaviour: swallow a - # negative port. The libpq would raise an exception for it. - if port is not None and int(port) > 0: - items.append(('port', port)) + if not items: + raise TypeError('missing dsn and no parameters') + else: + dsn = " ".join(["%s=%s" % (k, _param_escape(str(v))) + for (k, v) in items]) - items.extend( - [(k, v) for (k, v) in kwargs.iteritems() if v is not None]) - dsn = " ".join(["%s=%s" % (k, _param_escape(str(v))) - for (k, v) in items]) - - if not dsn: - raise InterfaceError('missing dsn and no parameters') - - return _connect(dsn, - connection_factory=connection_factory, async=async) + return _connect(dsn, connection_factory=connection_factory, async=async) __all__ = filter(lambda k: not k.startswith('_'), locals().keys()) diff --git a/tests/test_module.py b/tests/test_module.py index 870dbbc2..4083c368 100755 --- a/tests/test_module.py +++ b/tests/test_module.py @@ -40,10 +40,10 @@ class ConnectTestCase(unittest.TestCase): psycopg2._connect = self._connect_orig def test_there_has_to_be_something(self): - self.assertRaises(psycopg2.InterfaceError, psycopg2.connect) - self.assertRaises(psycopg2.InterfaceError, psycopg2.connect, + self.assertRaises(TypeError, psycopg2.connect) + self.assertRaises(TypeError, psycopg2.connect, connection_factory=lambda dsn, async=False: None) - self.assertRaises(psycopg2.InterfaceError, psycopg2.connect, + self.assertRaises(TypeError, psycopg2.connect, async=True) def test_no_keywords(self): @@ -127,6 +127,14 @@ class ConnectTestCase(unittest.TestCase): psycopg2.connect(database=r"\every thing'") self.assertEqual(self.args[0], r"dbname='\\every thing\''") + def test_no_kwargs_swallow(self): + self.assertRaises(TypeError, + psycopg2.connect, 'dbname=foo', database='foo') + self.assertRaises(TypeError, + psycopg2.connect, 'dbname=foo', user='postgres') + self.assertRaises(TypeError, + psycopg2.connect, 'dbname=foo', no_such_param='meh') + class ExceptionsTestCase(unittest.TestCase): def setUp(self):