Skip to content

Commit

Permalink
[FIX] core: schrodinger's -d/--database/db_name
Browse files Browse the repository at this point in the history
The -d/--database option is used to configure the db_name option. At
various places (essentially within the server subcommand) that option is
used as a comma-separated list of database names and is iterated over.
In other places (essentially within other subcommands) that option is
used as a single database, a non-comma-separated string and is used
as-is.

This work cleans up the mess and makes it always a list. In places where
a single value was expected, a warning is now emitted and the first item
of the list is used.
  • Loading branch information
Julien00859 committed Sep 22, 2023
1 parent adbfbbb commit 4832df6
Show file tree
Hide file tree
Showing 12 changed files with 76 additions and 44 deletions.
4 changes: 2 additions & 2 deletions addons/web/tests/test_db_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ def setUp(self):
)
self.startPatcher(self.verify_admin_password_patcher)

self.db_name = config['db_name']
self.assertTrue(self.db_name)
self.assertEqual(len(config['db_name']), 1)
self.db_name = config['db_name'][0]

# monkey-patch db-filter
self.addCleanup(operator.setitem, config, 'dbfilter', config['dbfilter'])
Expand Down
2 changes: 1 addition & 1 deletion odoo/cli/cloc.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def run(self, args):
c = cloc.Cloc()
if opt.database:
config.parse_config(['-d', opt.database] + unknown)
c.count_database(opt.database)
c.count_database()
if opt.path:
for i in opt.path:
c.count_path(i)
Expand Down
9 changes: 7 additions & 2 deletions odoo/cli/neutralize.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,15 @@ def run(self, args):
parser.add_option_group(group)
opt = odoo.tools.config.parse_config(args)

dbname = odoo.tools.config['db_name']
if not dbname:
dbnames = odoo.tools.config['db_name']
if not dbnames:
_logger.error('Neutralize command needs a database name. Use "-d" argument')
sys.exit(1)
if len(dbnames) > 1:
_logger.warning(
"Multiple databases provided with %s, only the first one %r will be neutralised",
odoo.tools.config.options_index['db_name'], dbnames[0])
dbname = dbnames[0]

if not opt.to_stdout:
_logger.info("Starting %s database neutralization", dbname)
Expand Down
12 changes: 11 additions & 1 deletion odoo/cli/populate.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,17 @@ def run(self, cmdargs):
parser.add_option_group(group)
opt = odoo.tools.config.parse_config(cmdargs)
populate_models = opt.populate_models and set(opt.populate_models.split(','))
dbname = odoo.tools.config['db_name']

dbnames = odoo.tools.config['db_name']
if not dbnames:
_logger.error('Populate command needs a database name. Use "-d" argument')
sys.exit(1)
if len(dbnames) > 1:
_logger.warning(
"Multiple databases provided with %s, only the first one %r will be populated",
odoo.tools.config.options_index['db_name'], dbnames[0])
dbname = dbnames[0]

registry = odoo.registry(dbname)
with registry.cursor() as cr:
env = odoo.api.Environment(cr, odoo.SUPERUSER_ID, {})
Expand Down
52 changes: 30 additions & 22 deletions odoo/cli/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,12 @@ def setup_pid_file():

def export_translation():
config = odoo.tools.config
dbname = config['db_name']
dbnames = config['db_name']
if len(dbnames) > 1:
_logger.warning(
"Multiple databases provided with %s, only the terms of the first one %r will be exported",
config.options_index['db_name'], dbnames[0])
dbname = dbnames[0]

if config["language"]:
msg = "language %s" % (config["language"],)
Expand All @@ -113,7 +118,12 @@ def export_translation():
def import_translation():
config = odoo.tools.config
overwrite = config["overwrite_existing_translations"]
dbname = config['db_name']
dbnames = config['db_name']
if len(dbnames) > 1:
_logger.warning(
"Multiple databases provided with %s, only the terms of the first one %r will be exported",
config.options_index['db_name'], dbnames[0])
dbname = dbnames[0]

registry = odoo.modules.registry.Registry.new(dbname)
with registry.cursor() as cr:
Expand All @@ -134,25 +144,23 @@ def main(args):
# bit overkill, but better safe than sorry I guess
csv.field_size_limit(500 * 1024 * 1024)

preload = []
if config['db_name']:
preload = config['db_name'].split(',')
for db_name in preload:
try:
odoo.service.db._create_empty_database(db_name)
for mod in config['server_wide_modules']:
config['init'][mod] = True
except ProgrammingError as err:
if err.pgcode == errorcodes.INSUFFICIENT_PRIVILEGE:
# We use an INFO loglevel on purpose in order to avoid
# reporting unnecessary warnings on build environment
# using restricted database access.
_logger.info("Could not determine if database %s exists, "
"skipping auto-creation: %s", db_name, err)
else:
raise err
except odoo.service.db.DatabaseExists:
pass
# preload
for db_name in config['db_name']:
try:
odoo.service.db._create_empty_database(db_name)
for mod in config['server_wide_modules']:
config['init'][mod] = True
except ProgrammingError as err:
if err.pgcode == errorcodes.INSUFFICIENT_PRIVILEGE:
# We use an INFO loglevel on purpose in order to avoid
# reporting unnecessary warnings on build environment
# using restricted database access.
_logger.info("Could not determine if database %s exists, "
"skipping auto-creation: %s", db_name, err)
else:
raise err
except odoo.service.db.DatabaseExists:
pass

