Code review comment for lp:~thumper/juju-core/proxy-config-omit

Reviewers: mp+210952_code.launchpad.net,

Message:
Please take a look.

Description:
Omit empty proxy settings in config.

I had changed them to default to "" because there is
no way to unset the value once you have set one for
an environment. The correct fix is to implement
'juju unset-env'

https://code.launchpad.net/~thumper/juju-core/proxy-config-omit/+merge/210952

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/75700043/

Affected files (+9, -16 lines):
   A [revision details]
   M environs/config/config.go
   M environs/config/config_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20140313162936-3e8iq4csv4axq6z8
+New revision: <email address hidden>

Index: environs/config/config.go
=== modified file 'environs/config/config.go'
--- environs/config/config.go 2014-03-09 20:48:29 +0000
+++ environs/config/config.go 2014-03-14 02:10:35 +0000
@@ -733,15 +733,13 @@
   "bootstrap-retry-delay": schema.Omit,
   "bootstrap-addresses-delay": schema.Omit,
   "rsyslog-ca-cert": schema.Omit,
-
- // Proxy values default to "", otherwise they can't be set to blank.
- "http-proxy": "",
- "https-proxy": "",
- "ftp-proxy": "",
- "no-proxy": "",
- "apt-http-proxy": "",
- "apt-https-proxy": "",
- "apt-ftp-proxy": "",
+ "http-proxy": schema.Omit,
+ "https-proxy": schema.Omit,
+ "ftp-proxy": schema.Omit,
+ "no-proxy": schema.Omit,
+ "apt-http-proxy": schema.Omit,
+ "apt-https-proxy": schema.Omit,
+ "apt-ftp-proxy": schema.Omit,

   // Deprecated fields, retain for backwards compatibility.
   "tools-url": "",

Index: environs/config/config_test.go
=== modified file 'environs/config/config_test.go'
--- environs/config/config_test.go 2014-03-13 07:54:56 +0000
+++ environs/config/config_test.go 2014-03-14 02:10:35 +0000
@@ -1026,13 +1026,6 @@
   attrs["tools-metadata-url"] = ""
   attrs["tools-url"] = ""
   attrs["image-stream"] = ""
- attrs["http-proxy"] = ""
- attrs["https-proxy"] = ""
- attrs["ftp-proxy"] = ""
- attrs["no-proxy"] = ""
- attrs["apt-http-proxy"] = ""
- attrs["apt-https-proxy"] = ""
- attrs["apt-ftp-proxy"] = ""

   // Default firewall mode is instance
   attrs["firewall-mode"] = string(config.FwInstance)