fix docker build output parser

[linuxfreakus]

Docker must be outputting something different than whatever docker was outputting when this tool was last updated. I was getting some output that had multiple json objects which resulted in ExtraData errors.

This fixes those errors and also upgrades the docker-py dependency to the newer docker 2.1.0 which meant changing how the util was creating its client. It seems to work fine otherwise and the tests all pass for me.

[jizhilong]

@linuxfreakus thanks for the PR.What's the version of docker you are using?Mine is 1.11.1, and never encountered ExtraData errors。

[linuxfreakus]

I'm using docker engine version 1.13.1 (latest release)

[jizhilong]

I did some tests on some of my private projects with docker 1.13.1, but no ExtraData errors were throwed.Could please provide your Dockerfiles and scripts to reproduce this issue?

BTW, upgrading docker-py may impact a lot, so if its version is not necessarily related to this issue, it's not a good idea to upgrade it here.

[linuxfreakus]

It was happening for me with every docker file that I tried... I'm using Docker for Mac too if that matters, though I doubt it would because it still is running the docker engine, just in a VM. I'm not at my laptop at the moment, but I'll grab one of my Dockerfiles a little later.

…

On Mon, Feb 20, 2017 at 12:50 AM, jzl ***@***.***> wrote: I did some tests on some of my private projects with docker 1.13.1, but no ExtraData errors were throwed.Could please provide your Dockerfiles and scripts to reproduce this issue? BTW, upgrading docker-py may impact a lot, so if its version is not necessarily related to this issue, it's not a good idea to upgrade it here. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABYplZpfYfrzUroY4V4X_9I5t75F-FgOks5reUWxgaJpZM4MEyFv> .

[linuxfreakus]

Here is a stripped down test of a couple images... I took out anything proprietary, it still gives me the errors if I use the unpatched code. Upgrading docker-py isn't strictly necessary, but I've been running into conflicts since we have followed the upgrades to that library here and the longer it goes without changing it, there will probably be more modules that have pulled in the newer version (I could work around it by either using a virtualenv or still having a patch but only for the lib version). It would be great if they'd have named the modules to match so that they could be used in parallel, but they didn't.

…

--Eric On Tue, Feb 21, 2017 at 7:23 AM, Eric Nichols <linuxfreakus@gmail.com> wrote:

It was happening for me with every docker file that I tried... I'm using Docker for Mac too if that matters, though I doubt it would because it still is running the docker engine, just in a VM. I'm not at my laptop at the moment, but I'll grab one of my Dockerfiles a little later. On Mon, Feb 20, 2017 at 12:50 AM, jzl ***@***.***> wrote: > I did some tests on some of my private projects with docker 1.13.1, but > no ExtraData errors were throwed.Could please provide your Dockerfiles > and scripts to reproduce this issue? > > BTW, upgrading docker-py may impact a lot, so if its version is not > necessarily related to this issue, it's not a good idea to upgrade it here. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#8 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/ABYplZpfYfrzUroY4V4X_9I5t75F-FgOks5reUWxgaJpZM4MEyFv> > . >

[linuxfreakus]

File didn't attach from email I guess? Here is is again it looks like github has a different way to attach files.

dmake-test.tar.gz

[jizhilong]

Hi @linuxfreakus , all goes well on my linux box with Dockerfiles you provided, I'll test again on my mac book later.

[jizhilong]

Hi @linuxfreakus, The issue was reproduced on my mac, and your patch to build.py succeeded to fix it.

But upgrading dependency on docker-py impacts too much, it will break any running docker-make installations.I'd suggest you remove the dependency part from this PR, and do it more gracefully in another one.

[linuxfreakus]

OK, your call on the dependency change. I can pull that out, although I think the impact specifically on existing docker-make installations would be limited to what happens if they upgrade since whatever they've installed (and potentially used pip freeze on) should be stable.

The impact that I can see is that docker-py would still be installed, then it would also install docker... this could be mitigated by having a try block to attempt one and fallback to the other depending on which one got imported... ultimately users should want to uninstall docker-py however since it will no longer receive changes and updates.

I'll update this PR to remove the dependency change and continue to use my patched fork since I would rather use the API version that is not at end of life.

[linuxfreakus]

And great work on this module BTW! I really can't understand why docker's own CLI doesn't already do this stuff since everyone pretty much has to do this by hand if not automated!

[linuxfreakus]

I potentially have another PR where I also integrate this module with docker-squash so that complicated hierarchies can be dealt with to keep the number of layers under control but I'm still playing with that... probably won't send that one until I'm happy with how it is working.

[jizhilong]

Great!Thanks for the PR again, I'll merge it once you have removed the dependency part.

And your idea of integrating docker-squash is very impressive, I am looking forward for your patches.

I opened a issue docker/docker-py#1476 on docker-py asking for a way to improve docker-make's dependency list. They gave me an idea, which, thanks to the great dynamics of Python, is determining the dependency on docker-py or docker(docker-py 2.x) in setup.py, instead of writing them as fixed dependency specifier in requirements.txt.

[linuxfreakus]

Hmm.... so it turns out that this still doesn't fix the problem, I have another Dockerfile that compiles some stuff from source and somehow the output that it spits out can cause the splitlines() based parsing to still break down.

Further investigation reveals that this has been an ongoing problem and python's json module does not make it easy because it exposes a lot less capability than one might like for handling edge cases.

docker/docker-py#1059

They ostensibly have a fix as of version 2.0.2 where you can pass decode=True to the build function and it does the parsing for you using some helpers that they wrote, but to do that we really would need to upgrade from docker-py to docker.

I'm going to do some more tinkering on my side since it looks like this PR, although helpful for some outputs, still does not solve the issue for all cases.

[jizhilong]

Interesting, I didn't expect this issue would be that complicated.So it's inevitable to do a non-back-compatible upgrade now.And looking forward to your patch!

[jizhilong]

Hi, @linuxfreakus I'll merge your PR now.But since the change to the dependency part may break upgradings, I'll improve that part later