if config["translate_out"]:
export_translation()
Expand All @@ -170,7 +178,7 @@ def main(args):
stop = config["stop_after_init"]

setup_pid_file()
rc = odoo.service.server.start(preload=preload, stop=stop)
rc = odoo.service.server.start(preload=list(config['db_name']), stop=stop)
sys.exit(rc)

class Server(Command):
Expand Down
11 changes: 10 additions & 1 deletion odoo/cli/shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,5 +120,14 @@ def shell(self, dbname):

def run(self, args):
self.init(args)
self.shell(config['db_name'])

dbnames = config['db_name']
if len(dbnames) > 1:
_logger.warning(
"Multiple databases provided with %s, the shell will open on the first one %r",
odoo.tools.config.options_index['db_name'], dbnames[0])
if not dbnames:
self.shell(None)
else:
self.shell(dbnames[0])
return 0
2 changes: 1 addition & 1 deletion odoo/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ def db_filter(dbs, host=None):
if config['db_name']:
# In case --db-filter is not provided and --database is passed, Odoo will
# use the value of --database as a comma separated list of exposed databases.
exposed_dbs = {db.strip() for db in config['db_name'].split(',')}
exposed_dbs = {db.strip() for db in config['db_name']}
return sorted(exposed_dbs.intersection(dbs))

return list(dbs)
Expand Down
3 changes: 1 addition & 2 deletions odoo/service/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -398,8 +398,7 @@ def list_dbs(force=False):
# In case --db-filter is not provided and --database is passed, Odoo will not
# fetch the list of databases available on the postgres server and instead will
# use the value of --database as comma seperated list of exposed databases.
res = sorted(db.strip() for db in odoo.tools.config['db_name'].split(','))
return res
return sorted(odoo.tools.config['db_name'])

chosen_template = odoo.tools.config['db_template']
templates_list = tuple(set(['postgres', chosen_template]))
Expand Down
2 changes: 1 addition & 1 deletion odoo/service/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -1189,7 +1189,7 @@ def sleep(self):

def _db_list(self):
if config['db_name']:
db_names = config['db_name'].split(',')
db_names = list(config['db_name'])
else:
db_names = odoo.service.db.list_dbs(True)
return db_names
Expand Down
10 changes: 5 additions & 5 deletions odoo/tests/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,11 @@ def get_db_name():
# database from XML-RPC).
if not db and hasattr(threading.current_thread(), 'dbname'):
return threading.current_thread().dbname
return db
if len(db) > 1:
_logger.warning(
"Multiple databases provided with %s, the first one %r is returned by %s",
odoo.tools.config.options_index['db_name'], db[0], get_db_name)
return db[0]


standalone_tests = defaultdict(list)
Expand All @@ -137,10 +141,6 @@ def register(func):
return register


# For backwards-compatibility - get_db_name() should be used instead
DB = get_db_name()


def new_test_user(env, login='', groups='base.group_user', context=None, **kwargs):
""" Helper function to create a new test user. It allows to quickly create
users given its login and groups (being a comma separated list of xml ids).
Expand Down
6 changes: 4 additions & 2 deletions odoo/tools/cloc.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,8 +286,10 @@ def count_env(self, env):
self.count_modules(env)
self.count_customization(env)

def count_database(self, database):
registry = odoo.registry(config['db_name'])
def count_database(self, database=None):
if database is None:
database = config['db_name'][0]
registry = odoo.registry(database)
with registry.cursor() as cr:
uid = odoo.SUPERUSER_ID
env = odoo.api.Environment(cr, uid, {})
Expand Down
7 changes: 3 additions & 4 deletions odoo/tools/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,8 @@ def _build_cli(self):

# Database Group
group = optparse.OptionGroup(parser, "Database related options")
group.add_option("-d", "--database", dest="db_name",
help="specify the database name")
group.add_option("-d", "--database", dest="db_name", type='comma', my_default=[],
help="database(s) used when installing or updating modules. Providing a comma-separated list restrict access to databases provided in list.")
group.add_option("-r", "--db_user", dest="db_user",
help="specify the database user name")
group.add_option("-w", "--db_password", dest="db_password",
Expand Down Expand Up @@ -450,8 +450,7 @@ def die(cond, msg):
self._load_cli_options(opt)
self._load_file_options(self['config'])

ismultidb = ',' in (self.options.get('db_name') or '')
die(ismultidb and (self['init'] or self['update']),
die(len(self['db_name']) > 1 and (self['init'] or self['update']),
"Cannot use -i/--init or -u/--update with multiple databases in the -d/--database/db_name")

self._postprocess_options()
Expand Down

0 comments on commit 4832df6

Please sign in to comment.