Skip to content

Commit

Permalink
fix RunwayModule not subscriptable & awacs/troposphere unload causing…
Browse files Browse the repository at this point in the history
… isinstance inconsistencies (#361)

* fix module not subscriptable

introduced in #354

* exclude awacs and troposphere from cfngin sys.modules unload

bug introduced in #351
resolves #360

* update cfngin test with new call args

* add test for sys_module_exclude
  • Loading branch information
ITProKyle authored Jun 9, 2020
1 parent adb01b2 commit 7760402
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 7 deletions.
6 changes: 4 additions & 2 deletions runway/cfngin/cfngin.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,13 @@ def deploy(self, force=False, sys_path=None):
sys_path = self.sys_path
config_files = self.find_config_files(sys_path=sys_path)

with SafeHaven(environ=self.__ctx.env_vars):
with SafeHaven(environ=self.__ctx.env_vars,
sys_modules_exclude=['awacs', 'troposphere']):
for config in config_files:
ctx = self.load(config)
LOGGER.info('%s: deploying...', os.path.basename(config))
with SafeHaven(argv=['stacker', 'build', ctx.config_path]):
with SafeHaven(argv=['stacker', 'build', ctx.config_path],
sys_modules_exclude=['awacs', 'troposphere']):
action = build.Action(
context=ctx,
provider_builder=self._get_provider_builder(
Expand Down
12 changes: 12 additions & 0 deletions runway/module/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,18 @@ def destroy(self):
raise NotImplementedError('You must implement the destroy() method '
'yourself!')

def __getitem__(self, key):
"""Make the object subscriptable.
Args:
key (str): Attribute to get.
Returns:
Any
"""
return getattr(self, key)


class RunwayModuleNpm(RunwayModule): # pylint: disable=abstract-method
"""Base class for Runway modules that use npm."""
Expand Down
11 changes: 9 additions & 2 deletions runway/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -297,12 +297,16 @@ class SafeHaven(AbstractContextManager):
"""

# pylint: disable=redefined-outer-name
def __init__(self, argv=None, environ=None, sys_path=None):
def __init__(self, argv=None, environ=None, sys_modules_exclude=None,
sys_path=None):
"""Instantiate class.
Args:
argv (Optional[List[str]]): Override the value of sys.argv.
environ (Optional[Dict[str, str]]): Update os.environ.
sys_modules_exclude (Optional[List[str]]): A list of modules to
exclude when reverting sys.modules to its previous state.
These are checked as ``module.startswith(name)``.
sys_path (Optional[List[str]]): Override the value of sys.path.
"""
Expand All @@ -314,6 +318,7 @@ def __init__(self, argv=None, environ=None, sys_path=None):
self.__sys_path = list(sys.path)
# more informative origin for log statements
self.log = logging.getLogger('runway.' + self.__class__.__name__)
self.sys_modules_exclude = sys_modules_exclude or []

if isinstance(argv, list):
sys.argv = argv
Expand Down Expand Up @@ -347,7 +352,9 @@ def reset_sys_modules(self):
# sys.modules can be manipulated to force reloading modules but,
# replacing it outright does not work as expected
for module in list(sys.modules.keys()):
if module not in self.__sys_modules:
if module not in self.__sys_modules and \
not any(module.startswith(n)
for n in self.sys_modules_exclude):
self.log.debug('removed sys.module: {"%s": "%s"}', module,
sys.modules.pop(module))

Expand Down
9 changes: 6 additions & 3 deletions tests/cfngin/test_cfngin.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,15 @@ def test_deploy(self, mock_action, cfngin_fixtures, tmp_path,
{'concurrency': 0,
'tail': False}])
patch_safehaven.assert_has_calls([
call(environ=context.env_vars),
call(environ=context.env_vars,
sys_modules_exclude=['awacs', 'troposphere']),
call.__enter__(),
call(argv=['stacker', 'build', str(tmp_path / 'basic.yml')]),
call(argv=['stacker', 'build', str(tmp_path / 'basic.yml')],
sys_modules_exclude=['awacs', 'troposphere']),
call.__enter__(),
call.__exit__(None, None, None),
call(argv=['stacker', 'build', str(tmp_path / 'basic2.yml')]),
call(argv=['stacker', 'build', str(tmp_path / 'basic2.yml')],
sys_modules_exclude=['awacs', 'troposphere']),
call.__enter__(),
call.__exit__(None, None, None),
call.__exit__(None, None, None),
Expand Down
16 changes: 16 additions & 0 deletions tests/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,22 @@ def test_sys_modules(self, caplog, monkeypatch):
assert caplog.messages[:2] == expected_logs
assert caplog.messages[-1] == 'leaving the safe haven...'

def test_sys_modules_exclude(self, monkeypatch):
"""Test sys.modules interations with excluded module."""
monkeypatch.setattr(SafeHaven, 'reset_all', MagicMock())

module = 'tests.fixtures.mock_hooks'

assert module not in sys.modules
with SafeHaven(sys_modules_exclude=module) as obj:
from .fixtures import mock_hooks # noqa pylint: disable=E,W,C
assert module in sys.modules
obj.reset_sys_modules()
assert module in sys.modules
# cleanup
del sys.modules[module]
assert module not in sys.modules

@pytest.mark.parametrize('provided', TEST_PARAMS)
def test_sys_path(self, provided, caplog, monkeypatch):
"""Test sys.path interactions."""
Expand Down

0 comments on commit 7760402

Please sign in to comment.