search

lmco / laikaboss

Laika BOSS: Object Scanning System
Apache License 2.0
732 stars 156 forks source link

Fix laika.py runtime error due to older pefile included in Dockerfile #21

Closed jjs357 closed 8 years ago

jjs357 commented 8 years ago

Saw a closed issue related to a pefile induced runtime error when running laika.py. The included change to the Dockerfile updates pefile and when I run laikaboss now in a container created using the change, the error is no longer there.

wzod commented 8 years ago

Nice catch!

To make things easy, I've created a pull request (#22) to remove the install of python-pefile from apt repo and simply use pip package manager (since that version of pefile does not have the issue).

I've also updated the Dockerfile on Dockerhub so the issue should be resolved with future Docker pulls.

Hope this helps!

jjs357 commented 8 years ago

Sounds good to me!

wzod commented 8 years ago

Push to Dockerhub is complete.

Thanks again!

jjs357 commented 8 years ago

But I just tried to use the command

docker pull wzod/laikaboss

to get the latest container but when I run this container and check what pefile I have (with pip show pefile) I am told I have 1.2.9.1. This version still has the bug in it. Laikaboss needs version: 1.2.10-114.

What happened? The manual upgrade method I used which did not remove mention of pefile from the apt-get section and had the upgrade option specifically in it works. I need to do a docker build with a local copy of the Laikaboss docker file still to get the error message go away.

I rescinded my own pull request thinking your was going to fix things.

Am I doing anything wrong?

Jim

On Oct 22, 2015, at 1:02 AM, Zod notifications@github.com wrote:

Nice catch!

To make things easy, I've created a pull request (#22 https://github.com/lmco/laikaboss/pull/22) to remove the install of python-pefile from apt repo and simply use pip package manager (since that version of pefile does not have the issue).

I've also updated the Dockerfile on Dockerhub so the issue should be resolved with future Docker pulls.

Hope this helps!

— Reply to this email directly or view it on GitHub https://github.com/lmco/laikaboss/pull/21#issuecomment-150106561.

wzod commented 8 years ago

The push just completed 4 minutes ago; please try again. I just tested on my end and no longer see the issue with pefile.

jjs357 commented 8 years ago

Yes — just tried again and now it works.

I guess I was premature in trying out the updated dockerfile.

Thanks.

Jim

On Oct 22, 2015, at 1:37 AM, Zod notifications@github.com wrote:

The push just completed 4 minutes ago; please try again. I just tested on my end and no longer see the issue with pefile.

— Reply to this email directly or view it on GitHub https://github.com/lmco/laikaboss/pull/21#issuecomment-150112649.

wzod commented 8 years ago

Excellent!

Thanks again for pointing out the issue with pefile!

azollman commented 8 years ago

Jim / Zod - Thanks for finding & fixing this issue!