From 763726bead1e32180e78b1a06676660f85bb3c89 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Thu, 21 Nov 2024 15:21:03 -0800 Subject: [PATCH 1/2] 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() From 4a905a328228e098fb26396a7ae2d2b063345da4 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Fri, 20 Dec 2024 12:10:45 -0800 Subject: [PATCH 2/2] Update all subcommands to be KolibriCommands if they don't have a command cls. --- kolibri/utils/cli.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/kolibri/utils/cli.py b/kolibri/utils/cli.py index ee3283fe6ba..30b8fc00fe9 100644 --- a/kolibri/utils/cli.py +++ b/kolibri/utils/cli.py @@ -389,7 +389,7 @@ def plugin(): pass -@plugin.command(help="Enable Kolibri plugins") +@plugin.command(cls=KolibriCommand, help="Enable Kolibri plugins") @click.argument("plugin_names", nargs=-1) @click.option("-d", "--default-plugins", default=False, is_flag=True) def enable(plugin_names, default_plugins): @@ -409,7 +409,7 @@ def enable(plugin_names, default_plugins): raise exception -@plugin.command(help="Disable Kolibri plugins") +@plugin.command(cls=KolibriCommand, help="Disable Kolibri plugins") @click.argument("plugin_names", nargs=-1) @click.option("-a", "--all-plugins", default=False, is_flag=True) def disable(plugin_names, all_plugins): @@ -429,7 +429,9 @@ def disable(plugin_names, all_plugins): raise exception -@plugin.command(help="Set Kolibri plugins to be enabled and disable all others") +@plugin.command( + cls=KolibriCommand, help="Set Kolibri plugins to be enabled and disable all others" +) @click.argument("plugin_names", nargs=-1) @click.pass_context def apply(ctx, plugin_names): @@ -451,7 +453,7 @@ def apply(ctx, plugin_names): raise exception -@plugin.command(help="List all available Kolibri plugins") +@plugin.command(cls=KolibriCommand, help="List all available Kolibri plugins") def list(): plugins = [plugin for plugin in iterate_plugins()] lang = "en" @@ -513,7 +515,10 @@ def _get_env_vars(): yield _format_env_var(envvar, v) -@configure.command(help="List all available environment variables to configure Kolibri") +@configure.command( + cls=KolibriCommand, + help="List all available environment variables to configure Kolibri", +) def list_env(): click.echo_via_pager(_get_env_vars())