[DKP-2535] Honour context if present for default client, add contexts support#3401
[DKP-2535] Honour context if present for default client, add contexts support#3401ebriney wants to merge 2 commits into
Conversation
Resolves a Docker CLI context (honouring DOCKER_CONTEXT, then currentContext in ~/.docker/config.json, then the built-in default context) and returns base_url / tls kwargs ready to be passed to APIClient. Used by the client to talk to Docker Desktop out of the box. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Emmanuel Briney <emmanuel.briney@docker.com>
DockerClient.from_env() now falls back to the current Docker CLI context when DOCKER_HOST is unset, so docker.from_env() targets Docker Desktop (or any other configured context) out of the box. DOCKER_HOST still wins when set, matching the CLI's precedence. Callers can opt out with use_context=False. Also adds DockerClient.from_context(name=None) (exposed as docker.from_context) for explicit context selection. Existing pool-size unit tests are pinned with use_context=False so they stay deterministic regardless of the developer's current context. New tests cover: current-context fallback, DOCKER_HOST precedence, opt-out, named-context selection, unknown-context safety, and default-context resolution. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Emmanuel Briney <emmanuel.briney@docker.com>
39699ff to
37e5945
Compare
| max_pool_size = kwargs.pop('max_pool_size', DEFAULT_MAX_POOL_SIZE) | ||
| version = kwargs.pop('version', None) | ||
| use_ssh_client = kwargs.pop('use_ssh_client', False) | ||
| use_context = kwargs.pop('use_context', True) |
There was a problem hiding this comment.
Making this True by default is potentially a breaking change.
Not sure what's the best approach
There was a problem hiding this comment.
DOCKER_HOST stills get precedence.
It will only break for people who changed context that's a bit the goal of the PR.
Default context is targeting default value so for users that didn't touch contexts it will be the same behavior, no?
There was a problem hiding this comment.
Perhaps we use context as default but switch back to previous implem if DOCKER_API_SKIP_CONTEXT=1 is set.
Lets users escape-hatch if it breaks them.
There was a problem hiding this comment.
Users could already set DOCKER_CONTEXT=default as an escape hatch right? So probably not worth adding another one.
There was a problem hiding this comment.
I agree this is a good default though.
Maybe we should just release this as next major version to properly signal this.
Summary of changes:
for APIClient.
All 14 new/adjusted tests pass; the remaining pytest tests/unit/ failures are pre-existing on main (verified by stashing and re-running). The behavior matches the Docker CLI: DOCKER_HOST wins, then DOCKER_CONTEXT, then currentContext, then default.