-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 printing out outputs in a pulumi program. #1531
Conversation
if row, has = display.eventUrnToResourceRow[""]; has { | ||
row.SetStep(step) | ||
display.stackUrn = urn |
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.
this was bug number one. we were only setting the display's "stack urn" if we heard about the stack and we had previously heard about a "global" message. Now we universally set the display's stackURN 4 lines above.
This is what was causing us to not print outputs. The stored stackURN was staying "" which meant this code didn't run:
// If we're not previewing, and we get stack outputs, then display them at the end.
if !display.isPreview {
if display.stackUrn != "" {
stackStep := display.eventUrnToResourceRow[display.stackUrn].Step()
} | ||
parentNode := display.getOrCreateTreeNode(result, parentURN, parentRow, urnToTreeNode) | ||
parentNode.childNodes = append(parentNode.childNodes, node) | ||
return node |
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 updated this code because hte previous logic kept breaking my brain. Effectively i wanted to say: for any resource (except the root-stack resource), you should alwyas be able to parent it to another resource. The easiest way to do this was to ensure we'd always have a root tree node to parent to. And the easiest way to do that was to ensure we had a root resource-row we'd generate the root node out of.
3eb195a
to
e0476a1
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
Excellent! Can we also add a test to prevent breaking again in the future? |
Checking! |
9f7cdd6
to
2387a2a
Compare
Fixes #1296