Skip to content
This repository has been archived by the owner on Jan 19, 2018. It is now read-only.

Refactor Nulecule config. #720

Merged
merged 3 commits into from
Sep 1, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions atomicapp/cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ def cli_fetch(args):
nm = NuleculeManager(app_spec=argdict['app_spec'],
destination=destination,
cli_answers=argdict['cli_answers'],
answers_file=argdict['answers'])
answers_file=argdict['answers'],
answers_format=argdict.get('answers_format'))
nm.fetch(**argdict)
# Clean up the files if the user asked us to. Otherwise
# notify the user where they can manage the application
Expand All @@ -81,7 +82,8 @@ def cli_run(args):
nm = NuleculeManager(app_spec=argdict['app_spec'],
destination=destination,
cli_answers=argdict['cli_answers'],
answers_file=argdict['answers'])
answers_file=argdict['answers'],
answers_format=argdict.get('answers_format'))
nm.run(**argdict)
# Clean up the files if the user asked us to. Otherwise
# notify the user where they can manage the application
Expand Down Expand Up @@ -306,7 +308,7 @@ def create_parser(self):
help="A file which will contain anwsers provided in interactive mode")
run_subparser.add_argument(
"--provider",
dest="cli_provider",
dest="provider",
choices=PROVIDERS,
help="The provider to use. Overrides provider value in answerfile.")
run_subparser.add_argument(
Expand Down Expand Up @@ -511,7 +513,8 @@ def run(self):
# and make a dictionary of it to pass along in args.
setattr(args, 'cli_answers', {})
for item in ['provider-api', 'provider-cafile', 'provider-auth',
'provider-config', 'provider-tlsverify', 'namespace']:
'provider-config', 'provider-tlsverify', 'namespace',
'provider']:
if hasattr(args, item) and getattr(args, item) is not None:
args.cli_answers[item] = getattr(args, item)

Expand Down
1 change: 1 addition & 0 deletions atomicapp/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
DEFAULTNAME_KEY = "default"
PROVIDER_KEY = "provider"
NAMESPACE_KEY = "namespace"
NAMESPACE_SEPARATOR = ":"
REQUIREMENTS_KEY = "requirements"

# Nulecule spec terminology vs the function within /providers
Expand Down
60 changes: 39 additions & 21 deletions atomicapp/nulecule/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
along with Atomic App. If not, see <http://www.gnu.org/licenses/>.
"""
import anymarkup
import copy
import logging
import os
import yaml
Expand All @@ -38,7 +37,7 @@
NAME_KEY,
INHERIT_KEY,
ARTIFACTS_KEY,
DEFAULT_PROVIDER)
NAMESPACE_SEPARATOR)
from atomicapp.utils import Utils
from atomicapp.requirements import Requirements
from atomicapp.nulecule.lib import NuleculeBase
Expand Down Expand Up @@ -76,7 +75,7 @@ def __init__(self, id, specversion, graph, basepath, metadata=None,
metadata (dict): Nulecule metadata
requirements (dict): Requirements for the Nulecule application
params (list): List of params for the Nulecule application
config (dict): Config data for the Nulecule application
config (atomicapp.nulecule.config.Config): Config data
namespace (str): Namespace of the current Nulecule application

Returns:
Expand All @@ -88,7 +87,7 @@ def __init__(self, id, specversion, graph, basepath, metadata=None,
self.metadata = metadata or {}
self.graph = graph
self.requirements = requirements
self.config = config or {}
self.config = config

@classmethod
def unpack(cls, image, dest, config=None, namespace=GLOBAL_CONF,
Expand All @@ -101,7 +100,7 @@ def unpack(cls, image, dest, config=None, namespace=GLOBAL_CONF,
image (str): A Docker image name.
dest (str): Destination path where Nulecule data from Docker
image should be extracted.
config (dict): Dictionary, config data for Nulecule application.
config: An instance of atomicapp.nulecule.config.Config
namespace (str): Namespace for Nulecule application.
nodeps (bool): Don't pull external Nulecule dependencies when
True.
Expand All @@ -115,7 +114,7 @@ def unpack(cls, image, dest, config=None, namespace=GLOBAL_CONF,
if Utils.running_on_openshift():
# pass general config data containing provider specific data
# to Openshift provider
op = OpenshiftProvider(config.get('general', {}), './', False)
op = OpenshiftProvider(config.globals, './', False)
op.artifacts = []
op.init()
op.extract(image, APP_ENT_PATH, dest, update)
Expand All @@ -138,7 +137,8 @@ def load_from_path(cls, src, config=None, namespace=GLOBAL_CONF,

Args:
src (str): Path to load Nulecule application from.
config (dict): Config data for Nulecule application.
config (atomicapp.nulecule.config.Config): Config data for
Nulecule application.
namespace (str): Namespace for Nulecule application.
nodeps (bool): Do not pull external applications if True.
dryrun (bool): Do not make any change to underlying host.
Expand Down Expand Up @@ -231,25 +231,23 @@ def load_config(self, config=None, ask=False, skip_asking=False):
It updates self.config.

Args:
config (dict): Existing config data, may be from ANSWERS
file or any other source.
config (atomicapp.nulecule.config.Config): Existing config data,
may be from ANSWERS file or any other source.

Returns:
None
"""
if config is None:
config = self.config
super(Nulecule, self).load_config(
config=config, ask=ask, skip_asking=skip_asking)
if self.namespace == GLOBAL_CONF and self.config[GLOBAL_CONF].get('provider') is None:
self.config[GLOBAL_CONF]['provider'] = DEFAULT_PROVIDER
logger.info("Provider not specified, using default provider - {}".
format(DEFAULT_PROVIDER))

