From 6d7d6a00cd95ee18e645177e251b83cf699f1639 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 21 Nov 2024 15:21:03 -0800 Subject: [PATCH] Make setting django settings and pythonpath idempotent and reusable. Ensure we validate pythonpath option before settings option. Set python path for all CLI commands so that it can be used for plugins too. --- kolibri/utils/cli.py | 43 ++++++++++++++++++++++++++----------------- kolibri/utils/main.py | 22 ++++++++++------------ 2 files changed, 36 insertions(+), 29 deletions(-) diff --git a/kolibri/utils/cli.py b/kolibri/utils/cli.py index 149e71dcd45..ee3283fe6ba 100644 --- a/kolibri/utils/cli.py +++ b/kolibri/utils/cli.py @@ -2,7 +2,7 @@ import signal import sys import traceback -from pkgutil import find_loader +from contextlib import contextmanager import click from django.core.management import execute_from_command_line @@ -19,32 +19,35 @@ from kolibri.plugins.utils import enable_plugin from kolibri.plugins.utils import iterate_plugins from kolibri.utils import server +from kolibri.utils.compat import module_exists from kolibri.utils.conf import OPTIONS from kolibri.utils.debian_check import check_debian_user from kolibri.utils.main import initialize +from kolibri.utils.main import set_django_settings_and_python_path from kolibri.utils.main import setup_logging logger = logging.getLogger(__name__) -# We use Unicode strings for Python 2.7 throughout the codebase, so choosing -# to silence these warnings. -# Ref: -# https://github.com/learningequality/kolibri/pull/5494#discussion_r318057385 -# https://github.com/PythonCharmers/python-future/issues/22 -click.disable_unicode_literals_warning = True +@contextmanager +def _patch_python_path(pythonpath): + if pythonpath: + sys.path.insert(0, pythonpath) + try: + yield + finally: + if pythonpath: + sys.path.remove(pythonpath) def validate_module(ctx, param, value): if value: - try: - if not find_loader(value): - raise ImportError - except ImportError: - raise click.BadParameter( - "{param} must be a valid python module import path" - ) + with _patch_python_path(ctx.params.get("pythonpath")): + if not module_exists(value): + raise click.BadParameter( + "{param} must be a valid python module import path" + ) return value @@ -72,8 +75,11 @@ def validate_module(ctx, param, value): pythonpath_option = click.Option( param_decls=["--pythonpath"], - type=click.Path(exists=True, file_okay=False), + type=click.Path(exists=True, file_okay=False, resolve_path=True), help="Add a path to the Python path", + # Set this to is_eager to ensure the option is set + # before we attempt to import the settings module + is_eager=True, ) skip_update_option = click.Option( @@ -91,11 +97,10 @@ def validate_module(ctx, param, value): ) -base_params = [debug_option, debug_database_option, noinput_option] +base_params = [debug_option, debug_database_option, noinput_option, pythonpath_option] initialize_params = base_params + [ settings_option, - pythonpath_option, skip_update_option, ] @@ -135,6 +140,8 @@ def invoke(self, ctx): debug=ctx.params.get("debug"), debug_database=ctx.params.get("debug_database"), ) + # We want to allow overriding the Python path for commands that don't require Django + set_django_settings_and_python_path(None, ctx.params.get("pythonpath")) for param in base_params: ctx.params.pop(param.name) return super(KolibriCommand, self).invoke(ctx) @@ -163,6 +170,8 @@ def invoke(self, ctx): debug=ctx.params.get("debug"), debug_database=ctx.params.get("debug_database"), ) + # We want to allow overriding the Python path for commands that don't require Django + set_django_settings_and_python_path(None, ctx.params.get("pythonpath")) for param in base_params: ctx.params.pop(param.name) return super(KolibriGroupCommand, self).invoke(ctx) diff --git a/kolibri/utils/main.py b/kolibri/utils/main.py index 19509d1ff5a..d050b5d3343 100644 --- a/kolibri/utils/main.py +++ b/kolibri/utils/main.py @@ -9,7 +9,6 @@ from django.conf import settings from django.core.exceptions import ValidationError from django.core.management import call_command -from django.core.management.base import handle_default_options from django.db.utils import DatabaseError import kolibri @@ -112,14 +111,6 @@ def _migrate_databases(): call_command("loaddata", "scopedefinitions") -class DefaultDjangoOptions(object): - __slots__ = ["settings", "pythonpath"] - - def __init__(self, settings, pythonpath): - self.settings = settings - self.pythonpath = pythonpath - - def setup_logging(debug=False, debug_database=False): """ Configures logging in cases where a Django environment is not supposed @@ -291,6 +282,15 @@ def _upgrades_after_django_setup(updated, version): logging.error(e) +def set_django_settings_and_python_path(django_settings, pythonpath): + + if django_settings: + os.environ["DJANGO_SETTINGS_MODULE"] = django_settings + + if pythonpath and pythonpath not in sys.path: + sys.path.insert(0, pythonpath) + + def initialize( # noqa C901 skip_update=False, settings=None, @@ -307,9 +307,7 @@ def initialize( # noqa C901 setup_logging(debug=debug, debug_database=debug_database) - default_options = DefaultDjangoOptions(settings, pythonpath) - - handle_default_options(default_options) + set_django_settings_and_python_path(settings, pythonpath) version = get_version()