Docker build environment config precedence

[kaos]

Describe the bug

I think this precedence is backwards:
https://github.com/pantsbuild/pants/blob/main/src/python/pants/backend/docker/util_rules/docker_build_env_test.py#L44-L46

Pants version
bleeding edge

OS
Any.

Additional info

I think that whatever value you have in your environment should be used over any hardcoded value in your configuration. So that the configuration may be used as a default fallback value, and the environment can be used to override that as required.

# pants.toml
[docker]
env_vars = ["DOCKER_HOST=/var/run/docker.sock"]

$ DOCKER_HOST=tcp://my.docker.build-server:12345 ./pants ...

I would expect the DOCKER_HOST setting in the env to be respected there.. it is currently not.

[kaos]

OK, now I remember the reason why it is the way it is. It comes from how the Docker env cli option works, which only uses the value from the process environment if the value is not specified. So docker --env NAME takes value from the proc env, where as docker --env NAME=VALUE does not.

So, it depends how you look at this option, what to expect, really. And I'm not sure if there is one correct answer. We could use both, controlled by a feature toggle option.. (yet another option), so not sure if that is desirable.

[benjyw]

I tend to think that it's right the way it is, for the reason you state - it mirrors how --env works. I don't think toggling this via option makes sense.

[kaos]

Thanks @benjyw. Trying to figure out how to have a default value in the configuration, that can be overridden by env var.. hmm, perhaps a separate pants.toml file is the answer to that.. 🤔

[martimlobao]

Just my two cents, but also feel that it makes sense to have the "hardcoded" values take precedence over any env variables that happen to be set in your local environment, unless explicitly overridden. If I want to spin up a docker container I don't want to have to unset every local env variable to be able to do so.

[benjyw]

Thanks @benjyw. Trying to figure out how to have a default value in the configuration, that can be overridden by env var.. hmm, perhaps a separate pants.toml file is the answer to that.. 🤔

My gut reaction is "don't do that" :) There is a cost to complexity. And there is already a lot of flexibility built in here.

[kaos]

Ok, yeah let's close this as "not a bug". :)