@marcust Thanks for your effort. However, the problem of backward-compatibility with older docker remote API versions is a bit more complex. Although your patch will solve the concrete problem with #288 it doesn't solve problems with changes in the JSON formats that are used in the docker API.

Thats why I've decided to drop backward-compatibility completey as it would exceed my spare time to maintain such a beast. This would require much more effort for testing for example.

Hey @marcuslinke, I get your reasoning. But I think from time to time it might be necessary to be "somewhat" backward compatible, meaning that for a period of time it might be useful to support an older version as well as the current version. I don't aim at supporting every version of the API ever, but #288 is a case where it is really easy to allow backward compatibility.

From my use-case point of view I have 20+ devs and a CI setup which is not realistically just updated to a newer docker version. Furthermore some distros come with older versions (Jessie comes with 1.6.2) and I think we should ease the pain of the clients and users.

In my opinion moby/moby#14092 was an unintended regression that got fixed in the clients after it was clear that the daemon is not going to support both variants.

I wrote this patch because of alexec/docker-java-orchestration#49 which currently fails because Circle CI comes with docker 1.6.2 and thus started failing tests when migrated to docker-java 2.x

@marcuslinke probably if @marcust wants support this issues, then he can help with this changes

But i fear that without objects it wouldn't help.

@marcust docker API sometimes changes field types from String to ints for example.

@KostyaSha I will happily revert/remove the commit when I don't need that anymore

I will provide a class descriptions if there is a chance to get it merged, up to now @marcuslinke doesn't seem to be a huge fan of this PR

My problem with this patch is that there is currently only one known issue where it would be useful (namely #288). It seems to be too much effort for this single aspect of backward-compatibility. @KostyaSha WDYT?

Keeping compatibility is of course good, but requires resources for testing/impelementing. Docker API breaks it all time i don't think that it will be possible to follow all their changes :(

I think 1.6 people should upgrade 🐬 to 1.7, 1.8. And soon will appear 1.9 with volumes 🎉
They are faster then java :D

I'm currently thinking of directly passing DockerClientConfig instead of only RemoteApiVersion into the constructors of the various DockerCmdExec implementations.

This would allow to implement the fix for #288 and additionally it would allow to avoid the awkward propagation of AuthConfigurations from the config over the command into the exec. (starting with https://github.com/docker-java/docker-java/blob/master/src/main/java/com/github/dockerjava/core/DockerClientImpl.java#L350-L357)

@marcust Would it be possible for you to adopt your PR accordingly? After that it would make sense to merge as it would provide a real benefit. Thanks!

@marcuslinke I adapted the PR.

I left the BuildImageCmd.withBuildAuthConfigs intact, because on the one hand there is a test (BuildImageCmdImpltest.testBuildFromPrivateRegistry) which quite explicitly tests this behavior and Ifelt that being API compatible may be a good thing as well. The value configured on the Cmd takes precedence to the auth config from the configuration.

@marcust Good point. I will try to merge ASAP. Thanks!

@marcuslinke Can we have a release with that?

@marcust Sorry for being late. Just released v.2.1.2. Should be available from maven central in a few hours.