Skip to content

Always initialize context store - #1690

Merged
thaJeztah merged 3 commits into
docker:masterfrom
jcsirot:fix-contextstore-for-plugins
Mar 18, 2019
Merged

Always initialize context store#1690
thaJeztah merged 3 commits into
docker:masterfrom
jcsirot:fix-contextstore-for-plugins

Conversation

@jcsirot

Copy link
Copy Markdown
Contributor

- What I did
Always initialize context store even when the CLI has already a client. It happens when the CLI is initialized from a plugin.

- A picture of a cute animal (not mandatory but encouraged)

image

Comment threadcli/command/cli.go Outdated
@codecov-io

codecov-io commented Feb 25, 2019

Copy link
Copy Markdown

Codecov Report

Merging #1690 into master will increase coverage by 0.05%.
The diff coverage is 27.27%.

@@ Coverage Diff @@## master #1690 +/- ##
==========================================
+ Coverage 56.13% 56.19% +0.05% 
==========================================
Files 306 306 Lines 21038 21037 -1 ==========================================
+ Hits 11809 11821 +12 + Misses 8374 8357 -17 - Partials 855 859 +4

ulyssessouza pushed a commit to docker-archive-public/docker.app that referenced this pull request Mar 7, 2019
- Update docker/cli is now pointing to ulyssessouza/cli.
The change needs the merge of docker/cli#1710
and docker/cli#1690
- Fix issues relative paths in Jenkinsfile and Jenkinsfile.baguette
- Avoid using '--config' in favor of env variable 'DOCKER_CONFIG'
Signed-off-by: Ulysses Souza <ulysses.souza@docker.com>
chris-crone pushed a commit to chris-crone/app that referenced this pull request Mar 7, 2019
- Update docker/cli is now pointing to ulyssessouza/cli.
The change needs the merge of docker/cli#1710
and docker/cli#1690
- Fix issues relative paths in Jenkinsfile and Jenkinsfile.baguette
- Avoid using '--config' in favor of env variable 'DOCKER_CONFIG'
Signed-off-by: Ulysses Souza <ulysses.souza@docker.com>
chris-crone pushed a commit to chris-crone/app that referenced this pull request Mar 8, 2019
- Update docker/cli is now pointing to chris-crone/cli.
The change needs the merge of docker/cli#1718
and docker/cli#1690
- Fix issues relative paths in Jenkinsfile and Jenkinsfile.baguette
- Avoid using '--config' in favor of env variable 'DOCKER_CONFIG'
Signed-off-by: Ulysses Souza <ulysses.souza@docker.com>
chris-crone pushed a commit to chris-crone/app that referenced this pull request Mar 8, 2019
- Update docker/cli is now pointing to chris-crone/cli.
The change needs the merge of docker/cli#1718
and docker/cli#1690
- Fix issues relative paths in Jenkinsfile and Jenkinsfile.baguette
- Avoid using '--config' in favor of env variable 'DOCKER_CONFIG'
Signed-off-by: Ulysses Souza <ulysses.souza@docker.com>
@simonferquel

Copy link
Copy Markdown
Contributor

Please rebase this one :)

Jean-Christophe Sirot added 3 commits March 13, 2019 14:18
Signed-off-by: Jean-Christophe Sirot <jean-christophe.sirot@docker.com>
…t case the `TestDialStdio` e2e test had to be modified: the `--tls` option triggers an error since the endpoint resolution tries to read the `${DOCKER_CERT_PATH}/ca.pem` file which does not exist.
Signed-off-by: Jean-Christophe Sirot <jean-christophe.sirot@docker.com>
Signed-off-by: Jean-Christophe Sirot <jean-christophe.sirot@docker.com>
@jcsirot
jcsirotforce-pushed the fix-contextstore-for-plugins branch from ef04489 to a1af6e2CompareMarch 13, 2019 13:19

@silvin-lubeckisilvin-lubecki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@silvin-lubecki

Copy link
Copy Markdown
Contributor

@thaJeztah PTAL

@thaJeztahthaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Comment threadcli/command/cli.go

cli.configFile = cliconfig.LoadDefaultConfigFile(cli.err)

cli.contextStore = store.New(cliconfig.ContextStoreDir(), cli.contextStoreConfig)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not for this PR, but wondering if we should consider having a WithContextStore() ops

@thaJeztah
thaJeztah merged commit 86a5a48 into docker:masterMar 18, 2019
@GordonTheTurtleGordonTheTurtle added this to the 19.03.0 milestone Mar 18, 2019
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@jcsirot@codecov-io@simonferquel@silvin-lubecki@thaJeztah@ijc@GordonTheTurtle