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

consistently handle environment variables #212

Merged
merged 2 commits into from
Sep 4, 2020
Merged

Conversation

EvanBoyle
Copy link
Contributor

This change makes sure that any environment variables passed in are appended to the current OS env vars. Adds tests to document behavior across languages.

Fixes #196

Note: there is a breaking change for go types for Args and Env, but this is in the category of "never worked in the first place". The unmarshalling fails when you specify environment variables as is, so this change to types just makes it work as intended.

Copy link
Contributor

@stack72 stack72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes to the yml files need to propagate through ci-mgmt as per the demo and there's a real question over changing from a pulumi.MapInput to a pulumi.StringMap incase this is going to be a breaking change for users

.github/workflows/master.yml Show resolved Hide resolved
@@ -82,7 +82,7 @@ type DockerBuildArgs struct {
// An optional map of named build-time argument variables to set during the Docker build.
// This flag allows you to pass built-time variables that can be accessed like environment variables
// inside the `RUN` instruction.
Args pulumi.MapInput `pulumi:"args"`
Args pulumi.StringMap `pulumi:"args"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a breaking change? Users will need to update their code right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is mentioned in the PR description. Args and Env don't work as implemented. There's a panic unmarshalling to the wrong type.

@EvanBoyle EvanBoyle merged commit 77e4d06 into master Sep 4, 2020
@pulumi-bot pulumi-bot deleted the evan/fixEnvVars branch September 4, 2020 15:42
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.

3 participants