feast icon indicating copy to clipboard operation
feast copied to clipboard

fix: Get container host addresses from testcontainers

Open tokoko opened this issue 1 year ago • 2 comments

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

tokoko avatar Feb 11 '24 17:02 tokoko

Looks like we are hitting some sort of rate limits on GCP

tokoko avatar Feb 12 '24 12:02 tokoko

@HaoXuAI Hi, can you take a look at this when you'll have some time?

tokoko avatar Feb 18 '24 14:02 tokoko

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!!

jeremyary avatar Feb 27 '24 20:02 jeremyary

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 avatar Mar 04 '24 17:03 jeremyary

sorry just saw this.

https://github.com/testcontainers/testcontainers-python/pull/61/files looks like default to localhost, so should be ok

HaoXuAI avatar Mar 05 '24 21:03 HaoXuAI

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 avatar Mar 06 '24 03:03 tokoko

@tokoko gotcha, thanks for sharing details!

jeremyary avatar Mar 06 '24 13:03 jeremyary