Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): Add support for working with env files #684

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Tranquility2
Copy link
Contributor

@Tranquility2 Tranquility2 commented Aug 15, 2024

Fix: #687

Users should not be required to load each env var manually if they have an env file.
Added support for loading a dot-env file.

Usage:

with DockerContainer("nginx:latest").with_env_file("my_env_file"):
    # We now have a container with the relevant env vars from the file

This is an implementation of docker run --env-file <file> ...

@Tranquility2 Tranquility2 changed the title feat(main): Add support to working with env files feat(core): Add support to working with env files Aug 15, 2024
Copy link

codecov bot commented Aug 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@8f1165d). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #684   +/-   ##
=======================================
  Coverage        ?   81.27%           
=======================================
  Files           ?       12           
  Lines           ?      614           
  Branches        ?       91           
=======================================
  Hits            ?      499           
  Misses          ?       88           
  Partials        ?       27           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Tranquility2 Tranquility2 changed the title feat(core): Add support to working with env files feat(core): Add support for working with env files Aug 16, 2024
@sasa-buklijas
Copy link

@Tranquility2

It is great, but not sure that it will be useful, to me at least.
I will explain my workflow, because maybe I am using testcontainers-python in non optimal way and somebody can correct me.

I start testcontainers-python via pytest and have 3 containers running: redis, postgres and my own FastAPI app.
I am accessing redis, postgres from FastAPI app and from pytest directly(for simulating rest of the system)
What I have found that different IP and ports are needed if my own FastAPI app(container) is accessing redis and postgres containers or if pytest is accessing redis and postgres container.
IP and port also changes if I am using Linux(docker engine) or OSX (Docker Desktop).

Do to all this I can not use one dot-env file.

If I am doing something wrong, please correct me.

@Tranquility2
Copy link
Contributor Author

Tranquility2 commented Aug 16, 2024

Let me try an sort this out :)

  1. Your workflow is fine, you need dynamic data that your are obtaining for previous runs, that's not the use case for env files.
  2. This PR relates to the first part of your question where you raised a very interesting point regarding using dotenv and sending current/some os.environ to the Container and thats where env file can be used. (You simply reflected something deeper in the question. maybe it was not intentional)
    For example, if I would have an Redis service with some config:
# Redis Bike Commpany Demo Application: Example .env file.
REDIS_URL=redis://localhost:6379/?decode_responses=True
BIKE_INDEX_NAME="idx:bikes"
STORE_INDEX_NAME="idx:stores"
REDIS_KEY_BASE="redisbikeco"
BIKE_KEY_BASE="redisbikeco:bike"
STORE_KEY_BASE="redisbikeco:store"
FLASK_ENV=development

We would now be able to load it without evoking with_env 7 times + keeping a single source of truth for the env config.

TL;DR your current solution is great as its dynamic, we also need to support static config from file (As this is the common case for quite a few services)

@sasa-buklijas
Copy link

@Tranquility2 OK, thanks for explanation.

Is there some way, one line, to get container internal IP address ?

Only solution that I have found is:

import docker
client = docker.from_env()

# redis
redis_container_id = redis.get_wrapped_container().id
redis_container = client.containers.get(redis_container_id)
redis_container_ip_address = redis_container.attrs['NetworkSettings']['Networks']['bridge']['IPAddress']

@Tranquility2
Copy link
Contributor Author

I assume you mean redis as the model in testcontainers, you can use this:

from testcontainers.redis import RedisContainer

with RedisContainer() as redis:
    redis_container_ip_address  = redis.get_docker_client().bridge_ip(redis._container.id)

(This should provide the same data as in the example you found)

@Tranquility2
Copy link
Contributor Author

Some more details (if you are interested)

Usually get_container_host_ip is good enough (it is based on client.api.base_url from the docker API)
but as you can see in the code,

# if inside_container() and not os.getenv("DOCKER_HOST") and not host.startswith("http://"):
some of the DinD code was disabled, but you can clearly see
self.get_docker_client().gateway_ip(self._container.id)
and self.get_docker_client().bridge_ip(self._container.id) are very useful for some cases.

@sasa-buklijas
Copy link

@Tranquility2 thanks

P.S.
from my experience on Linux(docker engine) I need bridge_ip if one container want to communicate with other.
On OSX(docker Desktop) I just replace localhost with host.docker.internal

@alexanderankin
Copy link
Collaborator

im not sure about this use case. the issue linked sets hostnames as variables. shouldn't those be constants in code for easy updating? like youre passing the same hostname in the same function (same module at least) to both set and pass to other containers. im not convinced.

@alexanderankin alexanderankin marked this pull request as draft August 17, 2024 19:19
@Tranquility2
Copy link
Contributor Author

I see this is confusing, let me open a new issue :)

@Tranquility2
Copy link
Contributor Author

Tranquility2 commented Aug 17, 2024

@alexanderankin I've updated the related issue to #687 (sorry for the mix up) just to be extra clear this is a docker feature, we are just adding support to do the same as docker run --env-file <file> ...

@Tranquility2 Tranquility2 marked this pull request as ready for review August 17, 2024 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Support static env config from file
3 participants