for component in self.components:
# FIXME: Find a better way to expose config data to components.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this FIXME can be removed since you're fixing it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yay! 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the FIXME comment is still there in the code.

# A component should not get access to all the variables,
# but only to variables it needs.
component.load_config(config=copy.deepcopy(self.config),
component.load_config(config=config,
ask=ask, skip_asking=skip_asking)
self.merge_config(self.config, component.config)

def load_components(self, nodeps=False, dryrun=False):
"""
Expand All @@ -270,8 +268,8 @@ def load_components(self, nodeps=False, dryrun=False):
node_name = node[NAME_KEY]
source = Utils.getSourceImage(node)
component = NuleculeComponent(
node_name, self.basepath, source,
node.get(PARAMS_KEY), node.get(ARTIFACTS_KEY),
self._get_component_namespace(node_name), self.basepath,
source, node.get(PARAMS_KEY), node.get(ARTIFACTS_KEY),
self.config)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PEP8? Fix spacing?

Ex:

self._get_component_namespace(node_name),
self.basepath,
source
...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

component.load(nodeps, dryrun)
components.append(component)
Expand All @@ -294,6 +292,24 @@ def render(self, provider_key=None, dryrun=False):
for component in self.components:
component.render(provider_key=provider_key, dryrun=dryrun)

def _get_component_namespace(self, component_name):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use a description somewhere for this function

"""
Get a unique namespace for a Nulecule graph item, by concatinating
the namespace of the current Nulecule (which could be the root Nulecule
app or a child or external Nulecule app) and name of the Nulecule
graph item.

Args:
component_name (str): Name of the Nulecule graph item

Returns:
A string
"""
current_namespace = '' if self.namespace == GLOBAL_CONF else self.namespace
return (
'%s%s%s' % (current_namespace, NAMESPACE_SEPARATOR, component_name)
if current_namespace else component_name)


class NuleculeComponent(NuleculeBase):

Expand Down Expand Up @@ -356,12 +372,13 @@ def load_config(self, config=None, ask=False, skip_asking=False):
"""
Load config for the Nulecule component.
"""
if config is None:
config = self.config
super(NuleculeComponent, self).load_config(
config, ask=ask, skip_asking=skip_asking)
if isinstance(self._app, Nulecule):
self._app.load_config(config=copy.deepcopy(self.config),
self._app.load_config(config=self.config,
ask=ask, skip_asking=skip_asking)
self.merge_config(self.config, self._app.config)

def load_external_application(self, dryrun=False, update=False):
"""
Expand All @@ -384,7 +401,8 @@ def load_external_application(self, dryrun=False, update=False):
'Found existing external application: %s '
'Loading: ' % self.name)
nulecule = Nulecule.load_from_path(
external_app_path, dryrun=dryrun, update=update)
external_app_path, dryrun=dryrun, update=update,
namespace=self.namespace)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PEP8

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

