diff --git a/cli/dcoscli/package/main.py b/cli/dcoscli/package/main.py index 53a2494..e8da8f7 100644 --- a/cli/dcoscli/package/main.py +++ b/cli/dcoscli/package/main.py @@ -52,6 +52,8 @@ import pkg_resources from dcos.api import (cmds, config, constants, emitting, errors, marathon, options, package, subcommand, util) +logger = util.get_logger(__name__) + emitter = emitting.FlatEmitter() @@ -312,6 +314,7 @@ def _install(package_name, options_path, app_id, cli, app): emitter.publish(err) return 1 except Exception as e: + logger.exception('Exception while generating options') emitter.publish(errors.DefaultError(e.message)) return 1 diff --git a/dcos/api/package.py b/dcos/api/package.py index f65f158..9b65843 100644 --- a/dcos/api/package.py +++ b/dcos/api/package.py @@ -1,6 +1,7 @@ import abc import base64 import collections +import copy import hashlib import json import os @@ -495,20 +496,49 @@ def _search_rank(pkg, query): def _extract_default_values(config_schema): """ :param config_schema: A json-schema describing configuration options. - :type config_schema: (dict, Error) + :type config_schema: dict + :returns: a dictionary with the default specified by the schema + :rtype: dict """ - properties = config_schema.get('properties') - schema_type = config_schema.get('type') + defaults = {} + for key, value in config_schema['properties'].items(): + if 'default' in value: + defaults[key] = value['default'] + elif value.get('type', '') == 'object': + # Generate the default value from the embedded schema + defaults[key] = _extract_default_values(value) - if schema_type != 'object' or properties is None: - return ({}, None) + return defaults - defaults = [(p, properties[p]['default']) - for p in properties - if 'default' in properties[p]] - return (dict(defaults), None) +def _merge_options(first, second): + """Merges the :code:`second` dictionary into the :code:`first` dictionary. + If both dictionaries have the same key and both values are dictionaries + then it recursively merges those two dictionaries. + + :param first: first dictionary + :type first: dict + :param second: second dictionary + :type second: dict + :returns: merged dictionary + :rtype: dict + """ + + result = copy.deepcopy(first) + for key, second_value in second.items(): + if key in first: + first_value = first[key] + + if (isinstance(first_value, collections.Mapping) and + isinstance(second_value, collections.Mapping)): + result[key] = _merge_options(first_value, second_value) + else: + result[key] = second_value + else: + result[key] = second_value + + return result def resolve_package(package_name, config): @@ -1093,13 +1123,12 @@ class Package(): if err is not None: return (None, err) - default_options, err = _extract_default_values(config_schema) - if err is not None: - return (None, err) + default_options = _extract_default_values(config_schema) + logger.info('Generated default options: %r', default_options) # Merge option overrides - options = dict(list(default_options.items()) + - list(user_options.items())) + options = _merge_options(default_options, user_options) + logger.info('Merged options: %r', options) # Validate options with the config schema err = util.validate_json(options, config_schema) diff --git a/tests/test_package.py b/tests/test_package.py index 7fae986..a642d83 100644 --- a/tests/test_package.py +++ b/tests/test_package.py @@ -1,33 +1,79 @@ -import json +import collections from dcos.api import package +import pytest + +MergeData = collections.namedtuple( + 'MergeData', + ['first', 'second', 'expected']) + + +@pytest.fixture(params=[ + MergeData( + first={}, + second={'a': 1}, + expected={'a': 1}), + MergeData( + first={'a': 'a'}, + second={'a': 1}, + expected={'a': 1}), + MergeData( + first={'b': 'b'}, + second={'a': 1}, + expected={'b': 'b', 'a': 1}), + MergeData( + first={'b': 'b'}, + second={}, + expected={'b': 'b'}), + MergeData( + first={'b': {'a': 'a'}}, + second={'b': {'c': 'c'}}, + expected={'b': {'c': 'c', 'a': 'a'}}), + ]) +def merge_data(request): + return request.param + def test_extract_default_values(): - - config_schema = json.loads(""" - { + config_schema = { "type": "object", "properties": { - "foo.bar": { - "type": "string", - "description": "A bar name." + "foo": { + "type": "object", + "properties": { + "bar": { + "type": "string", + "description": "A bar name." + }, + "baz": { + "type": "integer", + "description": "How many times to do baz.", + "minimum": 0, + "maximum": 16, + "required": False, + "default": 4 + } + } }, - "foo.baz": { - "type": "integer", - "description": "How many times to do baz.", - "minimum": 0, - "maximum": 16, - "required": false, - "default": 4 + "fiz": { + "type": "boolean", + "default": True, + }, + "buz": { + "type": "string" } } } - """) - expected = {'foo.baz': 4} + expected = {'foo': {'baz': 4}, 'fiz': True} - result, error = package._extract_default_values(config_schema) + result = package._extract_default_values(config_schema) - assert error is None assert result == expected + + +def test_option_merge(merge_data): + assert merge_data.expected == package._merge_options( + merge_data.first, + merge_data.second)