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

Support for 'stack output' command (reopened) #1002

Merged
merged 23 commits into from
Jan 16, 2024

Conversation

mikocot
Copy link
Contributor

@mikocot mikocot commented Jul 31, 2023

This PR introduced support for pulumi stack output command in addition to the existing ones.

The reasons for this change are explained in the following issue:
#868

I've chosen output as a name for the command as it reflects the method called by CLI, although internally in the sdk it calls stack.outputs().

As mentioned previously I have issues testing this solution, so I invite you to test it yourself and collaborate on the final solution. I have found no relevant tests nor I can see a good way to unit test it, but I've updated the test workflow.

__
Reopened due to GH not recognizing the changes correctly in the original one after rebase.

@mikocot
Copy link
Contributor Author

mikocot commented Jul 31, 2023

@RobbieMcKinstry please verify it on your side, since there have been multiple changes in the project in the meantime.
Then let's either merge it or reject it, there is no point for it to be hanging here for months, while it is still a feature WE DO NEED and we keep using ugly workarounds as this is not available.

@RobbieMcKinstry
Copy link
Contributor

Hi @mikocot , I'm afraid I'm no longer a maintainer on this repository. I'll refer you to @justinvp or @simenandre who are able to review your pull request.

As a fellow community member, thank you for your contribution, and your patience. I apologize for having lost track of your PR when I was a maintainer. It was an oversight.

@Frassle
Copy link
Member

Frassle commented Aug 9, 2023

This looks reasonable to me. @simenandre any comments before we merge? My only thought is maybe this should have a test for it, but I'm not up to speed with how we're doing testing in this repo, so I'll defer that to you.

@mikocot
Copy link
Contributor Author

mikocot commented Oct 9, 2023

@simenandre any comments? :)

@Frassle
Copy link
Member

Frassle commented Oct 19, 2023

Looks like something needs an update here, CI is complaining about:

Error: [tsl] ERROR in /home/runner/work/actions/actions/src/main.ts(101,5)
      TS2322: Type '{ up: () => Promise<string>; update: () => Promise<string>; refresh: () => Promise<string>; destroy: () => Promise<string>; preview: () => Promise<string>; output: () => Promise<...>; }' is not assignable to type 'Record<"up" | "update" | "refresh" | "destroy" | "preview", () => Promise<string>>'.
  Object literal may only specify known properties, and 'output' does not exist in type 'Record<"up" | "update" | "refresh" | "destroy" | "preview", () => Promise<string>>'.

@RobbieMcKinstry
Copy link
Contributor

Looks like something needs an update here, CI is complaining about:

Error: [tsl] ERROR in /home/runner/work/actions/actions/src/main.ts(101,5)
      TS2322: Type '{ up: () => Promise<string>; update: () => Promise<string>; refresh: () => Promise<string>; destroy: () => Promise<string>; preview: () => Promise<string>; output: () => Promise<...>; }' is not assignable to type 'Record<"up" | "update" | "refresh" | "destroy" | "preview", () => Promise<string>>'.
  Object literal may only specify known properties, and 'output' does not exist in type 'Record<"up" | "update" | "refresh" | "destroy" | "preview", () => Promise<string>>'.

I think output is missing from the enum of available commands.
https://github.com/pulumi/actions/blob/master/src/config.ts#L36C6-L36C6

@mikocot
Copy link
Contributor Author

mikocot commented Oct 20, 2023

to be honest it might have got out of date in the meantime 🙄

@mikocot
Copy link
Contributor Author

mikocot commented Jan 16, 2024

I fixed the previous conflict on the changelog, but now there is a new one. Can we please move on with this? @Frassle ?

@Frassle Frassle merged commit cd64c66 into pulumi:master Jan 16, 2024
76 checks passed
@Frassle
Copy link
Member

Frassle commented Jan 16, 2024

There we go :) Was a couple of minor things to fix up but merged now.

@mikocot
Copy link
Contributor Author

mikocot commented Jan 16, 2024

great, we'll test it as soon as it's out, thanks

@darena-patrick
Copy link

I pulled this down today and tried to use it in place of what I was previously running as preview - it doesn't seem to set the outputs the same way that preview does - is there something else special needed?

When running stack:
Screenshot 2024-01-31 at 4 40 16 PM

When running preview:
Screenshot 2024-01-31 at 4 39 25 PM

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.

4 participants