Allow configuring database engine via db config key
#8287
Conversation
MagicRB
force-pushed
the
allow-configuring-db-engine
branch
from
81f2d41 to
8f8546e
Compare
Signed-off-by: magic_rb <richard@brezak.sk>
MagicRB
force-pushed
the
allow-configuring-db-engine
branch
from
8f8546e to
b152da0
Compare
|
Just to confirm - have you checked that it's not possible to pass the DB client configuration that you need via the URL parameters? There's fair amount of configuration possible, e.g. |
|
|
||
| def load_db(self, filename, config_dict): | ||
| self.db = {"db_url": self.getDbUrlFromConfig(config_dict)} | ||
| self.db = self.getDbFromConfig(config_dict) |
| @@ -85,7 +85,7 @@ def __init__(self, basedir): | |||
|
|
|||
| # not configured yet - we don't build an engine until the first | |||
| # reconfig | |||
| self.configured_url = None | |||
| self.configured_db = None | |||
|
|
||
| db_url = self.configured_url | ||
| db_config = dict(self.configured_db) |
There was a problem hiding this comment.
I think self.configured_db.copy() would be more explicit of what's happening
| p = Properties() | ||
| p.master = self | ||
| return p.render(new_config.db['db_url']) |
There was a problem hiding this comment.
You can simply return p.render(new_config.db) which will render values of the dict automatically
|
|
||
| db_url = self.configured_url | ||
| db_config = dict(self.configured_db) |
There was a problem hiding this comment.
I think that we should put sqlalchemy kwargs into separate key. E.g. engine_kwargs to make it clear where they are used. This would make buildbot config structure more self descriptive.
|
Overall idea makes sense, I left several comments. |
Contributor Checklist:
newsfragmentsdirectory (and read theREADME.txtin that directory)_
So I'm not sure if this is the way to do it,
config['db']goes from only havingdb_urlas the only allowed key to allowing anything and passing all of it except fordb_urlto the backing DB engine. If this approach is fine ill write up the docs, newsfragments and all the rest.