Replace Config class with ConfigReader class #259
Conversation
theseanything
force-pushed
the
refactor-config-to-config-reader
branch
from
3713a8c
to
6bc4323
Compare
theseanything
force-pushed
the
refactor-config-to-config-reader
branch
from
6bc4323
to
a172d61
Compare
sceptre/config_reader.py
Outdated
|
|
||
| def __init__(self, sceptre_dir, variables=None): | ||
| self.logger = logging.getLogger(__name__) | ||
| self._deferred_constructors = [] |
| del self.templating_vars["environment_config"] | ||
| return stack | ||
|
|
||
| def construct_stack(self, rel_path): |
There was a problem hiding this comment.
rel_path not clear
sceptre/environment.py
Outdated
|
|
||
| self._is_leaf = None | ||
| self.stacks = [] | ||
| self.environments = [] |
There was a problem hiding this comment.
Potentially rename to sub_environments
| env = Environment(context.sceptre_dir, environment_name) | ||
| stack = env.stacks[basename] | ||
| config_reader = ConfigReader(context.sceptre_dir) | ||
| stack = config_reader.construct_stack(stack_name + ".yaml") |
There was a problem hiding this comment.
If we address #221 hard coding this .ymal extension may cause issues. Consider a different approach that does not use a hardcoded value for the extension.
There was a problem hiding this comment.
This is just for the stack config which can only be .yaml at the moment - but yeah possible improvement at a later date.
| :type environment_path: str | ||
| :param options: A dict of key-value pairs to update self.config with. | ||
| :type debug: dict | ||
| :param environment: The name of the environment. |
There was a problem hiding this comment.
A very pedantic point but parameters docstrings in reST format typically don't end with a full stop. If this is part of project's conventions then just ingnore this.
There was a problem hiding this comment.
We've used full stops everywhere else, so will just stick with it for now to be consistent.
sceptre/cli.py
Outdated
| :type environment: str | ||
| :param stack: The name of the stack. | ||
| :type stack: str | ||
| :returns: An Stack. |
There was a problem hiding this comment.
Typo. Change An Stack. to A Stack.
sceptre/hooks/__init__.py
Outdated
| hooks used in hooks. Environment and stack config and the connection | ||
| manager are supplied to the class, as they may be of use to inheriting | ||
| classes. | ||
| hooks used in hooks. |
There was a problem hiding this comment.
Consider changing hooks used in hooks. to hooks..
So that the sentence reads:
Hook is an abstract base class that should be inherited by all hooks.
theseanything
force-pushed
the
refactor-config-to-config-reader
branch
from
a7cb881
to
1c9cb23
Compare
This PR aims are replacing the Config class with a ConfigReader class. This is apart of an effort to simplify constructing Stack and Environment objects, addressing issue #5.
Currently Sceptre's main entry point when using it as Python module is to create an environment object using the
Environment(sceptre_dir, environment_path, options=None)constructor. The Environment class is responsible for reading the relevant config from a Sceptre project folder and creating Stack objects. Additionally Config class is a subclass of the built-in dict with custom class attributes - which make testing incredible difficult and confusing on how to access relevant properties (i.e. is it a key or a attribute -config["region"]vsconfig.sceptre_dir).The new ConfigReader class aims to fix these issues - by encapsulating functions related to gathering/parsing Sceptre project folders and reading in config (instead of the Environment class). -
construct_stackandconstruct_environmentmethods now provide the ability to generate Stack or Environment object respectively using config files.This API change include referencing stacks and environment by full relative paths - laying the foundation to solve issue #62.
[Resolve #issue-number](if a related issue exists).make test) are passing.make lint) offenses.and description in grammatically correct, complete sentences. - Sort of, difficult to do in stages.