Skip to content

Conversation

hustcat
Copy link
Contributor

@hustcat hustcat commented Nov 3, 2016

To fix skopeo #244.

Signed-off-by: Ye Yin eyniy@qq.com

@runcom
Copy link
Member

runcom commented Nov 3, 2016

we may want to support specific options somehow to be passed down to the daemon's client initialization here

@hustcat
Copy link
Contributor Author

hustcat commented Nov 3, 2016

@runcom Yeah, support options is better choice, when to support this?
Before it, I think configure docker daemon with environment variables is a choice.

Copy link
Collaborator

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

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

At the very least, the ImageSource implementation needs an equivalent change.

Though, in abstract, I’m not thrilled about a library changing its behavior using environment variables about which the caller has no idea and which the caller cannot prevent.

Could this be done by passing the needed information (and only the needed information—what is the use case?) through types.SystemContext (and perhaps by parsing environment variables in skopeo’s contextFromGlobalOptions) instead?

@mtrmac
Copy link
Collaborator

mtrmac commented Nov 3, 2016

Though, in abstract, I’m not thrilled about a library changing its behavior using environment variables about which the caller has no idea and which the caller cannot prevent.

(I do realize that there are various commonly expected behaviors where something like that is expected and needed, and atomic: does use various hidden environment variables like that; this again might be one of these cases. I do want to make sure that we are making a conscious decision.)

@runcom
Copy link
Member

runcom commented Nov 3, 2016

Could this be done by passing the needed information (and only the needed information—what is the use case?) through types.SystemContext (and perhaps by parsing environment variables in skopeo’s contextFromGlobalOptions instead?

@hustcat this is what I had in mind, starting with env variables isn't enough to me as a start.

giuseppe pushed a commit to giuseppe/image that referenced this pull request Jan 24, 2017
@hferentschik
Copy link
Contributor

At the very least, the ImageSource implementation needs an equivalent change.

+1

For what it's worth, this pull request together with a change to daemon_src.go is what I needed tp connect to a remote Docker daemon. Environment variables works for me, since I use containers/image as a library and can set the env settings in the calling code. However, I agree that is is not really explicit.

@hferentschik
Copy link
Contributor

Hi I created pull request #328 to supersede this pull request. The only difference is that it creates the Docker client from env variables for source and destination.

Maybe this is not the ultimate solution, but imo it would at least move things forward.

@rhatdan
Copy link
Member

rhatdan commented Jan 29, 2018

@mtrmac Please Close since other PR got merged.

@mtrmac
Copy link
Collaborator

mtrmac commented Jan 30, 2018

Yeah, #328 should allow making the same choices as this PR.

@mtrmac mtrmac closed this Jan 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docker daemon host need to be configurable
5 participants