Fix Docker hanging (fixes #519)

[gravelBridge]

Fixes #519

[hartwork]

Hi @gravelBridge, this seems to be about #519. The fix here may solve the hanging but does not seem to fix lack of integration with docker.transport.unixconn.UnixHTTPConnection. I think this pull request is mistaken.

[dschonholtz]

@hartwork Hey I'm one of the maintainers over at AutoGPT. This fixes our issue and we are using a Fork of vcrpy that has this change in it currently. https://github.com/Significant-Gravitas/vcrpy
We would really prefer to use the official package though.

Could you explain your concern above please?
I don't understand what you mean by: lack of integration with docker.transport.unixconn.UnixHTTPConnection
This works great for our use case.

[hartwork]

Hi @dschonholtz,

what I mean is that this pull requests fixes the 100% CPU use hanging symptom, but the call to the docker daemon is not intercepted —not recorded, not replayed — and as a result:

• the code does not create a cassette file,
• talks to the host docker daemon (which may even be dangerous depending on test code and host system state), and
• stops working when the docker daemon is not running.

That's pretty bad in my book.

If you want to see it yourself, try this for file test_foo.py and run pytest test_foo.py. It will create file 2.yml but there will be no 1.yml:

import docker
import vcr
import urllib

@vcr.use_cassette('1.yml')
def test_docker_images():
    env = docker.from_env()
    images = env.images()
    assert images

@vcr.use_cassette('2.yml')
def test_urllib():
    response = urllib.request.urlopen('http://www.iana.org/domains/reserved').read()
    assert b'Example domains' in response

Now that that's confirmed not to work, I will close the pull request as mistaken. If you see a misunderstanding here, please let me know and help me understand it, and we can always re-open. Thanks!

[Pwuts]

@hartwork could you point me to why this PR would cause the problem you indicate? I'm not familiar with this part of the VCRpy library code, but we have been missing out on patches for the past 2 months and now also v5.0.0 so I'd like to see if we can fix it (properly).

[hartwork]

@Pwuts please see my comment right above with details: #692 (comment)

[Pwuts]

I read it before commenting, and am just not sure why there are issues with docker-py specifically. Is it because Docker uses a custom request class? But then if only standard request classes are used, why is there a class instance check in the loop? And how would monkey-patching docker-py fix this issue?

[hartwork]

@Pwuts my impression is that docker-py uses a custom connection class UnixHTTPConnection implementing HTTP over a Unix file socket and so VCR.py doesn't not support it before someone adds specific support for it hijacking that kind of transport, which this pull request didn't do.