Skip to content

Commit

Permalink
[Resolve #1436] Handle exception in differ (#1437)
Browse files Browse the repository at this point in the history
See also Issue #1336. Before this change, in the event of Parameters
being entered of types other than str (e.g. bool, presumably float,
int etc), the differ would bomb out with a confusing AttributeError
error when trying to rstrip newline characters.

This changes to instead raise a SceptreException at that point, with an
informative error message to help the caller understand what they did
wrong.
  • Loading branch information
alexharv074 committed Feb 8, 2024
1 parent 609e4f1 commit 3f16bed
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 6 deletions.
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ $ git clone git@github.org:<github_username>/sceptre.git
[virtual environment](http://docs.python-guide.org/en/latest/dev/virtualenvs/))

```bash
$ poetry install --all-extras -v
$ poetry install --all-extras -v
```

4. Create a branch for local development:
Expand Down
20 changes: 15 additions & 5 deletions sceptre/diffing/stack_differ.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from cfn_tools import ODict
from yaml import Dumper

from sceptre.exceptions import SceptreException
from sceptre.plan.actions import StackActions
from sceptre.stack import Stack

Expand Down Expand Up @@ -173,9 +174,18 @@ def _extract_parameters_from_generated_stack(self, stack: Stack) -> dict:
if value is None:
continue

if isinstance(value, list):
value = ",".join(item.rstrip("\n") for item in value)
formatted_parameters[key] = value.rstrip("\n")
try:
if isinstance(value, list):
value = ",".join(item.rstrip("\n") for item in value)
formatted_parameters[key] = value.rstrip("\n")
# Other unexpected data can get through and this would blow up the differ
# and lead to quite confusing exceptions being raised. This check here could
# be removed in a future version of Sceptre if the reader class did sanity checking.
except AttributeError:
raise SceptreException(
f"Parameter '{key}' whose value is {value} "
f"is of type {type(value)} and not expected here"
)

return formatted_parameters

Expand Down Expand Up @@ -361,7 +371,7 @@ def __init__(
self,
show_no_echo=False,
*,
universal_template_loader: Callable[[str], Tuple[dict, str]] = cfn_flip.load
universal_template_loader: Callable[[str], Tuple[dict, str]] = cfn_flip.load,
):
"""Initializes a DeepDiffStackDiffer.
Expand Down Expand Up @@ -409,7 +419,7 @@ def __init__(
self,
show_no_echo=False,
*,
universal_template_loader: Callable[[str], Tuple[dict, str]] = cfn_flip.load
universal_template_loader: Callable[[str], Tuple[dict, str]] = cfn_flip.load,
):
"""Initializes a DifflibStackDiffer.
Expand Down
12 changes: 12 additions & 0 deletions tests/test_diffing/test_stack_differ.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
DeepDiffStackDiffer,
DifflibStackDiffer,
)
from sceptre.exceptions import SceptreException
from sceptre.plan.actions import StackActions
from sceptre.stack import Stack

Expand Down Expand Up @@ -343,6 +344,17 @@ def test_diff__generated_stack_has_none_for_parameter_value__its_treated_like_it
self.expected_deployed_config, expected_generated
)

def test_diff__generated_stack_has_a_bool(
self,
):
self.parameters_on_stack_config["new"] = True
message = (
"Parameter 'new' whose value is True is of type "
"<class 'bool'> and not expected here"
)
with pytest.raises(SceptreException, match=message):
self.differ.diff(self.actions)

def test_diff__stack_exists_with_same_config_but_template_does_not__compares_identical_configs(
self,
):
Expand Down

0 comments on commit 3f16bed

Please sign in to comment.