-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
CFn v2: support outputs #12536
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
CFn v2: support outputs #12536
Conversation
484cf82
to
451f836
Compare
LocalStack Community integration with Pro 2 files 2 suites 20m 47s ⏱️ Results for commit 271ec4c. ♻️ This comment has been updated with latest results. |
6b187b5
to
b07c444
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, it looks very good!
Aside from a couple of nits for future consideration, I see some tests in localstack/tests/aws/services/cloudformation/v2
failing due to the v2 returning empty Outputs fields; perhaps we can address this detail in this PR?
"Outputs": [ | ||
# TODO(parity): Description, ExportName | ||
# TODO(parity): what happens on describe stack when the stack has not been deployed yet? | ||
{"OutputKey": k, "OutputValue": v} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could instantiate these as localstack.aws.api.cloudformation.Output
@@ -27,10 +30,18 @@ | |||
LOG = logging.getLogger(__name__) | |||
|
|||
|
|||
@dataclass | |||
class ChangeSetModelExecutorResult: | |||
resources: dict |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving forward we could think of typing these
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to do a proper typing pass when everything has settled 👍 A big issue with the old provider is lack of types
b07c444
to
b111505
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Motivation
Currently the preprocessor supports evaluating outputs, but the executor does not use these values.
Changes
execute
method to return a structured value which includes the stack outputs