Skip to content

(PUP-1209) Allow setting empty environment vars#5426

Closed
petems wants to merge 1 commit intopuppetlabs:masterfrom
petems:PUP-1209-allow_empty_environment_settings
Closed

(PUP-1209) Allow setting empty environment vars#5426
petems wants to merge 1 commit intopuppetlabs:masterfrom
petems:PUP-1209-allow_empty_environment_settings

Conversation

@petems
Copy link
Contributor

@petems petems commented Dec 13, 2016

  • Adds branch to logic to allow setting empty environment variable
  • For example, allows setting of HTTP_PROXY to empty for an exec

* Adds branch to logic to allow setting empty environment variable
* For example, allows setting of `HTTP_PROXY` to empty for an exec

expect(@logs.map {|l| "#{l.level}: #{l.message}" }).to eq(["warning: Environment Setting WHATEVER set to `WHATEVER=`, if this is unexpected check the variable has been assigned in the string"])
end

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 needs better tests, but I'm not sure about the tests around environment settings, they're a bit complicated, might need a bit of assistance 👍

@puppetcla
Copy link

CLA signed by all contributors.

environment[env_name] = value
elsif setting =~ /^(\w+)=$/
env_name = $1
warning "Environment Setting #{env_name} set to `#{env_name}=`, if this is unexpected check the variable has been assigned in the string"
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting that it always warns when setting env variables. I'd like to see the messages cleaned up a bit so we always produce one warning that describes whether we're unsetting an environment variable or having no effect. Especially since the 2nd warning right now will always have an empty-string value.

@MosesMendoza
Copy link
Contributor

This PR hasn't been updated in some time, and it seems unlikely to get picked up in the near future. I'm going to close this for now, and we can re-open if/when PUP-1209 is prioritized for the 5.x series. We can track future conversation in that ticket.

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.

5 participants