CFn: improve error messages #11591
Merged
CFn: improve error messages #11591
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
simonrw
added
the
semver: patch
simonrw
force-pushed
the
cfn/dx/better-error-messages
branch
from
13f53f1 to
bfb4a18
Compare
|
The CI is green apart from one failing test with the arm tests: |
simonrw
force-pushed
the
cfn/dx/better-error-messages
branch
from
bfb4a18 to
0cff704
Compare
simonrw
force-pushed
the
cfn/dx/better-error-messages
branch
from
0cff704 to
5109aa9
Compare
simonrw
requested review from
dominikschubert,
pinzon and
Morijarti
as code owners
pinzon
approved these changes
Oct 7, 2024
dominikschubert
approved these changes
Oct 16, 2024
There was a problem hiding this comment.
LGTM! Nice addition to improve clarity and allow for more insight into root causes. In terms of the tests, I think it might be good to separate these kind of tests a bit more (e.g. a separate class) but that's more of an optional nit and up for discussion at a later point.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
When using our CloudFormation implementation, we often don't change errors when they are raised. This usually means the user is faced with horrible Python tracebacks that require our input to resolve.
A much better story is to parse and validate the template by converting the template into a meaningful graph of resources, with parameters, conditions and mappings resolved at template parse time, however we are not quite at that stage yet. The CFn implementation in LocalStack currently works, but is not pretty, and would take some effort to re-write at the moment, and at this stage it is not worth it.
The next best thing is to loose focus on parity with AWS and do what we can by improving error messages on obvious conditions. This PR tackles some of these things.
Changes
TODO
More improvements...