Skip to content
Permalink
Browse files

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
  • Loading branch information...
foosel committed Dec 16, 2016
1 parent e121569 commit 42e39220534ceb47890564f7b9305340acc47776
Showing with 100 additions and 61 deletions.
  1. +18 −10 src/octoprint/cli/__init__.py
  2. +4 −5 src/octoprint/cli/client.py
  3. +21 −24 src/octoprint/cli/config.py
  4. +3 −3 src/octoprint/cli/plugins.py
  5. +54 −19 src/octoprint/cli/server.py
@@ -47,16 +47,24 @@ def decorator(f):
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
@@ -9,7 +9,7 @@

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)
@@ -9,7 +9,7 @@
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,26 +89,25 @@ 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")
@standard_options(hidden=True)
@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,15 +118,15 @@ 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):
click.echo("Cannot append to non-list value at given path", err=True)
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,15 +147,15 @@ 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):
click.echo("Cannot insert into non-list value at given path", err=True)
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)
@@ -8,7 +8,7 @@
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 @@ def _initialize(self, ctx):
# 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)
@@ -9,7 +9,7 @@
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 log_startup(recorder=None, safe_mode=None, **kwargs):
#~~ 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 run(self):
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()

1 comment on commit 42e3922

@foosel

This comment has been minimized.

Copy link
Owner Author

commented on 42e3922 Dec 16, 2016

... and that commit summary of course should have been "Don't care about ordering of...". TGIF :)

Please sign in to comment.
You can’t perform that action at this time.