Regression in [13888]: `parse_connection_uri` may trim `params` for the returned dictionary

[pierre-yves@…]

13888 is assuming that the dictionary returned by parse_connection_uri contains a params key, replacing the call .setdefault('params', {}) by ['params'].

However, parse_connection_uri trims the returned dictionary from the key-value pairs for values coercible to False. The default value of params (= {}) is coercible to False.

Either parse_connection_uri should not trim such values, either 13888 should be reverted.

This currently makes the backup command unusable under some conditions.

[Jun Omae]

Okay. You're right. Reproduced using database connection string without parameters.

>>> from trac.test import EnvironmentStub
>>> env = EnvironmentStub()
>>> env.config.set('trac', 'database', 'postgres://user:password@127.0.0.1/tracdb')
>>> env.backup()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "trac/env.py", line 719, in backup
    return DatabaseManager(self).backup(dest)
  File "trac/db/api.py", line 523, in backup
    return connector.backup(dest)
  File "trac/db/postgres_backend.py", line 177, in backup
    db_params = db_prop['params']
KeyError: 'params'

[Ryan J Ollos]

Reverting change on line 165 of [13888] seems like the best option. I'm considering whether parse_connection_uri should at least include dictionary entries for parameters that are relevant for a particular scheme, but not sure we should change the behavior of the function.

[Ryan J Ollos]

Regression fixed in [13992]. I'm leaving the ticket open to commit regressions tests, provided I can find time for that before the conclusion of 1.1.5.