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

Add version and downsert #682

Closed
wants to merge 34 commits into from
Closed

Add version and downsert #682

wants to merge 34 commits into from

Conversation

Moon1706
Copy link
Contributor

@Moon1706 Moon1706 commented Jul 9, 2022

This PR appends 2 additional fields: version and downsert.
The version variable allows you to manage the target version of Pulumi. Default value is ^3.
In turn, downsert variable in combination with destroy command removes Pulumi stack. Default value is undefined.
PS: added checking downsert in the Promise impossible, because after this we use Stack variable a few times.

dependabot bot and others added 30 commits April 29, 2022 11:39
Bumps [lint-staged](https://github.com/okonet/lint-staged) from 12.3.8 to 12.4.0.
- [Release notes](https://github.com/okonet/lint-staged/releases)
- [Commits](lint-staged/lint-staged@v12.3.8...v12.4.0)

---
updated-dependencies:
- dependency-name: lint-staged
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
# This is the 1st commit message:

Add scheme

# This is the commit message #2:

Test errors

# This is the commit message #3:

chore(dist): Update dist [automated commit]
@RobbieMcKinstry RobbieMcKinstry self-requested a review July 11, 2022 15:07
@RobbieMcKinstry
Copy link
Contributor

Hi @Moon1706 Thanks for your contribution. 👏🏻
However, I'm seeing the version selection is currently until development in #661 . I'd like to give first-time contributors an opportunity to get their contributions merged in and celebrated, especially since that PR pre-dates this one.

Also, regarding downsert, I recently posited in #681 that it might be preferable to add a pulumi stack command instead of an option on destroy. Would you add your thoughts to that issue? I'd like to hear what you have to say about that solution. :)

In the future, would you please submit separate PRs for individual features? It's very unlikely we merge a PR for more than 1 feature at a time. This helps reduce the maintenance burden, helps us to review PRs quickly, and reduces the likelihood of introducing regressions.

Thanks again for your active participation in our OSS community.

@RobbieMcKinstry RobbieMcKinstry removed their request for review July 11, 2022 15:14
@RobbieMcKinstry RobbieMcKinstry added kind/enhancement Improvements or new features blocked The issue cannot be resolved without 3rd party action. impact/usability Something that impacts users' ability to use the product easily and intuitively awaiting-feedback Blocked on input from the author labels Jul 12, 2022
@Moon1706
Copy link
Contributor Author

Moon1706 commented Jul 14, 2022

@RobbieMcKinstry
Okay. It's a draft from my own Pulumi Action and I wanna rewrite this code.
About conception. Mmm, that's a tough question. Probably, we can realize 3 various scenarios:

  1. We'll just append downsert option field. Easy, but in perspective we flood code with if section.
  2. We'll create an additional command stack rm. That's string from official Pulumi command. So, in this case, we should or realize of command, supported by stack or just make only one action for stack rm, but it can confuse users.
  3. Rename remove stack command to rm. I reckon it's a bad idea, because this doen't documented on the Pulumi site.

To recap, I think the first case is the best, but I opened for another suggestion.

@RobbieMcKinstry
Copy link
Contributor

#681 (comment)
I think I'm happy to move forward with adding a stack command with an args subcommand. :)

@Moon1706
Copy link
Contributor Author

@RobbieMcKinstry
Sorry for a long answer, a lot of work. I've thought about this suggestion and unfortunately I don't agree that's a good idea. Now, this GHA supports a declarative approach with something like CRUD requests. If we'll move to the command and args we can get strange nonsense behavior, when a person'll execute many actions like iterative commands. And that's uncorrelated with the GHA conception. But if you really wanna realize this, I can suggest this NPM package to execute Pulumi commands.

@RobbieMcKinstry
Copy link
Contributor

Thanks for the suggestion! I haven't seen this package before. I'll take a look.

I do agree that the args escape hatch is undesirable because it turns the action from declarative to imperative.
I just worry the pulumi CLI has many commands and subcommands, and supporting every possible execution of these in a declarative format will take a long time and result in a large amount of code.

I'll give it some more thought! Maybe it's enough to support only the subcommands users request once they request them, slowing the complexity growth until it's needed.

@Moon1706
Copy link
Contributor Author

@RobbieMcKinstry
Yes, please. And I have also additional suggestion. That's strange, but maybe interesting. We can create advanced configuration with Yaml structure. Like in config-map field. User'll add config with Yaml structure, we'll parse config and execute Pulumi with settings. In config Yaml we can organize 2 section: declarative and imperative and user'll choose how he or she wanna start Pulumi command.
Little disclaimer, that's just an idea. I don't see that someone makes GHA with this style.

@RobbieMcKinstry
Copy link
Contributor

Quick follow up:
The merge of #661 adds the ability to select the Pulumi version as an action input.
This feature will be released later this week in a new minor release

@Moon1706 Moon1706 closed this by deleting the head repository Oct 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-feedback Blocked on input from the author blocked The issue cannot be resolved without 3rd party action. impact/usability Something that impacts users' ability to use the product easily and intuitively kind/enhancement Improvements or new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants