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

fix: display no outputs if outputs are not defined. #1790

Merged
merged 1 commit into from
Feb 12, 2020

Conversation

sriram-mv
Copy link
Contributor

Why is this change necessary?

  • bug from previous PR which always looked for outputs in deployed
    templates.

How does it address the issue?

  • Explicitly check for outputs, before attempting to display them.
  • Added extra tests to verify so that it does not cause regressions in
    the future.

What side effects does this change have?

  • None.

Checklist:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -401,7 +401,8 @@ def wait_for_execute(self, stack_name, changeset_type):
raise deploy_exceptions.DeployFailedError(stack_name=stack_name, msg=str(ex))

outputs = self.get_stack_outputs(stack_name=stack_name, echo=False)
self._stack_outputs(outputs)
if outputs:
self._stack_outputs(outputs)
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be given a more descriptive name? When I saw this code in your old PR, I thought it was functional - turns out this is just cosmetic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah true, I'll change it to display stack outputs.

Why is this change necessary?

* bug from previous PR which always looked for outputs in deployed
templates.

How does it address the issue?

* Explicitly check for outputs, before attempting to display them.
* Added extra tests to verify so that it does not cause regressions in
the future.

What side effects does this change have?

* None.
@sriram-mv sriram-mv merged commit b352127 into aws:develop Feb 12, 2020
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.

None yet

3 participants