Don't mix unset and exports, and cater for spaces in the cert path #351
Conversation
| } | ||
| } | ||
| } | ||
| env := exports(socket, certPath) |
There was a problem hiding this comment.
The downside to this is that if another env var is introduced tomorrow, this change will no longer report it, as it hard-codes the env var keys. Your call, though, Sven.
nit: looks like 201/202 were not both formatted by gofmt.
Otherwise, LGTM.
There was a problem hiding this comment.
yes, the magic future proofing was neat looking, and worked for TLS, but with b2d being slowly on the way out, and as I fixed the spaced path in docker-machine, figure going with the same code they have is safe enough :)
Signed-off-by: Sven Dowideit <SvenDowideit@home.org.au>
SvenDowideit
force-pushed
the
use-machine-env-code
branch
from
7ee551b to
1c48ea8
Compare
|
Another env var was just introduced with #345. Also this one is not always present and can be either no_proxy or NO_PROXY. So I think they old way of doing it dynamic needs to stay. |
|
If we suggest that people use |
|
no? the issue lists a number of problems with the code currently. |
|
@databus23 cool - I'll code it in too - presumably machine has added it to their code? |
|
What I'm saying is that using |
|
As was mentioned in boot2docker/boot2docker#716 (comment) 👍 |
|
What I'm trying to say is that we should do this, yes, but we should also update our documentation to include |
|
@tianon oh. ic - is that what you didn't say :p |
| @@ -225,7 +218,7 @@ func exports(socket, certPath string) map[string]string { | |||
| if certPath == "" { | |||
| out["DOCKER_TLS_VERIFY"] = "" | |||
| } else { | |||
| out["DOCKER_TLS_VERIFY"] = "1" | |||
| out["DOCKER_TLS_VERIFY"] = "yes" | |||
There was a problem hiding this comment.
What's the motivation for changing this one?
There was a problem hiding this comment.
to make it the same as in machine....
I'm basically taking the POV that if our code looks more like theirs, then its will be easier for people that might need to back-port changes.
There was a problem hiding this comment.
There was a problem hiding this comment.
|
replaced by #361 |
Signed-off-by: Sven Dowideit SvenDowideit@home.org.au
I've copied the docker-machine code, and then escape the spaces.
Closes: boot2docker/boot2docker#716 boot2docker/boot2docker#715