Support passing environment variables to dapr cli

[evhen14]

Resolving issue #326

There is a "standard" way for running processes. However, the original implementation is not using it. Lacking understanding why it was done this way, I only extended the current implementation.

[evhen14]

@microsoft-github-policy-service agree

[evhen14]

@philliphoff is it possible to get a review for this PR.

[philliphoff]

There is a "standard" way for running processes. However, the original implementation is not using it. Lacking understanding why it was done this way, I only extended the current implementation.

@evhen14 Much of the task implementation is from the early days of VS Code's custom task support, before there was better integration with the shell/terminal. What would you consider to be the "standard" way.

[philliphoff]

For consistency, it'd be great to support this in the daprdCommandTaskProvider and daprdDownTaskProvider as well.

[yevgen-el8]

I'll add env for the daprd command. The daprdDownTaskProvider doesn't need env variables as it only sends SIGKILL to the existing process.

[philliphoff]

Fair enough.

[yevgen-el8]

There is a "standard" way for running processes. However, the original implementation is not using it. Lacking understanding why it was done this way, I only extended the current implementation.

@evhen14 Much of the task implementation is from the early days of VS Code's custom task support, before there was better integration with the shell/terminal. What would you consider to be the "standard" way.

I'm new to VS Code extensions and it seems like https://code.visualstudio.com/api/references/vscode-api#ProcessExecution does take into account the options from the task definition and that's what I refer to.

[philliphoff]

I'm new to VS Code extensions and it seems like https://code.visualstudio.com/api/references/vscode-api#ProcessExecution does take into account the options from the task definition and that's what I refer to.

@yevgen-el8 It looks like ProcessExecution will automatically merge the parent process environment, but would still rely on explicitly merging the environment from the task definition. Another downside to ProcessExecution is, as far as I can tell, that you don't notification of output or completion, which is necessary for custom tasks.

[philliphoff]

Looks good overall. I think the only thing left is to flesh out the task definitions for both dapr and daprd tasks (e.g. to include the env property on the option property).

[yevgen-el8]

Looks good overall. I think the only thing left is to flesh out the task definitions for both dapr and daprd tasks (e.g. to include the env property on the option property).

I'm not sure what you mean there. The task definition already has options defined and Options interface has env property

[philliphoff]

I'm not sure what you mean there. The task definition already has options defined and Options interface has env property

@yevgen-el8 Sorry, I meant the task JSON schema definition in the package.json.

[evhen14]

I'm not sure what you mean there. The task definition already has options defined and Options interface has env property

@yevgen-el8 Sorry, I meant the task JSON schema definition in the package.json.

@philliphoff Thank you. Please have a look.

[philliphoff]

Looks good, thanks for the contribution!