feast
feast copied to clipboard
fix: Get container host addresses from testcontainers
What this PR does / why we need it:
Testing container hosts are hardcoded to either localhost, which makes the life for people who develop inside containers hard. This PR instead gets host from testcontainer's get_container_host_ip
. This also doesn't always get it right, but at least lets the developer override the values with some env vars. For example in my case (vscode running inside Docker for Windows WSL container with docker socket mounted) this config seems to get tests working:
volumes:
- //var/run/docker.sock:/var/run/docker.sock
environment:
TC_HOST: host.docker.internal
DOCKER_HOST: unix:///var/run/docker.sock
Looks like we are hitting some sort of rate limits on GCP
@HaoXuAI Hi, can you take a look at this when you'll have some time?
heya @HaoXuAI, please give me a shout if this can't fit on your plate - not rushing at all, just want to make sure we're covered or if I can offer help or anything else. Thanks!!
This also doesn't always get it right
@tokoko can you share a bit more on suspected failure scenarios? Would it be a matter of not resolving and we can default out to localhost? Or more complex due to pulling incorrect value(s)?
sorry just saw this.
https://github.com/testcontainers/testcontainers-python/pull/61/files looks like default to localhost
, so should be ok
This also doesn't always get it right
@tokoko can you share a bit more on suspected failure scenarios? Would it be a matter of not resolving and we can default out to localhost? Or more complex due to pulling incorrect value(s)?
@jeremyary It always defaults to localhost in non-docker scenarios, so that shouldn't be a problem. I haven't really investigated all scenarios, the one where it got confused was mine actually, with both tests and testcontainers running inside docker, but not in the same docker network. It resolved host to testcontainer's internal IP which wasn't routable from my container. That's why I had to override it using TC_HOST
.
@tokoko gotcha, thanks for sharing details!