Pull out the docker client #215
Conversation
|
I'm trying to split up my commits since each of these will be easier to review individually and they are each a feature. |
rthallisey
force-pushed
the
breakup-docker-client
branch
from
711825d
to
e1a092b
Compare
|
Testing: Run the normal broker tests. |
Make a seperate directory for clients so they are not apart of the APB code. This will make the clients easy to initialize and create new ones.
rthallisey
force-pushed
the
breakup-docker-client
branch
from
e1a092b
to
d2ce427
Compare
There was a problem hiding this comment.
The docker specific stuff here makes sense to me, but I have some thoughts on bringing more stuff out of apb/client.go into the clients package that's related to the k8s commit. Full PR review: #222
| @@ -40,8 +39,6 @@ First cut might have to pass kubecfg from broker. FIRST SPRINT broker passes use | |||
| admin/admin | |||
| */ | |||
|
|
|||
| var DockerSocket = "unix:///var/run/docker.sock" | |||
| func NewClient(log *logging.Logger) (*Client, error) { | ||
| dockerClient, err := docker.NewClient(DockerSocket) | ||
| dockerClient, err := clients.Docker(log) |
| ) | ||
|
|
||
| func Docker(log *logging.Logger) (*docker.Client, error) { | ||
| dockerClient, err := docker.NewClient(DockerSocket) |
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return config, nil |
There was a problem hiding this comment.
💡 Ok so this file is from the previous PR. Finally understood what's going on. I find the dependent PRs much harder to review. I would've preferred a clients feature to do all of the cleanup at once.
|
|
||
| const ( | ||
| DockerSocket = "unix:///var/run/docker.sock" | ||
| ) |
|
Approving this PR as well. Again removing |
| logging "github.com/op/go-logging" | ||
| ) | ||
|
|
||
| func Docker(log *logging.Logger) (*docker.Client, error) { |
There was a problem hiding this comment.
Before Merge, I think this needs doc comments.
|
Working in #222 |
Take out the docker client creation from the apbs package and put it along side the kubernetes client.
Partially-fixes: #213
Depends-on: #214