launchpad-reviewers team mailing list archive

[Colin Watson <mp+439909@xxxxxxxxxxxxxxxxxx>]

The proposal to merge ~cjwatson/launchpad:sqlbase-connect-set-role-after-connecting into launchpad:master has been updated.

Description changed to:

Commit 31b283765f2cb0c3c6551c46c02dd84cd51b505e reworked `LaunchpadDatabase.raw_connect` to add the ability to switch database role after connecting; this covers everything that uses the Storm store interface.  However, a few scripts (including those that run without the Zope component architecture and those that do some particularly arcane things with database connections) construct database connections a little more directly, typically via `lp.services.database.sqlbase.connect`.  The one I've noticed is `librarian-gc`, which failed to work in a charmed deployment configured with `set_role_after_connecting: True`.

To fix this, rework `sqlbase.connect` so that it honours `set_role_after_connecting` as well.  Ideally we'd now also rework `LaunchpadDatabase.raw_connect` on top of `sqlbase.connect` so that we only have one implementation of this logic, but that's difficult since in that case the actual connection is made by code in a base class, so I've just left an XXX comment for now.

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/439909
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:sqlbase-connect-set-role-after-connecting into launchpad:master.