Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Resolve #4] Fail gracefully on bad input #5

Merged
merged 6 commits into from
Jun 19, 2024

Conversation

alex-harvey-z3q
Copy link
Contributor

@alex-harvey-z3q alex-harvey-z3q commented Jun 15, 2024

Given a call to this filter like:

policy_document:
  {{ updated_policy_document | unquote_resolvers(output_indent=4, trim=True) }}

If the input does not contain a list or a dict, the filter fails confusingly like:

  File "/app/sceptre/sceptre/config/reader.py", line 509, in _render
    raise SceptreException(message) from err
sceptre.exceptions.SceptreException: s3-bucket-generic.yaml - unquote_resolvers - ('cannot represent an object', Undefined)
Templating vars saved to: /tmp/vars_fdup6pqa

This adds code to fail gracefully in this scenario.

Also updates dependencies to get the tests passing again.

Given a call to this filter like:

```jinja
policy_document:
  {{ updated_policy_document | unquote_resolvers(output_indent=4, trim=True) }}
```

If the input does not contain a list of a dict, the filter fails confusingly like:

```
  File "/app/sceptre/sceptre/config/reader.py", line 509, in _render
    raise SceptreException(message) from err
sceptre.exceptions.SceptreException: s3-bucket-generic.yaml - unquote_resolvers - ('cannot represent an object', Undefined)
Templating vars saved to: /tmp/vars_fdup6pqa
```

The filter should fail gracefully in this scenario.
@alex-harvey-z3q alex-harvey-z3q requested a review from zaro0508 June 15, 2024 05:45
@alex-harvey-z3q alex-harvey-z3q self-assigned this Jun 15, 2024
Copy link
Contributor

@zaro0508 zaro0508 left a comment

Choose a reason for hiding this comment

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

this PR seems fine to me however the pre-commit job is failing.

Also this repo is still setup to build and release with CircleCI. Sceptre has migrated away from CircleCi to GH actions therefore unit tests jobs for this project are no longer running either. You might want to enable unit tests in GH actions. For an example on how that's done you can take a look at the sceptre-resolver-template test and release workflows

@alex-harvey-z3q alex-harvey-z3q requested a review from zaro0508 June 18, 2024 13:43
@alex-harvey-z3q
Copy link
Contributor Author

Not certain, but I think I've done everything I can from my side!

@zaro0508
Copy link
Contributor

zaro0508 commented Jun 18, 2024

@alexharv074 I have fixed pre-commit and updated this repo to run tests on GH actions with PR #8 and #9.

@zaro0508
Copy link
Contributor

@alexharv074 i have updated this PR with master, but I think i made a mistake by not changing the types-PyYAML dependency to pyyaml. I think you wanted to set it to pyyaml, correct? if so please update this PR.

Copy link
Contributor

@zaro0508 zaro0508 left a comment

Choose a reason for hiding this comment

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

NVM @alexharv074, i have updated the pyyaml dependency.

@alex-harvey-z3q alex-harvey-z3q merged commit ff8751a into Sceptre:master Jun 19, 2024
9 checks passed
@alex-harvey-z3q alex-harvey-z3q deleted the ah/4-fail-gracefully branch June 19, 2024 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants