From 42e39220534ceb47890564f7b9305340acc47776 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gina=20H=C3=A4u=C3=9Fge?= Date: Fri, 16 Dec 2016 12:18:05 +0100 Subject: [PATCH] Don't care about common params on CLI --basedir, --config, --verbose, --safe may now come before or after subcommands and should still be evaluated. For the server commands (legacy, "server" and "daemon"), the same should now hold true for the related parameters --host, --port, --debug, --logging, --iknowwhatimdoing and also --pid (for daemon command). While having the parameters belong to the individual commands and only there (which is click's basic approach) is way more cleaner, too many people were running into issues with that strict approach after all. I just hope the somewhat hackish approach with context injection needed to get the less strict version to work won't backfire badly in the long run. See also #1633 and #1657 --- src/octoprint/cli/__init__.py | 28 +++++++++----- src/octoprint/cli/client.py | 9 ++--- src/octoprint/cli/config.py | 45 ++++++++++----------- src/octoprint/cli/plugins.py | 6 +-- src/octoprint/cli/server.py | 73 ++++++++++++++++++++++++++--------- 5 files changed, 100 insertions(+), 61 deletions(-) diff --git a/src/octoprint/cli/__init__.py b/src/octoprint/cli/__init__.py index 91c6b55b..a6156b46 100644 --- a/src/octoprint/cli/__init__.py +++ b/src/octoprint/cli/__init__.py @@ -47,16 +47,24 @@ def hidden_option(*param_decls, **attrs): return f return decorator -#~~ helper for settings context options +#~~ helper for setting context options def set_ctx_obj_option(ctx, param, value): """Helper for setting eager options on the context.""" if ctx.obj is None: ctx.obj = OctoPrintContext() - - if hasattr(ctx.obj, param.name): + if value != param.default: setattr(ctx.obj, param.name, value) +#~~ helper for retrieving context options + +def get_ctx_obj_option(ctx, key, default, include_parents=True): + if include_parents and hasattr(ctx, "parent") and ctx.parent: + fallback = get_ctx_obj_option(ctx.parent, key, default) + else: + fallback = default + return getattr(ctx.obj, key, fallback) + #~~ helper for setting a lot of bulk options def bulk_options(options): @@ -105,13 +113,13 @@ def standard_options(hidden=False): #~~ helper for settings legacy options we still have to support on "octoprint" legacy_options = bulk_options([ - hidden_option("--host", type=click.STRING), - hidden_option("--port", type=click.INT), - hidden_option("--logging", type=click.Path()), - hidden_option("--debug", "-d", is_flag=True), - hidden_option("--daemon", type=click.Choice(["start", "stop", "restart"])), - hidden_option("--pid", type=click.Path(), default="/tmp/octoprint.pid"), - hidden_option("--iknowwhatimdoing", "allow_root", is_flag=True), + hidden_option("--host", type=click.STRING, callback=set_ctx_obj_option), + hidden_option("--port", type=click.INT, callback=set_ctx_obj_option), + hidden_option("--logging", type=click.Path(), callback=set_ctx_obj_option), + hidden_option("--debug", "-d", is_flag=True, callback=set_ctx_obj_option), + hidden_option("--daemon", type=click.Choice(["start", "stop", "restart"]), callback=set_ctx_obj_option), + hidden_option("--pid", type=click.Path(), default="/tmp/octoprint.pid", callback=set_ctx_obj_option), + hidden_option("--iknowwhatimdoing", "allow_root", is_flag=True, callback=set_ctx_obj_option), ]) """Legacy options available directly on the "octoprint" command in earlier versions. Kept available for reasons of backwards compatibility, but hidden from the diff --git a/src/octoprint/cli/client.py b/src/octoprint/cli/client.py index dc77f164..cafcd181 100644 --- a/src/octoprint/cli/client.py +++ b/src/octoprint/cli/client.py @@ -9,7 +9,7 @@ import json import octoprint_client -from octoprint.cli import pass_octoprint_ctx, bulk_options, standard_options +from octoprint.cli import get_ctx_obj_option from octoprint import init_settings, FatalStartupError @@ -35,13 +35,12 @@ def client_commands(): @click.option("--httppass", type=click.STRING) @click.option("--https", is_flag=True) @click.option("--prefix", type=click.STRING) -@pass_octoprint_ctx @click.pass_context -def client(ctx, obj, host, port, httpuser, httppass, https, prefix): +def client(ctx, host, port, httpuser, httppass, https, prefix): """Basic API client.""" try: - obj.settings = init_settings(obj.basedir, obj.configfile) - octoprint_client.init_client(obj.settings, https=https, httpuser=httpuser, httppass=httppass, host=host, port=port, prefix=prefix) + ctx.obj.settings = init_settings(get_ctx_obj_option(ctx, "basedir", None), get_ctx_obj_option(ctx, "configfile", None)) + octoprint_client.init_client(ctx.obj.settings, https=https, httpuser=httpuser, httppass=httppass, host=host, port=port, prefix=prefix) except FatalStartupError as e: click.echo(e.message, err=True) click.echo("There was a fatal error initializing the client.", err=True) diff --git a/src/octoprint/cli/config.py b/src/octoprint/cli/config.py index de0250cd..5e21aad8 100644 --- a/src/octoprint/cli/config.py +++ b/src/octoprint/cli/config.py @@ -9,7 +9,7 @@ import click import logging from octoprint import init_settings, FatalStartupError -from octoprint.cli import pass_octoprint_ctx, standard_options, bulk_options +from octoprint.cli import pass_octoprint_ctx, standard_options, bulk_options, get_ctx_obj_option import yaml import json @@ -46,13 +46,12 @@ def config_commands(): pass @config_commands.group(name="config") -@pass_octoprint_ctx @click.pass_context -def config(ctx, obj): +def config(ctx): """Basic config manipulation.""" - logging.basicConfig(level=logging.DEBUG if obj.verbosity > 0 else logging.WARN) + logging.basicConfig(level=logging.DEBUG if get_ctx_obj_option(ctx, "verbosity", 0) > 0 else logging.WARN) try: - obj.settings = init_settings(obj.basedir, obj.configfile) + ctx.obj.settings = init_settings(get_ctx_obj_option(ctx, "basedir", None), get_ctx_obj_option(ctx, "configfile", None)) except FatalStartupError as e: click.echo(e.message, err=True) click.echo("There was a fatal error initializing the client.", err=True) @@ -73,7 +72,7 @@ def config(ctx, obj): help="Parse value from json") @pass_octoprint_ctx @click.pass_context -def set_command(ctx, obj, path, value, as_bool, as_float, as_int, as_json): +def set_command(ctx, path, value, as_bool, as_float, as_int, as_json): """Sets a config path to the provided value.""" if as_json: try: @@ -90,16 +89,16 @@ def set_command(ctx, obj, path, value, as_bool, as_float, as_int, as_json): elif as_int: data_type = int - _set_helper(obj.settings, path, value, data_type=data_type) + _set_helper(ctx.obj.settings, path, value, data_type=data_type) @config.command(name="remove") @standard_options(hidden=True) @click.argument("path", type=click.STRING) -@pass_octoprint_ctx -def remove_command(obj, path): +@click.pass_context +def remove_command(ctx, path): """Removes a config path.""" - _set_helper(obj.settings, path, None) + _set_helper(ctx.obj.settings, path, None) @config.command(name="append_value") @@ -107,9 +106,8 @@ def remove_command(obj, path): @click.argument("path", type=click.STRING) @click.argument("value", type=click.STRING) @click.option("--json", "as_json", is_flag=True) -@pass_octoprint_ctx @click.pass_context -def append_value_command(ctx, obj, path, value, as_json=False): +def append_value_command(ctx, path, value, as_json=False): """Appends value to list behind config path.""" path = _to_settings_path(path) @@ -120,7 +118,7 @@ def append_value_command(ctx, obj, path, value, as_json=False): click.echo(e.message, err=True) ctx.exit(-1) - current = obj.settings.get(path) + current = ctx.obj.settings.get(path) if current is None: current = [] if not isinstance(current, list): @@ -128,7 +126,7 @@ def append_value_command(ctx, obj, path, value, as_json=False): ctx.exit(-1) current.append(value) - _set_helper(obj.settings, path, current) + _set_helper(ctx.obj.settings, path, current) @config.command(name="insert_value") @@ -137,9 +135,8 @@ def append_value_command(ctx, obj, path, value, as_json=False): @click.argument("index", type=click.INT) @click.argument("value", type=click.STRING) @click.option("--json", "as_json", is_flag=True) -@pass_octoprint_ctx @click.pass_context -def insert_value_command(ctx, obj, path, index, value, as_json=False): +def insert_value_command(ctx, path, index, value, as_json=False): """Inserts value at index of list behind config key.""" path = _to_settings_path(path) @@ -150,7 +147,7 @@ def insert_value_command(ctx, obj, path, index, value, as_json=False): click.echo(e.message, err=True) ctx.exit(-1) - current = obj.settings.get(path) + current = ctx.obj.settings.get(path) if current is None: current = [] if not isinstance(current, list): @@ -158,7 +155,7 @@ def insert_value_command(ctx, obj, path, index, value, as_json=False): ctx.exit(-1) current.insert(index, value) - _set_helper(obj.settings, path, current) + _set_helper(ctx.obj.settings, path, current) @config.command(name="remove_value") @@ -168,7 +165,7 @@ def insert_value_command(ctx, obj, path, index, value, as_json=False): @click.option("--json", "as_json", is_flag=True) @pass_octoprint_ctx @click.pass_context -def remove_value_command(ctx, obj, path, value, as_json=False): +def remove_value_command(ctx, path, value, as_json=False): """Removes value from list at config path.""" path = _to_settings_path(path) @@ -179,7 +176,7 @@ def remove_value_command(ctx, obj, path, value, as_json=False): click.echo(e.message, err=True) ctx.exit(-1) - current = obj.settings.get(path) + current = ctx.obj.settings.get(path) if current is None: current = [] if not isinstance(current, list): @@ -191,7 +188,7 @@ def remove_value_command(ctx, obj, path, value, as_json=False): ctx.exit() current.remove(value) - _set_helper(obj.settings, path, current) + _set_helper(ctx.obj.settings, path, current) @config.command(name="get") @@ -203,11 +200,11 @@ def remove_value_command(ctx, obj, path, value, as_json=False): @click.option("--raw", "as_raw", is_flag=True, help="Output value as raw string representation") @standard_options(hidden=True) -@pass_octoprint_ctx -def get_command(obj, path, as_json=False, as_yaml=False, as_raw=False): +@click.pass_context +def get_command(ctx, path, as_json=False, as_yaml=False, as_raw=False): """Retrieves value from config path.""" path = _to_settings_path(path) - value = obj.settings.get(path, merged=True) + value = ctx.obj.settings.get(path, merged=True) if as_json: output = json.dumps(value) diff --git a/src/octoprint/cli/plugins.py b/src/octoprint/cli/plugins.py index 44d60952..8b13eb1c 100644 --- a/src/octoprint/cli/plugins.py +++ b/src/octoprint/cli/plugins.py @@ -8,7 +8,7 @@ __copyright__ = "Copyright (C) 2015 The OctoPrint Project - Released under terms import click import logging -from octoprint.cli import pass_octoprint_ctx, OctoPrintContext +from octoprint.cli import pass_octoprint_ctx, OctoPrintContext, get_ctx_obj_option #~~ "octoprint plugin:command" commands @@ -50,8 +50,8 @@ class OctoPrintPluginCommands(click.MultiCommand): # context (basedir and configfile) from octoprint import init_settings, init_pluginsystem, FatalStartupError try: - self.settings = init_settings(ctx.obj.basedir, ctx.obj.configfile) - self.plugin_manager = init_pluginsystem(self.settings, safe_mode=ctx.obj.safe_mode) + self.settings = init_settings(get_ctx_obj_option(ctx, "basedir", None), get_ctx_obj_option(ctx, "configfile", None)) + self.plugin_manager = init_pluginsystem(self.settings, safe_mode=get_ctx_obj_option(ctx, "safe_mode", False)) except FatalStartupError as e: click.echo(e.message, err=True) click.echo("There was a fatal error initializing the settings or the plugin system.", err=True) diff --git a/src/octoprint/cli/server.py b/src/octoprint/cli/server.py index 2ea6fc4b..d2319136 100644 --- a/src/octoprint/cli/server.py +++ b/src/octoprint/cli/server.py @@ -9,7 +9,7 @@ import click import logging import sys -from octoprint.cli import pass_octoprint_ctx, bulk_options, standard_options +from octoprint.cli import bulk_options, standard_options, set_ctx_obj_option, get_ctx_obj_option def run_server(basedir, configfile, host, port, debug, allow_root, logging_config, verbosity, safe_mode, octoprint_daemon=None): """Initializes the environment and starts up the server.""" @@ -69,51 +69,87 @@ def run_server(basedir, configfile, host, port, debug, allow_root, logging_confi #~~ server options server_options = bulk_options([ - click.option("--host", type=click.STRING, + click.option("--host", type=click.STRING, callback=set_ctx_obj_option, help="Specify the host on which to bind the server."), - click.option("--port", type=click.INT, + click.option("--port", type=click.INT, callback=set_ctx_obj_option, help="Specify the port on which to bind the server."), - click.option("--logging", type=click.Path(), + click.option("--logging", type=click.Path(), callback=set_ctx_obj_option, help="Specify the config file to use for configuring logging."), - click.option("--iknowwhatimdoing", "allow_root", is_flag=True, + click.option("--iknowwhatimdoing", "allow_root", is_flag=True, callback=set_ctx_obj_option, help="Allow OctoPrint to run as user root."), - click.option("--debug", is_flag=True, help="Enable debug mode.") + click.option("--debug", is_flag=True, callback=set_ctx_obj_option, + help="Enable debug mode.") ]) """Decorator to add the options shared among the server commands: ``--host``, ``--port``, ``--logging``, ``--iknowwhatimdoing`` and ``--debug``.""" +daemon_options = bulk_options([ + click.option("--pid", type=click.Path(), default="/tmp/octoprint.pid", callback=set_ctx_obj_option, + help="Pidfile to use for daemonizing.") +]) +"""Decorator to add the options for the daemon subcommand: ``--pid``.""" + #~~ "octoprint serve" and "octoprint daemon" commands @click.group() -@pass_octoprint_ctx -def server_commands(obj): +def server_commands(): pass @server_commands.command(name="serve") @server_options @standard_options(hidden=True) -@pass_octoprint_ctx -def serve_command(obj, host, port, logging, allow_root, debug): +@click.pass_context +def serve_command(ctx, **kwargs): """Starts the OctoPrint server.""" - run_server(obj.basedir, obj.configfile, host, port, debug, - allow_root, logging, obj.verbosity, obj.safe_mode) + + def get_value(key): + return get_ctx_obj_option(ctx, key, kwargs.get(key)) + + host = get_value("host") + port = get_value("port") + logging = get_value("logging") + allow_root = get_value("allow_root") + debug = get_value("debug") + + basedir = get_value("basedir") + configfile = get_value("configfile") + verbosity = get_value("verbosity") + safe_mode = get_value("safe_mode") + + run_server(basedir, configfile, host, port, debug, + allow_root, logging, verbosity, safe_mode) @server_commands.command(name="daemon") -@click.option("--pid", type=click.Path(), default="/tmp/octoprint.pid", - help="Pidfile to use for daemonizing.") @server_options +@daemon_options @standard_options(hidden=True) @click.argument("command", type=click.Choice(["start", "stop", "restart", "status"]), metavar="start|stop|restart|status") -@pass_octoprint_ctx -def daemon_command(octoprint_ctx, pid, host, port, logging, allow_root, debug, command): +@click.pass_context +def daemon_command(ctx, command, **kwargs): """ Starts, stops or restarts in daemon mode. Please note that daemon mode is only supported under Linux right now. """ + + def get_value(key): + return get_ctx_obj_option(ctx, key, kwargs.get(key)) + + host = get_value("host") + port = get_value("port") + logging = get_value("logging") + allow_root = get_value("allow_root") + debug = get_value("debug") + pid = get_value("pid") + + basedir = get_value("basedir") + configfile = get_value("configfile") + verbosity = get_value("verbosity") + safe_mode = get_value("safe_mode") + if sys.platform == "darwin" or sys.platform == "win32": click.echo("Sorry, daemon mode is only supported under Linux right now", file=sys.stderr) @@ -144,9 +180,8 @@ def daemon_command(octoprint_ctx, pid, host, port, logging, allow_root, debug, c self._allow_root, self._logging_config, self._verbosity, self._safe_mode, octoprint_daemon=self) - octoprint_daemon = OctoPrintDaemon(pid, octoprint_ctx.basedir, octoprint_ctx.configfile, - host, port, debug, allow_root, logging, octoprint_ctx.verbosity, - octoprint_ctx.safe_mode) + octoprint_daemon = OctoPrintDaemon(pid, basedir, configfile, host, port, debug, allow_root, logging, verbosity, + safe_mode) if command == "start": octoprint_daemon.start()