elif not dryrun:
logger.info('Pulling external application: %s' % self.name)
nulecule = Nulecule.unpack(
Expand Down Expand Up @@ -436,7 +454,7 @@ def render(self, provider_key=None, dryrun=False):
raise NuleculeException(
"Data for provider \"%s\" are not part of this app"
% provider_key)
context = self.get_context()
context = self.config.context(self.namespace)
for provider in self.artifacts:
if provider_key and provider != provider_key:
continue
Expand Down
173 changes: 173 additions & 0 deletions atomicapp/nulecule/config.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
import copy
import logging

from atomicapp.constants import (GLOBAL_CONF,
LOGGER_COCKPIT,
DEFAULT_PROVIDER,
DEFAULT_ANSWERS)
from collections import defaultdict

cockpit_logger = logging.getLogger(LOGGER_COCKPIT)


class Config(object):
"""
This class allows to store config data in different scopes along with
source info for the data. When fetching the value for a key in a scope,
the source info and the PRIORITY order of sources is taken into account.

Data sources:
cli: Config data coming from the CLI
runtime: Config data resolved during atomic app runtime. For example,
when the value for a parameter in a Nulecule or Nulecule graph
item is missing in answers data, we first try to load the default
value for the parameter. When there's no default value, or when
the user has specified to forcefully ask the user for values, we
ask the user for data. These data collected/resolved during runtime
form the runtime data.
answers: Config data coming from answers file
defaults: Default config data specified in atomicapp/constants.py

The priority order of the data sources is:
cli > runtime > answers > defaults
"""

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we get a summary of each source below in a comment. This would help future readers of the code.

PRIORITY = (
'cli',
'runtime',
'answers',
'defaults'
)

def __init__(self, answers=None, cli=None):
"""
Initialize a Config instance.

Args:
answers (dict): Answers data
cli (dict): CLI data
"""
answers = answers or {}
cli = cli or {}
# We use a defaultdict of defaultdicts so that we can avoid doing
# redundant checks in a nested dictionary if the value of the keys
# are dictionaries or None.
self._data = defaultdict(defaultdict)
# Initialize default data dict
self._data['defaults'] = defaultdict(defaultdict)
# Initialize answers data dict
self._data['answers'] = defaultdict(defaultdict)
# Initialize cli data dict
self._data['cli'] = defaultdict(defaultdict)
# Initialize runtime data dict
self._data['runtime'] = defaultdict(defaultdict)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be nice to have a 1 line comment above each one of these for loops below

# Load default answers
for scope, data in DEFAULT_ANSWERS.items():
for key, value in data.items():
self.set(key, value, scope=scope, source='defaults')
self.set('provider', DEFAULT_PROVIDER, scope=GLOBAL_CONF, source='defaults')

# Load answers data
for scope, data in answers.items():
for key, value in data.items():
self.set(key, value, scope=scope, source='answers')

# Load cli data
for key, value in cli.items():
self.set(key, value, scope=GLOBAL_CONF, source='cli')

def get(self, key, scope=GLOBAL_CONF, ignore_sources=[]):
"""
Get the value of a key in a scope. This takes care of resolving
the value by going through the PRIORITY order of the various
sources of data.

Args:
key (str): Key
scope (str): Scope from which to fetch the value for the key

Returns:
Value for the key.
"""
for source in self.PRIORITY:
if source in ignore_sources:
continue
value = self._data[source][scope].get(key) or self._data[source][
GLOBAL_CONF].get(key)
if value:
return value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know None will get returned here if no call to return is given but should we explicitly do it anyway in order to stop people being confused about whether we meant to do it that way or not.

return None

def set(self, key, value, source, scope=GLOBAL_CONF):
"""
Set the value for a key within a scope along with specifying the
source of the value.

Args:
key (str): Key
value: Value
scope (str): Scope in which to store the value
source (str): Source of the value
"""
self._data[source][scope][key] = value

def context(self, scope=GLOBAL_CONF):
"""
Get context data for the scope of Nulecule graph item by aggregating
the data from various sources taking their priority order into
account. This context data, which is a flat dictionary, is used to
render the variables in the artifacts of Nulecule graph item.

Args:
scope (str): Scope (or namespace) for the Nulecule graph item.
Returns:
A dictionary
"""
result = {}
for source in reversed(self.PRIORITY):
source_data = self._data[source]
result.update(copy.deepcopy(source_data.get(GLOBAL_CONF) or {}))
if scope != GLOBAL_CONF:
result.update(copy.deepcopy(source_data.get(scope) or {}))
return result

def runtime_answers(self):
"""
Get runtime answers.

Returns:
A defaultdict containing runtime answers data.
"""
answers = defaultdict(dict)

for source in reversed(self.PRIORITY):
for scope, data in (self._data.get(source) or {}).items():
answers[scope].update(copy.deepcopy(data))

# Remove empty sections for answers
for key, value in answers.items():
if not value:
answers.pop(key)

return answers

def update_source(self, source, data):
"""
Update answers data for a source.

Args:
source (str): Source name
data (dict): Answers data
"""
data = data or {}
if source not in self._data:
raise

# clean up source data
for k in self._data[source]:
self._data[source].pop(k)

for scope, data in data.items():
for key, value in data.items():
self.set(key, value, scope=scope, source=source)
Loading