-
Notifications
You must be signed in to change notification settings - Fork 156
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Multiple docker sockets support #303
Multiple docker sockets support #303
Conversation
plugin/loader.go
Outdated
wrappedClient := docker.WrapClient(dockerClient) | ||
|
||
dockerClients = append(dockerClients, wrappedClient) | ||
skipEvents = append(skipEvents, true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that the old code initialized skipEvents as false instead of true.
Are you intentionally changing this?
Should we change this line to false?
@amaelFr code looks good and integration tests are passing |
plugin/loader.go
Outdated
wrappedClient := docker.WrapClient(dockerClient) | ||
|
||
dockerClients = append(dockerClients, wrappedClient) | ||
skipEvents = append(skipEvents, true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing here, should it be false instead of true?
After several searches, it appears to be an error in my code, but it is mitigated by calling the update function further down in the code. The two instance of `append' could be replace by a line of code like dockerLoader.skipEvents = make([]bool, len(dockerLoader.dockerClients)) Do you want me to update the code ? |
Yes, sure. The one line instead of 2 appends seems better. |
I have updated the part of the code concerned by your review. |
I tried to add an implementation for multiple docker sockets, related to the problem I opened on the topic.
I used your implementation and only changed the docker client to an array (and associated variables like jump events).
Could you please take a look at it, if it meets your principles?
Thanks