Allow opt-out of adding stdout/stderr env variables for remote command

[julsemaan]

Context

At the moment, stdout and stderr of previous runs are passed as environment variables on future invocations. When stdout or stderr is too large, this cause an error.

Proposed change

355 fixed it for local commands, this fixed it for remote commands. See #285 for full details on the issue

See this comment for details around local vs remote functionality : https://github.com/pulumi/pulumi-command/issues/285#issuecomment-2122415592

[julsemaan]

I wasn't able to successfully write a test for this one given I couldn't use the preview mode and it was trying to actually SSH into hosts. I'm lacking understanding of how to properly mock this within this plugin. Not sure if the PR can still be merged without the test.

[github-actions[bot]]

PR is now waiting for a maintainer to run the acceptance tests. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

[thomas11]

Thanks for the PR @julsemaan!

The change is simple enough that we could get away without a test. But if you wanted to try your hand at it, I've had success with spinning up a local, in-process SSH server and testing that way. The PR isn't merged yet but you can see it here.

[julsemaan]

Hey! If I can get away without a test, I'll take it 😄

I could always implement a test in a follow-up (pinky promess?). I'll be able to reuse some of your code vs doing some copy/pasta

[thomas11]

Sure, I think we can get away with that :)

However, I just noticed that your change is missing adding the new property to Annotate for the description and setting the default to true.

[github-actions[bot]]

PR is now waiting for a maintainer to run the acceptance tests. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

[julsemaan]

Sure, I think we can get away with that :)

However, I just noticed that your change is missing adding the new property to Annotate for the description and setting the default to true.

Good catch, just pushed the change

[github-actions[bot]]

PR is now waiting for a maintainer to run the acceptance tests. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

[github-actions[bot]]

PR is now waiting for a maintainer to run the acceptance tests. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

[mikhailshilkov]

@julsemaan For any current reviewers and future readers, could you please update the description of your PR to include a detailed explanation of your change and motivation for doing it?

Here are some example of good PR descriptions: https://github.com/pulumi/pulumi-kubernetes-operator/pull/575, https://github.com/pulumi/pulumi-github/pull/280, https://github.com/pulumi/pulumi/pull/13066.

Thank you for your contribution!

[julsemaan]

@julsemaan For any current reviewers and future readers, could you please update the description of your PR to include a detailed explanation of your change and motivation for doing it?

Here are some example of good PR descriptions: pulumi/pulumi-kubernetes-operator#575, pulumi/pulumi-github#280, pulumi/pulumi#13066.

Thank you for your contribution!

Hey! That's done, I could suggest to have a PR template so that contributors know more precisely what is expected to provide when opening a PR. Github makes this quite easy, see https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/creating-a-pull-request-template-for-your-repository

[thomas11]

@julsemaan, on second thought, I believe we should include test coverage here. If you just copy roughly lines 23-53 from here, that should give you the setup to use SSH in your test.

[theneva]

Hi, @julsemaan! Thank you so much for implementing the fix :smile:

Given that this whole thing stems from my feature request, and you never asked to dive into the remote command resource, I went ahead and wrote a test that I think does the job. I sent a pull request to your fork here: https://github.com/julsemaan/pulumi-command/pull/1

@thomas11 Do you think the test I wrote looks okay? (If yes, I believe you are also able to pull my commit into this PR, assuming Julien turned maintainer edits on.)

[julsemaan]

@theneva , thanks for doing the test :) I just pulled it into this branch

[github-actions[bot]]

PR is now waiting for a maintainer to run the acceptance tests. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

[theneva]

@thomas11 Thanks for the review! I tried following your feedback here: https://github.com/julsemaan/pulumi-command/pull/2/files.

Since I don't have push access to the main repo or Julien's fork, this workflow kind of sucks for everyone involved… I think it's easiest if you just look at the PR I sent to Julien's branch in case you have more feedback, so that they don't have to jump for every feedback loop.

(Sorry for all the notifications, @julsemaan 😅)

[github-actions[bot]]

PR is now waiting for a maintainer to run the acceptance tests. Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

[thomas11]

/run-acceptance-tests

[pulumi-bot]

Please view the PR build: https://github.com/pulumi/pulumi-command/actions/runs/9549446507

[thomas11]

/run-acceptance-tests

[pulumi-bot]

Please view the PR build: https://github.com/pulumi/pulumi-command/actions/runs/9549958732

[thomas11]

Merged! Thank you very much for all your efforts, @julsemaan and @theneva!