From d42a0c33213f8ac167cc3142c1ccdfb2db5605c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Armando=20Garc=C3=ADa=20Sancio?= Date: Sat, 21 Mar 2015 11:13:10 -0700 Subject: [PATCH] dcos-588 Improve config subcommand There is quite a bit going on in this commit. At a high-level this improves the config subcommand so that it can support more value type and not just strings. To do this we need to know the schema for each subsection. The config subcommand queries all of the subcommands for this information. Here are all of the changes includes in this commit: * Fix the makefile so that the test, doc and packages target depend on env. * Add support for append, prepend, unset a list element and validate in the config subcommand. * Extend the marathon, package and subcommand subcommands so that they return the schema for their section of the config file. * Adds config schema file the python package. * The module jsonschema returns poorly formatted validation errors. This commit includes regular expression to clean up those messages. --- Makefile | 6 +- cli/Makefile | 6 +- cli/dcoscli/config/main.py | 220 ++++++++++++++- cli/dcoscli/data/config-schema/marathon.json | 22 ++ cli/dcoscli/data/config-schema/package.json | 24 ++ .../data/config-schema/subcommand.json | 12 + cli/dcoscli/main.py | 22 +- cli/dcoscli/marathon/main.py | 128 +++++---- cli/dcoscli/package/main.py | 51 +++- cli/dcoscli/subcommand/main.py | 27 ++ cli/setup.py | 5 +- cli/tests/data/dcos.toml | 8 +- cli/tests/integrations/cli/test_config.py | 263 ++++++++++++++++-- cli/tests/integrations/cli/test_marathon.py | 31 ++- cli/tests/integrations/cli/test_package.py | 7 +- dcos/api/emitting.py | 14 +- dcos/api/jsonitem.py | 59 ++-- dcos/api/subcommand.py | 46 ++- dcos/api/util.py | 23 +- setup.py | 2 +- tests/test_jsonitem.py | 4 +- 21 files changed, 809 insertions(+), 171 deletions(-) create mode 100644 cli/dcoscli/data/config-schema/marathon.json create mode 100644 cli/dcoscli/data/config-schema/package.json create mode 100644 cli/dcoscli/data/config-schema/subcommand.json diff --git a/Makefile b/Makefile index 5eafd4e..9a4a81a 100644 --- a/Makefile +++ b/Makefile @@ -6,11 +6,11 @@ clean: env: bin/env.sh -test: +test: env bin/test.sh -doc: +doc: env bin/doc.sh -packages: +packages: env bin/packages.sh diff --git a/cli/Makefile b/cli/Makefile index 5eafd4e..9a4a81a 100644 --- a/cli/Makefile +++ b/cli/Makefile @@ -6,11 +6,11 @@ clean: env: bin/env.sh -test: +test: env bin/test.sh -doc: +doc: env bin/doc.sh -packages: +packages: env bin/packages.sh diff --git a/cli/dcoscli/config/main.py b/cli/dcoscli/config/main.py index 31dbcd8..92a7a14 100644 --- a/cli/dcoscli/config/main.py +++ b/cli/dcoscli/config/main.py @@ -1,14 +1,23 @@ """Get and set DCOS command line options Usage: + dcos config append dcos config info + dcos config prepend dcos config set - dcos config unset dcos config show [] + dcos config unset [--index=] + dcos config validate Options: - -h, --help Show this screen - --version Show version + -h, --help Show this screen + --version Show version + --index= Index into the list. The first element in the list has an + index of zero + +Positional Arguments: + The name of the property + The value of the property """ import collections @@ -16,8 +25,10 @@ import os import dcoscli import docopt +import six import toml -from dcos.api import cmds, config, constants, emitting, errors, options, util +from dcos.api import (cmds, config, constants, emitting, errors, jsonitem, + options, subcommand, util) emitter = emitting.FlatEmitter() @@ -58,15 +69,30 @@ def _cmds(): arg_keys=['', ''], function=_set), + cmds.Command( + hierarchy=['config', 'append'], + arg_keys=['', ''], + function=_append), + + cmds.Command( + hierarchy=['config', 'prepend'], + arg_keys=['', ''], + function=_prepend), + cmds.Command( hierarchy=['config', 'unset'], - arg_keys=[''], + arg_keys=['', '--index'], function=_unset), cmds.Command( hierarchy=['config', 'show'], arg_keys=[''], function=_show), + + cmds.Command( + hierarchy=['config', 'validate'], + arg_keys=[], + function=_validate), ] @@ -87,13 +113,64 @@ def _set(name, value): """ config_path, toml_config = _load_config() - toml_config[name] = value + + section, subkey = _split_key(name) + + config_schema, err = _get_config_schema(section) + if err is not None: + emitter.publish(err) + return 1 + + python_value, err = jsonitem.parse_json_value(subkey, value, config_schema) + if err is not None: + emitter.publish(err) + return 1 + + toml_config[name] = python_value _save_config_file(config_path, toml_config) return 0 -def _unset(name): +def _append(name, value): + """ + :returns: process status + :rtype: int + """ + + config_path, toml_config = _load_config() + + python_value, err = _parse_array_item(name, value) + if err is not None: + emitter.publish(err) + return 1 + + toml_config[name] = toml_config.get(name, []) + python_value + _save_config_file(config_path, toml_config) + + return 0 + + +def _prepend(name, value): + """ + :returns: process status + :rtype: int + """ + + config_path, toml_config = _load_config() + + python_value, err = _parse_array_item(name, value) + if err is not None: + emitter.publish(err) + return 1 + + toml_config[name] = python_value + toml_config.get(name, []) + _save_config_file(config_path, toml_config) + + return 0 + + +def _unset(name, index): """ :returns: process status :rtype: int @@ -110,6 +187,28 @@ def _unset(name): elif isinstance(value, collections.Mapping): emitter.publish(_generate_choice_msg(name, value)) return 1 + elif (isinstance(value, collections.Sequence) and + not isinstance(value, six.string_types)): + if index is not None: + index, err = util.parse_int(index) + if err is not None: + emitter.publish(err) + return 1 + + if index < 0 or index >= len(value): + emitter.publish( + errors.DefaultError( + 'Index ({}) is out of bounds - possible values are ' + 'between {} and {}'.format(index, 0, len(value) - 1))) + return 1 + + value.pop(index) + toml_config[name] = value + elif index is not None: + emitter.publish( + errors.DefaultError( + 'Unsetting based on an index is only supported for lists')) + return 1 _save_config_file(config_path, toml_config) @@ -144,6 +243,38 @@ def _show(name): return 0 +def _validate(): + """ + :returns: process status + :rtype: int + """ + + _, toml_config = _load_config() + + root_schema = { + '$schema': 'http://json-schema.org/schema#', + 'type': 'object', + 'properties': {}, + 'additionalProperties': False, + } + + # Load the config schema from all the subsections into the root schema + for section in toml_config.keys(): + config_schema, err = _get_config_schema(section) + if err is not None: + emitter.publish(err) + return 1 + + root_schema['properties'][section] = config_schema + + err = util.validate_json(toml_config._dictionary, root_schema) + if err is not None: + emitter.publish(err) + return 1 + + return 0 + + def _generate_choice_msg(name, value): """ :param name: name of the property @@ -183,3 +314,78 @@ def _save_config_file(config_path, toml_config): serial = toml.dumps(toml_config._dictionary) with open(config_path, 'w') as config_file: config_file.write(serial) + + +def _get_config_schema(command): + """ + :param command: the subcommand name + :type command: str + :returns: the subcommand's configuration schema + :rtype: (dict, dcos.api.errors.Error) + """ + + executable, err = subcommand.command_executables( + command, + util.dcos_path()) + if err is not None: + return (None, err) + + return (subcommand.config_schema(executable), None) + + +def _split_key(name): + """ + :param name: the full property path - e.g. marathon.host + :type name: str + :returns: the section and property name + :rtype: ((str, str), Error) + """ + + terms = name.split('.', 1) + if len(terms) != 2: + emitter.publish( + errors.DefaultError('Property name must have both a section and ' + 'key:
. - E.g. marathon.host')) + return 1 + + return (terms[0], terms[1]) + + +def _parse_array_item(name, value): + """ + :param name: the name of the property + :type name: str + :param value: the value to parse + :type value: str + :returns: the parsed value as an array with one element + :rtype: (list of any, dcos.api.errors.Error) where any is string, int, + float, bool, array or dict + """ + + section, subkey = _split_key(name) + + config_schema, err = _get_config_schema(section) + if err is not None: + return (None, err) + + parser, err = jsonitem.find_parser(subkey, config_schema) + if err is not None: + return (None, err) + + if parser.schema['type'] != 'array': + return ( + None, + errors.DefaultError( + "Append/Prepend not supported on '{0}' properties - use 'dcos " + "config set {0} {1}'".format(name, value)) + ) + + if ('items' in parser.schema and + parser.schema['items']['type'] == 'string'): + + value = '["' + value + '"]' + else: + # We are going to assume that wrapping it in an array is enough + value = '[' + value + ']' + + return parser(value) diff --git a/cli/dcoscli/data/config-schema/marathon.json b/cli/dcoscli/data/config-schema/marathon.json new file mode 100644 index 0000000..bfc966f --- /dev/null +++ b/cli/dcoscli/data/config-schema/marathon.json @@ -0,0 +1,22 @@ +{ + "$schema": "http://json-schema.org/schema#", + "type": "object", + "properties": { + "host": { + "type": "string", + "title": "Marathon hostname or IP address", + "description": "", + "default": "localhost" + }, + "port": { + "type": "integer", + "title": "Marathon port", + "description": "", + "default": 8080, + "minimum": 1, + "maximum": 65535 + } + }, + "additionalProperties": false, + "required": ["host", "port"] +} diff --git a/cli/dcoscli/data/config-schema/package.json b/cli/dcoscli/data/config-schema/package.json new file mode 100644 index 0000000..6e3db55 --- /dev/null +++ b/cli/dcoscli/data/config-schema/package.json @@ -0,0 +1,24 @@ +{ + "$schema": "http://json-schema.org/schema#", + "type": "object", + "properties": { + "sources": { + "type": "array", + "items": { + "type": "string" + }, + "title": "Package sources", + "description": "The list of package source in search order", + "default": [ "git://github.com/mesosphere/universe.git" ], + "additionalItems": false + }, + "cache": { + "type": "string", + "title": "Package cache directory", + "description": "Path to the local package cache directory", + "default": "/tmp/cache" + } + }, + "additionalProperties": false, + "required": ["sources", "cache"] +} diff --git a/cli/dcoscli/data/config-schema/subcommand.json b/cli/dcoscli/data/config-schema/subcommand.json new file mode 100644 index 0000000..d751b9a --- /dev/null +++ b/cli/dcoscli/data/config-schema/subcommand.json @@ -0,0 +1,12 @@ +{ + "$schema": "http://json-schema.org/schema#", + "type": "object", + "properties": { + "pip_find_links": { + "type": "string", + "title": "Location for finding packages", + "description": "If a url or path to an html file, then parse for links to archives. If a local path or file:// url that’s a directory, then look for archives in the directory listing." + } + }, + "additionalProperties": false +} diff --git a/cli/dcoscli/main.py b/cli/dcoscli/main.py index 12dedd9..3eb666d 100644 --- a/cli/dcoscli/main.py +++ b/cli/dcoscli/main.py @@ -32,7 +32,7 @@ import subprocess import dcoscli import docopt -from dcos.api import constants, emitting, errors, subcommand, util +from dcos.api import constants, emitting, subcommand, util emitter = emitting.FlatEmitter() @@ -56,22 +56,12 @@ def main(): command = args[''] - executables = [ - command_path - for command_path in subcommand.list_paths(util.dcos_path()) - if subcommand.noun(command_path) == command - ] + executable, err = subcommand.command_executables(command, util.dcos_path()) + if err is not None: + emitter.publish(err) + return 1 - if len(executables) > 1: - msg = 'Found more than one executable for command {!r}.' - emitter.publish(errors.DefaultError(msg.format(command))) - return 1 - if len(executables) == 0: - msg = "{!r} is not a dcos command. See 'dcos help'." - emitter.publish(errors.DefaultError(msg.format(command))) - return 1 - else: - return subprocess.call(executables + [command] + args['']) + return subprocess.call([executable, command] + args['']) def _config_log_level_environ(log_level): diff --git a/cli/dcoscli/marathon/main.py b/cli/dcoscli/marathon/main.py index 2265ab9..2a629c7 100644 --- a/cli/dcoscli/marathon/main.py +++ b/cli/dcoscli/marathon/main.py @@ -1,5 +1,7 @@ -""" +"""Deploy and manage applications on the DCOS + Usage: + dcos marathon --config-schema dcos marathon app add [] dcos marathon app list dcos marathon app remove [--force] @@ -22,7 +24,7 @@ Options: -h, --help Show this screen --version Show version --force This flag disable checks in Marathon during - update operations. + update operations --app-version= This flag specifies the application version to use for the command. The application version () can be specified as an @@ -31,22 +33,24 @@ Options: Relative values must be specified as a negative integer and they represent the version from the currently deployed - application definition. + application definition + --config-schema Show the configuration schema for the Marathon + subcommand --max-count= Maximum number of entries to try to fetch and return --interval= Number of seconds to wait between actions Positional arguments: - The application id - The application resource; for a detailed - description see (https://mesosphere.github.io/ - marathon/docs/rest-api.html#post-/v2/apps) - The deployment id - The number of instances to start - Optional key-value pairs to be included in the - command. The separator between the key and value - must be the '=' character. E.g. cpus=2.0 - The task id + The application id + The application resource; for a detailed + description see (https://mesosphere.github.io/ + marathon/docs/rest-api.html#post-/v2/apps) + The deployment id + The number of instances to start + Optional key-value pairs to be included in the + command. The separator between the key and + value must be the '=' character. E.g. cpus=2.0 + The task id """ import json import os @@ -73,10 +77,6 @@ def main(): __doc__, version='dcos-marathon version {}'.format(dcoscli.version)) - if not args['marathon']: - emitter.publish(options.make_generic_usage_message(__doc__)) - return 1 - returncode, err = cmds.execute(_cmds(), args) if err is not None: emitter.publish(err) @@ -94,88 +94,118 @@ def _cmds(): return [ cmds.Command( - hierarchy=['version', 'list'], + hierarchy=['marathon', 'version', 'list'], arg_keys=['', '--max-count'], function=_version_list), cmds.Command( - hierarchy=['deployment', 'list'], + hierarchy=['marathon', 'deployment', 'list'], arg_keys=[''], function=_deployment_list), cmds.Command( - hierarchy=['deployment', 'rollback'], + hierarchy=['marathon', 'deployment', 'rollback'], arg_keys=[''], function=_deployment_rollback), cmds.Command( - hierarchy=['deployment', 'stop'], + hierarchy=['marathon', 'deployment', 'stop'], arg_keys=[''], function=_deployment_stop), cmds.Command( - hierarchy=['deployment', 'watch'], + hierarchy=['marathon', 'deployment', 'watch'], arg_keys=['', '--max-count', '--interval'], function=_deployment_watch), cmds.Command( - hierarchy=['task', 'list'], + hierarchy=['marathon', 'task', 'list'], arg_keys=[''], function=_task_list), cmds.Command( - hierarchy=['task', 'show'], + hierarchy=['marathon', 'task', 'show'], arg_keys=[''], function=_task_show), - cmds.Command(hierarchy=['info'], arg_keys=[], function=_info), + cmds.Command( + hierarchy=['marathon', 'info'], + arg_keys=[], + function=_info), cmds.Command( - hierarchy=['app', 'add'], + hierarchy=['marathon', 'app', 'add'], arg_keys=[''], function=_add), - cmds.Command(hierarchy=['list'], arg_keys=[], function=_list), + cmds.Command( + hierarchy=['marathon', 'app', 'list'], + arg_keys=[], + function=_list), cmds.Command( - hierarchy=['app', 'remove'], + hierarchy=['marathon', 'app', 'remove'], arg_keys=['', '--force'], function=_remove), cmds.Command( - hierarchy=['app', 'show'], + hierarchy=['marathon', 'app', 'show'], arg_keys=['', '--app-version'], function=_show), cmds.Command( - hierarchy=['app', 'start'], + hierarchy=['marathon', 'app', 'start'], arg_keys=['', '', '--force'], function=_start), cmds.Command( - hierarchy=['app', 'stop'], + hierarchy=['marathon', 'app', 'stop'], arg_keys=['', '--force'], function=_stop), cmds.Command( - hierarchy=['app', 'update'], + hierarchy=['marathon', 'app', 'update'], arg_keys=['', '', '--force'], function=_update), cmds.Command( - hierarchy=['app', 'restart'], + hierarchy=['marathon', 'app', 'restart'], arg_keys=['', '--force'], function=_restart), + + cmds.Command( + hierarchy=['marathon'], + arg_keys=['--config-schema'], + function=_marathon), ] +def _marathon(config_schema): + """ + :returns: Process status + :rtype: int + """ + + if config_schema: + schema = json.loads( + pkg_resources.resource_string( + 'dcoscli', + 'data/config-schema/marathon.json').decode('utf-8')) + emitter.publish(schema) + else: + emitter.publish(options.make_generic_usage_message(__doc__)) + return 1 + + return 0 + + def _info(): """ :returns: Process status :rtype: int """ - emitter.publish('Deploy and manage applications on the DCOS') + emitter.publish(__doc__.split('\n')[0]) return 0 @@ -354,7 +384,7 @@ def _start(app_id, instances, force): if instances is None: instances = 1 else: - instances, err = _parse_int(instances) + instances, err = util.parse_int(instances) if err is not None: emitter.publish(err) return 1 @@ -541,7 +571,7 @@ def _version_list(app_id, max_count): """ if max_count is not None: - max_count, err = _parse_int(max_count) + max_count, err = util.parse_int(max_count) if err is not None: emitter.publish(err) return 1 @@ -637,13 +667,13 @@ def _deployment_watch(deployment_id, max_count, interval): """ if max_count is not None: - max_count, err = _parse_int(max_count) + max_count, err = util.parse_int(max_count) if err is not None: emitter.publish(err) return 1 if interval is not None: - interval, err = _parse_int(interval) + interval, err = util.parse_int(interval) if err is not None: emitter.publish(err) return 1 @@ -763,7 +793,7 @@ def _calculate_version(client, app_id, version): """ # First let's try to parse it as a negative integer - value, err = _parse_int(version) + value, err = util.parse_int(version) if err is None and value < 0: value = -1 * value # We have a negative value let's ask Marathon for the last abs(value) @@ -789,23 +819,3 @@ def _calculate_version(client, app_id, version): else: # Let's assume that we have an absolute version return (version, None) - - -def _parse_int(string): - """ - :param string: String to parse as an integer - :type string: str - :returns: The interger value of the string - :rtype: (int, Error) - """ - - try: - return (int(string), None) - except: - error = sys.exc_info()[0] - logger = util.get_logger(__name__) - logger.error( - 'Unhandled exception while parsing string as int: %r -- %r', - string, - error) - return (None, errors.DefaultError('Error parsing string as int')) diff --git a/cli/dcoscli/package/main.py b/cli/dcoscli/package/main.py index 8ca038f..c3421e7 100644 --- a/cli/dcoscli/package/main.py +++ b/cli/dcoscli/package/main.py @@ -1,5 +1,7 @@ -""" +"""Install and manage DCOS software packages + Usage: + dcos package --config-schema dcos package describe dcos package info dcos package install [--options= --app-id=] @@ -37,6 +39,7 @@ import os import dcoscli import docopt +import pkg_resources import toml from dcos.api import (cmds, config, constants, emitting, marathon, options, package, util) @@ -54,10 +57,6 @@ def main(): __doc__, version='dcos-package version {}'.format(dcoscli.version)) - if not args['package']: - emitter.publish(options.make_generic_usage_message(__doc__)) - return 1 - returncode, err = cmds.execute(_cmds(), args) if err is not None: emitter.publish(err) @@ -75,47 +74,71 @@ def _cmds(): return [ cmds.Command( - hierarchy=['info'], + hierarchy=['package', 'info'], arg_keys=[], function=_info), cmds.Command( - hierarchy=['sources'], + hierarchy=['package', 'sources'], arg_keys=[], function=_list_sources), cmds.Command( - hierarchy=['update'], + hierarchy=['package', 'update'], arg_keys=[], function=_update), cmds.Command( - hierarchy=['describe'], + hierarchy=['package', 'describe'], arg_keys=[''], function=_describe), cmds.Command( - hierarchy=['install'], + hierarchy=['package', 'install'], arg_keys=['', '--options', '--app-id'], function=_install), cmds.Command( - hierarchy=['list'], + hierarchy=['package', 'list'], arg_keys=[], function=_list), cmds.Command( - hierarchy=['search'], + hierarchy=['package', 'search'], arg_keys=[''], function=_search), cmds.Command( - hierarchy=['uninstall'], + hierarchy=['package', 'uninstall'], arg_keys=['', '--all', '--app-id'], function=_uninstall), + + cmds.Command( + hierarchy=['package'], + arg_keys=['--config-schema'], + function=_package), ] +def _package(config_schema): + """ + :returns: Process status + :rtype: int + """ + + if config_schema: + schema = json.loads( + pkg_resources.resource_string( + 'dcoscli', + 'data/config-schema/package.json').decode('utf-8')) + emitter.publish(schema) + else: + emitter.publish(options.make_generic_usage_message(__doc__)) + return 1 + + return 0 + + def _load_config(): """ :returns: Configuration object @@ -132,7 +155,7 @@ def _info(): :rtype: int """ - emitter.publish('Install and manage DCOS software packages') + emitter.publish(__doc__.split('\n')[0]) return 0 diff --git a/cli/dcoscli/subcommand/main.py b/cli/dcoscli/subcommand/main.py index 45ca939..1580958 100644 --- a/cli/dcoscli/subcommand/main.py +++ b/cli/dcoscli/subcommand/main.py @@ -1,6 +1,7 @@ """Install and manage DCOS CLI Subcommands Usage: + dcos subcommand --config-schema dcos subcommand info dcos subcommand install dcos subcommand list @@ -14,12 +15,14 @@ Positional arguments: The subcommand package wheel The name of the subcommand package """ +import json import os import shutil import subprocess import dcoscli import docopt +import pkg_resources import pkginfo from dcos.api import (cmds, config, constants, emitting, errors, options, subcommand, util) @@ -73,9 +76,33 @@ def _cmds(): hierarchy=['subcommand', 'info'], arg_keys=[], function=_info), + + cmds.Command( + hierarchy=['subcommand'], + arg_keys=['--config-schema'], + function=_subcommand), ] +def _subcommand(config_schema): + """ + :returns: Process status + :rtype: int + """ + + if config_schema: + schema = json.loads( + pkg_resources.resource_string( + 'dcoscli', + 'data/config-schema/subcommand.json').decode('utf-8')) + emitter.publish(schema) + else: + emitter.publish(options.make_generic_usage_message(__doc__)) + return 1 + + return 0 + + def _info(): """ :returns: the process return code diff --git a/cli/setup.py b/cli/setup.py index ccab4d4..ac77710 100644 --- a/cli/setup.py +++ b/cli/setup.py @@ -73,7 +73,10 @@ setup( # installed, specify them here. If using Python 2.6 or less, then these # have to be included in MANIFEST.in as well. package_data={ - 'dcoscli': ['data/*'], + 'dcoscli': [ + 'data/*.json', + 'data/config-schema/*.json', + ], }, # To provide executable scripts, use entry points in preference to the diff --git a/cli/tests/data/dcos.toml b/cli/tests/data/dcos.toml index 2af0b3b..8fe33fc 100644 --- a/cli/tests/data/dcos.toml +++ b/cli/tests/data/dcos.toml @@ -1,10 +1,8 @@ [subcommand] pip_find_links = "../dist" -[foo] -bar = true -[marathon] -host = "localhost" -port = 8080 [package] sources = [ "git://github.com/mesosphere/universe.git", "https://github.com/mesosphere/universe/archive/master.zip",] cache = "tmp/cache" +[marathon] +host = "localhost" +port = 8080 diff --git a/cli/tests/integrations/cli/test_config.py b/cli/tests/integrations/cli/test_config.py index 44d8e80..3155c52 100644 --- a/cli/tests/integrations/cli/test_config.py +++ b/cli/tests/integrations/cli/test_config.py @@ -1,5 +1,7 @@ +import json import os +import six from dcos.api import constants import pytest @@ -21,14 +23,23 @@ def test_help(): assert stdout == b"""Get and set DCOS command line options Usage: + dcos config append dcos config info + dcos config prepend dcos config set - dcos config unset dcos config show [] + dcos config unset [--index=] + dcos config validate Options: - -h, --help Show this screen - --version Show version + -h, --help Show this screen + --version Show version + --index= Index into the list. The first element in the list has an + index of zero + +Positional Arguments: + The name of the property + The value of the property """ assert stderr == b'' @@ -55,8 +66,7 @@ def test_list_property(env): env) assert returncode == 0 - assert stdout == b"""foo.bar=True -marathon.host=localhost + assert stdout == b"""marathon.host=localhost marathon.port=8080 package.cache=tmp/cache package.sources=['git://github.com/mesosphere/universe.git', \ @@ -74,10 +84,6 @@ def test_get_existing_integral_property(env): _get_value('marathon.port', 8080, env) -def test_get_existing_boolean_property(env): - _get_value('foo.bar', True, env) - - def test_get_missing_property(env): _get_missing_value('missing.property', env) @@ -97,14 +103,114 @@ def test_get_top_property(env): ) -def test_set_existing_property(env): +def test_set_existing_string_property(env): _set_value('marathon.host', 'newhost', env) _get_value('marathon.host', 'newhost', env) _set_value('marathon.host', 'localhost', env) +def test_set_existing_integral_property(env): + _set_value('marathon.port', '8181', env) + _get_value('marathon.port', 8181, env) + _set_value('marathon.port', '8080', env) + + +def test_append_empty_list(env): + _unset_value('package.sources', None, env) + _append_value( + 'package.sources', + 'git://github.com/mesosphere/universe.git', + env) + _get_value( + 'package.sources', + ['git://github.com/mesosphere/universe.git'], + env) + _append_value( + 'package.sources', + 'https://github.com/mesosphere/universe/archive/master.zip', + env) + _get_value( + 'package.sources', + ['git://github.com/mesosphere/universe.git', + 'https://github.com/mesosphere/universe/archive/master.zip'], + env) + + +def test_prepend_empty_list(env): + _unset_value('package.sources', None, env) + _prepend_value( + 'package.sources', + 'https://github.com/mesosphere/universe/archive/master.zip', + env) + _get_value( + 'package.sources', + ['https://github.com/mesosphere/universe/archive/master.zip'], + env) + _prepend_value( + 'package.sources', + 'git://github.com/mesosphere/universe.git', + env) + _get_value( + 'package.sources', + ['git://github.com/mesosphere/universe.git', + 'https://github.com/mesosphere/universe/archive/master.zip'], + env) + + +def test_append_list(env): + _append_value( + 'package.sources', + 'new_uri', + env) + _get_value( + 'package.sources', + ['git://github.com/mesosphere/universe.git', + 'https://github.com/mesosphere/universe/archive/master.zip', + 'new_uri'], + env) + _unset_value('package.sources', '2', env) + + +def test_prepend_list(env): + _prepend_value( + 'package.sources', + 'new_uri', + env) + _get_value( + 'package.sources', + ['new_uri', + 'git://github.com/mesosphere/universe.git', + 'https://github.com/mesosphere/universe/archive/master.zip'], + env) + _unset_value('package.sources', '0', env) + + +def test_append_non_list(env): + returncode, stdout, stderr = exec_command( + ['dcos', 'config', 'append', 'marathon.host', 'new_uri'], + env) + + assert returncode == 1 + assert stdout == b'' + assert (stderr == + b"Append/Prepend not supported on 'marathon.host' " + b"properties - use 'dcos config set marathon.host new_uri'\n") + + +def test_prepend_non_list(env): + returncode, stdout, stderr = exec_command( + ['dcos', 'config', 'prepend', 'marathon.host', 'new_uri'], + env) + + assert returncode == 1 + assert stdout == b'' + assert (stderr == + b"Append/Prepend not supported on 'marathon.host' " + b"properties - use 'dcos config set marathon.host new_uri'\n") + + def test_unset_property(env): - _unset_value('marathon.host', env) + _unset_value('marathon.host', None, env) _get_missing_value('marathon.host', env) _set_value('marathon.host', 'localhost', env) @@ -134,10 +240,102 @@ def test_unset_top_property(env): ) +def test_unset_whole_list(env): + _unset_value('package.sources', None, env) + _get_missing_value('package.sources', env) + _set_value( + 'package.sources', + '["git://github.com/mesosphere/universe.git", ' + '"https://github.com/mesosphere/universe/archive/master.zip"]', + env) + + +def test_unset_list_index(env): + _unset_value('package.sources', '0', env) + _get_value( + 'package.sources', + ['https://github.com/mesosphere/universe/archive/master.zip'], + env) + _prepend_value( + 'package.sources', + 'git://github.com/mesosphere/universe.git', + env) + + +def test_unset_outbound_index(env): + returncode, stdout, stderr = exec_command( + ['dcos', 'config', 'unset', '--index=3', 'package.sources'], + env) + + assert returncode == 1 + assert stdout == b'' + assert (stderr == + b'Index (3) is out of bounds - possible values are ' + b'between 0 and 1\n') + + +def test_unset_bad_index(env): + returncode, stdout, stderr = exec_command( + ['dcos', 'config', 'unset', '--index=number', 'package.sources'], + env) + + assert returncode == 1 + assert stdout == b'' + assert stderr == b'Error parsing string as int\n' + + +def test_unset_index_from_string(env): + returncode, stdout, stderr = exec_command( + ['dcos', 'config', 'unset', '--index=0', 'marathon.host'], + env) + + assert returncode == 1 + assert stdout == b'' + assert (stderr == + b'Unsetting based on an index is only supported for lists\n') + + +def test_validate(env): + returncode, stdout, stderr = exec_command( + ['dcos', 'config', 'validate'], + env) + + assert returncode == 0 + assert stdout == b'' + assert stderr == b'' + + +def test_validation_error(env): + _unset_value('marathon.host', None, env) + + returncode, stdout, stderr = exec_command( + ['dcos', 'config', 'validate'], + env) + + assert returncode == 1 + assert stdout == b'' + assert stderr == b"""Error: 'host' is a required property +Path: marathon +Value: {"port": 8080} +""" + + _set_value('marathon.host', 'localhost', env) + + +def test_set_property_key(env): + returncode, stdout, stderr = exec_command( + ['dcos', 'config', 'set', 'path.to.value', 'cool new value'], + env) + + assert returncode == 1 + assert stdout == b'' + assert stderr == b"'path' is not a dcos command.\n" + + def test_set_missing_property(env): - _set_value('path.to.value', 'cool new value', env) - _get_value('path.to.value', 'cool new value', env) - _unset_value('path.to.value', env) + _unset_value('marathon.host', None, env) + _set_value('marathon.host', 'localhost', env) + _get_value('marathon.host', 'localhost', env) def _set_value(key, value, env): @@ -150,20 +348,47 @@ def _set_value(key, value, env): assert stderr == b'' +def _append_value(key, value, env): + returncode, stdout, stderr = exec_command( + ['dcos', 'config', 'append', key, value], + env) + + assert returncode == 0 + assert stdout == b'' + assert stderr == b'' + + +def _prepend_value(key, value, env): + returncode, stdout, stderr = exec_command( + ['dcos', 'config', 'prepend', key, value], + env) + + assert returncode == 0 + assert stdout == b'' + assert stderr == b'' + + def _get_value(key, value, env): returncode, stdout, stderr = exec_command( ['dcos', 'config', 'show', key], env) + if isinstance(value, six.string_types): + result = json.loads('"' + stdout.decode('utf-8').strip() + '"') + else: + result = json.loads(stdout.decode('utf-8').strip()) + assert returncode == 0 - assert stdout == '{}\n'.format(value).encode('utf-8') + assert result == value assert stderr == b'' -def _unset_value(key, env): - returncode, stdout, stderr = exec_command( - ['dcos', 'config', 'unset', key], - env) +def _unset_value(key, index, env): + cmd = ['dcos', 'config', 'unset', key] + if index is not None: + cmd.append('--index={}'.format(index)) + + returncode, stdout, stderr = exec_command(cmd, env) assert returncode == 0 assert stdout == b'' diff --git a/cli/tests/integrations/cli/test_marathon.py b/cli/tests/integrations/cli/test_marathon.py index f934a40..7c585aa 100644 --- a/cli/tests/integrations/cli/test_marathon.py +++ b/cli/tests/integrations/cli/test_marathon.py @@ -7,7 +7,10 @@ def test_help(): returncode, stdout, stderr = exec_command(['dcos', 'marathon', '--help']) assert returncode == 0 - assert stdout == b"""Usage: + assert stdout == b"""Deploy and manage applications on the DCOS + +Usage: + dcos marathon --config-schema dcos marathon app add [] dcos marathon app list dcos marathon app remove [--force] @@ -30,7 +33,7 @@ Options: -h, --help Show this screen --version Show version --force This flag disable checks in Marathon during - update operations. + update operations --app-version= This flag specifies the application version to use for the command. The application version () can be specified as an @@ -39,22 +42,24 @@ Options: Relative values must be specified as a negative integer and they represent the version from the currently deployed - application definition. + application definition + --config-schema Show the configuration schema for the Marathon + subcommand --max-count= Maximum number of entries to try to fetch and return --interval= Number of seconds to wait between actions Positional arguments: - The application id - The application resource; for a detailed - description see (https://mesosphere.github.io/ - marathon/docs/rest-api.html#post-/v2/apps) - The deployment id - The number of instances to start - Optional key-value pairs to be included in the - command. The separator between the key and value - must be the '=' character. E.g. cpus=2.0 - The task id + The application id + The application resource; for a detailed + description see (https://mesosphere.github.io/ + marathon/docs/rest-api.html#post-/v2/apps) + The deployment id + The number of instances to start + Optional key-value pairs to be included in the + command. The separator between the key and + value must be the '=' character. E.g. cpus=2.0 + The task id """ assert stderr == b'' diff --git a/cli/tests/integrations/cli/test_package.py b/cli/tests/integrations/cli/test_package.py index d0e24b0..179df97 100644 --- a/cli/tests/integrations/cli/test_package.py +++ b/cli/tests/integrations/cli/test_package.py @@ -5,7 +5,10 @@ def test_package(): returncode, stdout, stderr = exec_command(['dcos', 'package', '--help']) assert returncode == 0 - assert stdout == b"""Usage: + assert stdout == b"""Install and manage DCOS software packages + +Usage: + dcos package --config-schema dcos package describe dcos package info dcos package install [--options= --app-id=] @@ -112,7 +115,7 @@ Error: 'mesos-dns/config-url' is a required property Value: {"mesos-dns/host": false} Error: False is not of type 'string' -Path: mesos-dns/host +Path: mesos-dns/host Value: false """ diff --git a/dcos/api/emitting.py b/dcos/api/emitting.py index 48d4eda..4205700 100644 --- a/dcos/api/emitting.py +++ b/dcos/api/emitting.py @@ -9,6 +9,7 @@ import sys import pager import pygments +import six from dcos.api import constants, errors, util from pygments.formatters import Terminal256Formatter from pygments.lexers import JsonLexer @@ -74,16 +75,19 @@ def print_handler(event): # Do nothing pass - elif isinstance(event, basestring): + elif isinstance(event, six.string_types): _page(event, pager_command) - elif isinstance(event, collections.Mapping) or isinstance(event, list): - processed_json = _process_json(event, pager_command) - _page(processed_json, pager_command) - elif isinstance(event, errors.Error): print(event.error(), file=sys.stderr) + elif (isinstance(event, collections.Mapping) or + isinstance(event, collections.Sequence) or isinstance(event, bool) or + isinstance(event, six.integer_types) or isinstance(event, float)): + # These are all valid JSON types let's treat them different + processed_json = _process_json(event, pager_command) + _page(processed_json, pager_command) + else: logger.debug('Printing unknown type: %s, %r.', type(event), event) _page(event, pager_command) diff --git a/dcos/api/jsonitem.py b/dcos/api/jsonitem.py index 3558d13..1aec07d 100644 --- a/dcos/api/jsonitem.py +++ b/dcos/api/jsonitem.py @@ -26,25 +26,46 @@ def parse_json_item(json_item, schema): # Check that it is a valid key in our jsonschema key = terms[0] - value_type, err = _check_key_with_schema(key, schema) - if err is not None: - return (None, err) - - value, err = value_type(terms[1]) + value, err = parse_json_value(key, terms[1], schema) if err is not None: return (None, err) return ((key, value), None) -def _check_key_with_schema(key, schema): +def parse_json_value(key, value, schema): + """Parse the json value based on a schema. + + :param key: the key property + :type key: str + :param value: the value of property + :type value: str + :param schema: The JSON schema to use for parsing + :type schema: dict + :returns: parsed value + :rtype: (any, Error) where any is one of str, int, float, bool, + list or dict + """ + + value_type, err = find_parser(key, schema) + if err is not None: + return (None, err) + + python_value, err = value_type(value) + if err is not None: + return (None, err) + + return (python_value, None) + + +def find_parser(key, schema): """ :param key: JSON field :type key: str :param schema: The JSON schema to use :type schema: dict :returns: A callable capable of parsing a string to its type - :rtype: (_ValueTypeParser, Error) + :rtype: (ValueTypeParser, Error) """ key_schema = schema['properties'].get(key) @@ -57,18 +78,18 @@ def _check_key_with_schema(key, schema): 'Possible values are: {}'.format(key, keys)) ) else: - return (_ValueTypeParser(key_schema['type']), None) + return (ValueTypeParser(key_schema), None) -class _ValueTypeParser(object): +class ValueTypeParser(object): """Callable for parsing a string against a known JSON type. - :param value_type: The JSON type as a string - :type value_type: str + :param schema: The JSON type as a schema + :type schema: dict """ - def __init__(self, value_type): - self._value_type = value_type + def __init__(self, schema): + self.schema = schema def __call__(self, value): """ @@ -81,17 +102,17 @@ class _ValueTypeParser(object): value = _clean_value(value) - if self._value_type == 'string': + if self.schema['type'] == 'string': return _parse_string(value) - elif self._value_type == 'object': + elif self.schema['type'] == 'object': return _parse_object(value) - elif self._value_type == 'number': + elif self.schema['type'] == 'number': return _parse_number(value) - elif self._value_type == 'integer': + elif self.schema['type'] == 'integer': return _parse_integer(value) - elif self._value_type == 'boolean': + elif self.schema['type'] == 'boolean': return _parse_boolean(value) - elif self._value_type == 'array': + elif self.schema['type'] == 'array': return _parse_array(value) else: return ( diff --git a/dcos/api/subcommand.py b/dcos/api/subcommand.py index 3262372..44b89fd 100644 --- a/dcos/api/subcommand.py +++ b/dcos/api/subcommand.py @@ -1,7 +1,36 @@ +import json import os import subprocess -from dcos.api import constants +from dcos.api import constants, errors + + +def command_executables(subcommand, dcos_path): + """List the real path to executable dcos program for specified subcommand. + + :param subcommand: name of subcommand. E.g. marathon + :type subcommand: str + :param dcos_path: path to the dcos cli directory + :type dcos_path: str + :returns: the dcos program path + :rtype: (str, dcos.api.errors.Error) + """ + + executables = [ + command_path + for command_path in list_paths(dcos_path) + if noun(command_path) == subcommand + ] + + if len(executables) > 1: + msg = 'Found more than one executable for command {!r}.' + return (None, errors.DefaultError(msg.format(subcommand))) + + if len(executables) == 0: + msg = "{!r} is not a dcos command." + return (None, errors.DefaultError(msg.format(subcommand))) + + return (executables[0], None) def list_paths(dcos_path): @@ -94,6 +123,21 @@ def info(executable_path): return out.decode('utf-8').strip() +def config_schema(executable_path): + """Collects subcommand config schema + + :param executable_path: real path to the dcos subcommand + :type executable_path: str + :returns: the subcommand config schema + :rtype: dict + """ + + out = subprocess.check_output( + [executable_path, noun(executable_path), '--config-schema']) + + return json.loads(out.decode('utf-8')) + + def noun(executable_path): """Extracts the subcommand single noun from the path to the executable. E.g for :code:`bin/dcos-subcommand` this method returns :code:`subcommand`. diff --git a/dcos/api/util.py b/dcos/api/util.py index ff046e0..517ca9a 100644 --- a/dcos/api/util.py +++ b/dcos/api/util.py @@ -199,7 +199,7 @@ def validate_json(instance, schema): def format(error): message = 'Error: {}\n'.format(hack_error_message_fix(error.message)) if len(error.absolute_path) > 0: - message += 'Path: {}\n'.format(error.absolute_path[0]) + message += 'Path: {}\n'.format('.'.join(error.absolute_path)) message += 'Value: {}'.format(json.dumps(error.instance)) return message @@ -210,3 +210,24 @@ def validate_json(instance, schema): else: errors_as_str = str.join('\n\n', formatted_errors) return errors.DefaultError(errors_as_str) + + +def parse_int(string): + """Parse string and an integer + + :param string: string to parse as an integer + :type string: str + :returns: the interger value of the string + :rtype: (int, Error) + """ + + try: + return (int(string), None) + except: + error = sys.exc_info()[0] + logger = get_logger(__name__) + logger.error( + 'Unhandled exception while parsing string as int: %r -- %r', + string, + error) + return (None, errors.DefaultError('Error parsing string as int')) diff --git a/setup.py b/setup.py index a3fab5e..65e7012 100644 --- a/setup.py +++ b/setup.py @@ -70,8 +70,8 @@ setup( install_requires=[ 'gitpython', 'jsonschema', - 'portalocker', 'pager', + 'portalocker', 'pygments', 'pystache', 'requests', diff --git a/tests/test_jsonitem.py b/tests/test_jsonitem.py index 6153d7b..43655d6 100644 --- a/tests/test_jsonitem.py +++ b/tests/test_jsonitem.py @@ -197,9 +197,9 @@ def test_parse_invalid_arrays(bad_array): assert isinstance(error, errors.Error) -def test_check_key_with_schema(schema, jsonitem_tuple): +def test_find_parser(schema, jsonitem_tuple): key, string_value, value = jsonitem_tuple - value_type, err = jsonitem._check_key_with_schema(key, schema) + value_type, err = jsonitem.find_parser(key, schema) assert err is None assert value_type(string_value) == (value, None)