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

fix: typing in core #504

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -22,13 +22,13 @@ repos:
# Explicitly setting config to prevent Ruff from using `pyproject.toml` in sub packages.
args: [ '--fix', '--exit-non-zero-on-fix', '--config', 'pyproject.toml' ]

# - repo: local
# hooks:
# - id: mypy
# name: mypy
# entry: poetry run mypy
# args: ["--config-file", "pyproject.toml"]
# files: "core" # start with the core being type checked
# language: system
# types: [ python ]
# require_serial: true
- repo: local
hooks:
- id: mypy
name: mypy
entry: poetry run mypy
args: ["--config-file", "pyproject.toml"]
files: "core" # start with the core being type checked
language: system
types: [ python ]
require_serial: true
8 changes: 8 additions & 0 deletions core/testcontainers/compose/__init__.py
Original file line number Diff line number Diff line change
@@ -6,3 +6,11 @@
ComposeContainer,
DockerCompose,
)

__all__ = [
"ContainerIsNotRunning",
"NoSuchPortExposed",
"PublishedPort",
"ComposeContainer",
"DockerCompose",
]
57 changes: 35 additions & 22 deletions core/testcontainers/compose/compose.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
from dataclasses import dataclass, field, fields
from dataclasses import Field, dataclass, field, fields
from functools import cached_property
from json import loads
from os import PathLike
from re import split
from subprocess import CompletedProcess
from subprocess import run as subprocess_run
from typing import Callable, Literal, Optional, TypeVar, Union
from typing import Any, Callable, ClassVar, Literal, Optional, Protocol, TypeVar, Union, cast
from urllib.error import HTTPError, URLError
from urllib.request import urlopen

@@ -15,14 +15,18 @@
_IPT = TypeVar("_IPT")


def _ignore_properties(cls: type[_IPT], dict_: any) -> _IPT:
class DataclassInstance(Protocol):
__dataclass_fields__: ClassVar[dict[str, Field[Any]]]


def _ignore_properties(cls: type[_IPT], dict_: Union[_IPT, dict[str, Any]]) -> _IPT:
"""omits extra fields like @JsonIgnoreProperties(ignoreUnknown = true)

https://gist.github.com/alexanderankin/2a4549ac03554a31bef6eaaf2eaf7fd5"""
if isinstance(dict_, cls):
return dict_
class_fields = {f.name for f in fields(cls)}
filtered = {k: v for k, v in dict_.items() if k in class_fields}
class_fields = {f.name for f in fields(cast(type[DataclassInstance], cls))}
filtered = {k: v for k, v in cast(dict[str, Any], dict_).items() if k in class_fields}
return cls(**filtered)


@@ -34,8 +38,8 @@ class PublishedPort:
"""

URL: Optional[str] = None
TargetPort: Optional[str] = None
PublishedPort: Optional[str] = None
TargetPort: Optional[int] = None
PublishedPort: Optional[int] = None
Protocol: Optional[str] = None


@@ -61,21 +65,21 @@ class ComposeContainer:
Name: Optional[str] = None
Command: Optional[str] = None
Project: Optional[str] = None
Service: Optional[str] = None
Service: str = ""
State: Optional[str] = None
Health: Optional[str] = None
ExitCode: Optional[str] = None
ExitCode: Optional[int] = None
Publishers: list[PublishedPort] = field(default_factory=list)

def __post_init__(self):
def __post_init__(self) -> None:
if self.Publishers:
self.Publishers = [_ignore_properties(PublishedPort, p) for p in self.Publishers]

def get_publisher(
self,
by_port: Optional[int] = None,
by_host: Optional[str] = None,
prefer_ip_version: Literal["IPV4", "IPv6"] = "IPv4",
prefer_ip_version: Literal["IPv4", "IPv6"] = "IPv4",
) -> PublishedPort:
remaining_publishers = self.Publishers

@@ -98,8 +102,8 @@ def get_publisher(
)

@staticmethod
def _matches_protocol(prefer_ip_version, r):
return (":" in r.URL) is (prefer_ip_version == "IPv6")
def _matches_protocol(prefer_ip_version: str, r: PublishedPort) -> bool:
return (":" in (r.URL or "")) is (prefer_ip_version == "IPv6")


@dataclass
@@ -151,23 +155,23 @@ class DockerCompose:
image: "hello-world"
"""

context: Union[str, PathLike]
context: Union[str, PathLike[Any]]
compose_file_name: Optional[Union[str, list[str]]] = None
pull: bool = False
build: bool = False
wait: bool = True
env_file: Optional[str] = None
services: Optional[list[str]] = None

def __post_init__(self):
def __post_init__(self) -> None:
if isinstance(self.compose_file_name, str):
self.compose_file_name = [self.compose_file_name]

def __enter__(self) -> "DockerCompose":
self.start()
return self

def __exit__(self, exc_type, exc_val, exc_tb) -> None:
def __exit__(self, exc_type: Any, exc_val: Any, exc_tb: Any) -> None:
self.stop()

def docker_compose_command(self) -> list[str]:
@@ -217,7 +221,7 @@ def start(self) -> None:

self._run_command(cmd=up_cmd)

def stop(self, down=True) -> None:
def stop(self, down: bool = True) -> None:
"""
Stops the docker compose environment.
"""
@@ -243,7 +247,7 @@ def get_logs(self, *services: str) -> tuple[str, str]:
result = self._run_command(cmd=logs_cmd)
return result.stdout.decode("utf-8"), result.stderr.decode("utf-8")

def get_containers(self, include_all=False) -> list[ComposeContainer]:
def get_containers(self, include_all: bool = False) -> list[ComposeContainer]:
"""
Fetch information about running containers via `docker compose ps --format json`.
Available only in V2 of compose.
@@ -326,7 +330,7 @@ def exec_in_container(
def _run_command(
self,
cmd: Union[str, list[str]],
context: Optional[str] = None,
context: Optional[Union[str, PathLike[Any]]] = None,
) -> CompletedProcess[bytes]:
context = context or self.context
return subprocess_run(
@@ -340,7 +344,7 @@ def get_service_port(
self,
service_name: Optional[str] = None,
port: Optional[int] = None,
):
) -> Optional[int]:
"""
Returns the mapped port for one of the services.

@@ -362,7 +366,7 @@ def get_service_host(
self,
service_name: Optional[str] = None,
port: Optional[int] = None,
):
) -> Optional[str]:
"""
Returns the host for one of the services.

@@ -384,7 +388,7 @@ def get_service_host_and_port(
self,
service_name: Optional[str] = None,
port: Optional[int] = None,
):
) -> tuple[Optional[str], Optional[int]]:
publisher = self.get_container(service_name).get_publisher(by_port=port)
return publisher.URL, publisher.PublishedPort

@@ -401,3 +405,12 @@ def wait_for(self, url: str) -> "DockerCompose":
with urlopen(url) as response:
response.read()
return self


__all__ = [
"ContainerIsNotRunning",
"NoSuchPortExposed",
"PublishedPort",
"ComposeContainer",
"DockerCompose",
]
Empty file.
61 changes: 38 additions & 23 deletions core/testcontainers/core/container.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from os import PathLike
from platform import system
from socket import socket
from typing import TYPE_CHECKING, Optional
from typing import TYPE_CHECKING, Any, Optional, TypedDict, Union

from testcontainers.core.config import RYUK_DISABLED, RYUK_DOCKER_SOCKET, RYUK_IMAGE, RYUK_PRIVILEGED
from testcontainers.core.docker_client import DockerClient
@@ -10,11 +11,16 @@
from testcontainers.core.waiting_utils import wait_container_is_ready, wait_for_logs

if TYPE_CHECKING:
from docker.models.containers import Container
from docker.models.containers import Container, ExecResult

logger = setup_logger(__name__)


class VolumeDict(TypedDict):
bind: str
mode: str


class DockerContainer:
"""
Basic container object to spin up Docker instances.
@@ -31,19 +37,26 @@ class DockerContainer:
def __init__(
self,
image: str,
docker_client_kw: Optional[dict] = None,
**kwargs,
docker_client_kw: Optional[dict[str, Any]] = None,
**kwargs: Any,
) -> None:
self.env = {}
self.ports = {}
self.volumes = {}
self.env: dict[str, str] = {}
self.ports: dict[int, Optional[int]] = {}
self.volumes: dict[str, VolumeDict] = {}
self.image = image
self._docker = DockerClient(**(docker_client_kw or {}))
self._container = None
self._command = None
self._name = None
self._container: Optional["Container"] = None
self._command: Union[str, list[str], None] = None
self._name: Optional[str] = None
self._kwargs = kwargs

@property
def _use_container(self) -> "Container":
"""todo fail fast with better error when it is None"""
container = self._container
assert container is not None
return container

def with_env(self, key: str, value: str) -> "DockerContainer":
self.env[key] = value
return self
@@ -57,7 +70,7 @@ def with_exposed_ports(self, *ports: int) -> "DockerContainer":
self.ports[port] = None
return self

def with_kwargs(self, **kwargs) -> "DockerContainer":
def with_kwargs(self, **kwargs: Any) -> "DockerContainer":
self._kwargs = kwargs
return self

@@ -66,7 +79,7 @@ def maybe_emulate_amd64(self) -> "DockerContainer":
return self.with_kwargs(platform="linux/amd64")
return self

def start(self):
def start(self) -> "DockerContainer":
if not RYUK_DISABLED and self.image != RYUK_IMAGE:
logger.debug("Creating Ryuk container")
Reaper.get_instance()
@@ -85,14 +98,14 @@ def start(self):
logger.info("Container started: %s", self._container.short_id)
return self

def stop(self, force=True, delete_volume=True) -> None:
self._container.remove(force=force, v=delete_volume)
def stop(self, force: bool = True, delete_volume: bool = True) -> None:
self._use_container.remove(force=force, v=delete_volume)
self.get_docker_client().client.close()

def __enter__(self):
def __enter__(self) -> "DockerContainer":
return self.start()

def __exit__(self, exc_type, exc_val, exc_tb) -> None:
def __exit__(self, exc_type: Any, exc_val: Any, exc_tb: Any) -> None:
self.stop()

def get_container_host_ip(self) -> str:
@@ -120,13 +133,13 @@ def get_container_host_ip(self) -> str:

@wait_container_is_ready()
def get_exposed_port(self, port: int) -> str:
mapped_port = self.get_docker_client().port(self._container.id, port)
mapped_port = self.get_docker_client().port(self._use_container.id, port)
if inside_container():
gateway_ip = self.get_docker_client().gateway_ip(self._container.id)
gateway_ip = self.get_docker_client().gateway_ip(self._use_container.id)
host = self.get_docker_client().host()

if gateway_ip == host:
return port
return str(port)
return mapped_port

def with_command(self, command: str) -> "DockerContainer":
@@ -137,9 +150,11 @@ def with_name(self, name: str) -> "DockerContainer":
self._name = name
return self

def with_volume_mapping(self, host: str, container: str, mode: str = "ro") -> "DockerContainer":
mapping = {"bind": container, "mode": mode}
self.volumes[host] = mapping
def with_volume_mapping(
self, host: Union[str, PathLike[Any]], container: str, mode: str = "ro"
) -> "DockerContainer":
mapping: VolumeDict = {"bind": container, "mode": mode}
self.volumes[str(host)] = mapping
return self

def get_wrapped_container(self) -> "Container":
@@ -153,7 +168,7 @@ def get_logs(self) -> tuple[bytes, bytes]:
raise ContainerStartException("Container should be started before getting logs")
return self._container.logs(stderr=False), self._container.logs(stdout=False)

def exec(self, command) -> tuple[int, str]:
def exec(self, command: Union[str, list[str]]) -> "ExecResult":
if not self._container:
raise ContainerStartException("Container should be started before executing a command")
return self._container.exec_run(command)
32 changes: 16 additions & 16 deletions core/testcontainers/core/docker_client.py
Original file line number Diff line number Diff line change
@@ -17,7 +17,7 @@
import urllib.parse
from os.path import exists
from pathlib import Path
from typing import Optional, Union
from typing import Any, Optional, Union, cast

import docker
from docker.models.containers import Container, ContainerCollection
@@ -35,7 +35,7 @@ class DockerClient:
Thin wrapper around :class:`docker.DockerClient` for a more functional interface.
"""

def __init__(self, **kwargs) -> None:
def __init__(self, **kwargs: Any) -> None:
docker_host = get_docker_host()

if docker_host:
@@ -50,14 +50,14 @@ def run(
self,
image: str,
command: Optional[Union[str, list[str]]] = None,
environment: Optional[dict] = None,
ports: Optional[dict] = None,
environment: Optional[dict[str, str]] = None,
ports: Optional[dict[int, Optional[int]]] = None,
labels: Optional[dict[str, str]] = None,
detach: bool = False,
stdout: bool = True,
stderr: bool = False,
remove: bool = False,
**kwargs,
**kwargs: Any,
) -> Container:
# If the user has specified a network, we'll assume the user knows best
if "network" not in kwargs and not get_docker_host():
@@ -98,36 +98,36 @@ def find_host_network(self) -> Optional[str]:
except ipaddress.AddressValueError:
continue
if docker_host in subnet:
return network.name
return cast(str, network.name)
except ipaddress.AddressValueError:
pass
return None

def port(self, container_id: str, port: int) -> int:
def port(self, container_id: str, port: int) -> str:
"""
Lookup the public-facing port that is NAT-ed to :code:`port`.
"""
port_mappings = self.client.api.port(container_id, port)
if not port_mappings:
raise ConnectionError(f"Port mapping for container {container_id} and port {port} is " "not available")
return port_mappings[0]["HostPort"]
return cast(str, port_mappings[0]["HostPort"])

def get_container(self, container_id: str) -> Container:
def get_container(self, container_id: str) -> dict[str, Any]:
"""
Get the container with a given identifier.
"""
containers = self.client.api.containers(filters={"id": container_id})
if not containers:
raise RuntimeError(f"Could not get container with id {container_id}")
return containers[0]
return cast(dict[str, Any], containers[0])

def bridge_ip(self, container_id: str) -> str:
"""
Get the bridge ip address for a container.
"""
container = self.get_container(container_id)
network_name = self.network_name(container_id)
return container["NetworkSettings"]["Networks"][network_name]["IPAddress"]
return cast(str, container["NetworkSettings"]["Networks"][network_name]["IPAddress"])

def network_name(self, container_id: str) -> str:
"""
@@ -137,17 +137,17 @@ def network_name(self, container_id: str) -> str:
name = container["HostConfig"]["NetworkMode"]
if name == "default":
return "bridge"
return name
return cast(str, name)

def gateway_ip(self, container_id: str) -> str:
"""
Get the gateway ip address for a container.
"""
container = self.get_container(container_id)
network_name = self.network_name(container_id)
return container["NetworkSettings"]["Networks"][network_name]["Gateway"]
return cast(str, container["NetworkSettings"]["Networks"][network_name]["Gateway"])

def host(self) -> str:
def host(self) -> Optional[str]:
"""
Get the hostname or ip address of the docker host.
"""
@@ -162,7 +162,7 @@ def host(self) -> str:
except ValueError:
return None
if "http" in url.scheme or "tcp" in url.scheme:
return url.hostname
return cast(str, url.hostname)
if inside_container() and ("unix" in url.scheme or "npipe" in url.scheme):
ip_address = default_gateway_ip()
if ip_address:
@@ -181,7 +181,7 @@ def read_tc_properties() -> dict[str, str]:
tc_files = [item for item in [TC_GLOBAL] if exists(item)]
if not tc_files:
return {}
settings = {}
settings: dict[str, str] = {}

for file in tc_files:
tuples = []
4 changes: 2 additions & 2 deletions core/testcontainers/core/generic.py
Original file line number Diff line number Diff line change
@@ -10,7 +10,7 @@
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
from typing import Optional
from typing import Any, Optional

from testcontainers.core.container import DockerContainer
from testcontainers.core.exceptions import ContainerStartException
@@ -52,7 +52,7 @@ def _create_connection_url(
host: Optional[str] = None,
port: Optional[int] = None,
dbname: Optional[str] = None,
**kwargs,
**kwargs: Any,
) -> str:
if raise_for_deprecated_parameter(kwargs, "db_name", "dbname"):
raise ValueError(f"Unexpected arguments: {','.join(kwargs)}")
Empty file.
13 changes: 8 additions & 5 deletions core/testcontainers/core/utils.py
Original file line number Diff line number Diff line change
@@ -3,10 +3,13 @@
import platform
import subprocess
import sys
from contextlib import suppress
from typing import Any, Optional

LINUX = "linux"
MAC = "mac"
WIN = "win"
UNKNOWN = "unknown"


def setup_logger(name: str) -> logging.Logger:
@@ -26,6 +29,7 @@ def os_name() -> str:
return MAC
elif pl == "win32":
return WIN
return UNKNOWN


def is_mac() -> bool:
@@ -53,24 +57,23 @@ def inside_container() -> bool:
return os.path.exists("/.dockerenv")


def default_gateway_ip() -> str:
def default_gateway_ip() -> Optional[str]:
"""
Returns gateway IP address of the host that testcontainer process is
running on
https://github.com/testcontainers/testcontainers-java/blob/3ad8d80e2484864e554744a4800a81f6b7982168/core/src/main/java/org/testcontainers/dockerclient/DockerClientConfigUtils.java#L27
"""
cmd = ["sh", "-c", "ip route|awk '/default/ { print $3 }'"]
try:
with suppress(subprocess.SubprocessError):
process = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
ip_address = process.communicate()[0]
if ip_address and process.returncode == 0:
return ip_address.decode("utf-8").strip().strip("\n")
except subprocess.SubprocessError:
return None
return None


def raise_for_deprecated_parameter(kwargs: dict, name: str, replacement: str) -> dict:
def raise_for_deprecated_parameter(kwargs: dict[str, Any], name: str, replacement: str) -> dict[str, Any]:
"""
Raise an error if a dictionary of keyword arguments contains a key and suggest the replacement.
"""
13 changes: 8 additions & 5 deletions core/testcontainers/core/waiting_utils.py
Original file line number Diff line number Diff line change
@@ -31,7 +31,7 @@
TRANSIENT_EXCEPTIONS = (TimeoutError, ConnectionError)


def wait_container_is_ready(*transient_exceptions) -> Callable:
def wait_container_is_ready(*transient_exceptions: type[Exception]) -> Callable[..., Any]:
"""
Wait until container is ready.
@@ -44,8 +44,8 @@ def wait_container_is_ready(*transient_exceptions) -> Callable:
"""
transient_exceptions = TRANSIENT_EXCEPTIONS + tuple(transient_exceptions)

@wrapt.decorator
def wrapper(wrapped: Callable, instance: Any, args: list, kwargs: dict) -> Any:
@wrapt.decorator # type: ignore[misc]
def wrapper(wrapped: Callable[..., Any], instance: Any, args: list[Any], kwargs: dict[str, Any]) -> Any:
from testcontainers.core.container import DockerContainer

if isinstance(instance, DockerContainer):
@@ -69,7 +69,7 @@ def wrapper(wrapped: Callable, instance: Any, args: list, kwargs: dict) -> Any:
f"{kwargs}). Exception: {exception}"
)

return wrapper
return wrapper # type: ignore[no-any-return]


@wait_container_is_ready()
@@ -78,7 +78,10 @@ def wait_for(condition: Callable[..., bool]) -> bool:


def wait_for_logs(
container: "DockerContainer", predicate: Union[Callable, str], timeout: Optional[float] = None, interval: float = 1
container: "DockerContainer",
predicate: Union[Callable[..., Any], str],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the callable here be Callable[(str,) bool]? It's used as if predicate(stdout) or predicate(stderr):

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in java it also takes regex i dont remember if that is the case here but i will leave this thread open to review when and if this PR goes on. I think it belongs in a draft state for now. i just wanted to record the results of going through the exercise to add types. for example, some of the places we return ports are actually str | int (well, union, for 3.8 compatibility)

timeout: Optional[float] = None,
interval: float = 1,
) -> float:
"""
Wait for the container to emit logs satisfying the predicate.
9 changes: 5 additions & 4 deletions core/tests/test_compose.py
Original file line number Diff line number Diff line change
@@ -86,7 +86,7 @@ def test_compose_logs():
stdout, stderr = basic.get_logs()
container = basic.get_container()

assert not stderr
assert not stderr or "`version` is obsolete" in stderr
assert stdout
lines = split(r"\r?\n", stdout)

@@ -95,7 +95,7 @@ def test_compose_logs():
# either the line is blank or the first column (|-separated) contains the service name
# this is a safe way to split the string
# docker changes the prefix between versions 24 and 25
assert not line or container.Service in next(iter(line.split("|")), None)
assert not line or container.Service in next(iter(line.split("|")), ())


# noinspection HttpUrlsUsage
@@ -124,10 +124,11 @@ def test_compose_multiple_containers_and_ports():
e.match("get_container failed")
e.match("not exactly 1 container")

assert multiple.get_container("alpine")
assert multiple.get_container("alpine2")
assert multiple.get_container("alpine") is not None
assert multiple.get_container("alpine2") is not None

a2p = multiple.get_service_port("alpine2")
assert a2p is not None
assert a2p > 0 # > 1024

with pytest.raises(NoSuchPortExposed) as e:
3 changes: 2 additions & 1 deletion core/tests/test_docker_client.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from typing import Any
from unittest.mock import MagicMock, patch

import docker
@@ -7,7 +8,7 @@


def test_docker_client_from_env():
test_kwargs = {"test_kw": "test_value"}
test_kwargs: dict[str, Any] = {"test_kw": "test_value"}
mock_docker = MagicMock(spec=docker)
with patch("testcontainers.core.docker_client.docker", mock_docker):
DockerClient(**test_kwargs)
5 changes: 4 additions & 1 deletion core/tests/test_docker_in_docker.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import time
import socket

from docker.models.containers import Container

from testcontainers.core.container import DockerContainer
from testcontainers.core.docker_client import DockerClient
from testcontainers.core.waiting_utils import wait_for_logs


def _wait_for_dind_return_ip(client, dind):
def _wait_for_dind_return_ip(client: DockerClient, dind: Container):
# get ip address for DOCKER_HOST
# avoiding DockerContainer class here to prevent code changes affecting the test
docker_host_ip = client.bridge_ip(dind.id)
2 changes: 1 addition & 1 deletion core/tests/test_new_docker_api.py
Original file line number Diff line number Diff line change
@@ -21,7 +21,7 @@ def test_docker_kwargs():
container_second = DockerContainer("nginx:latest")

with container_first:
container_second.with_kwargs(volumes_from=[container_first._container.short_id])
container_second.with_kwargs(volumes_from=[container_first._use_container.short_id])
with container_second:
files_first = container_first.exec("ls /code").output.decode("utf-8").strip()
files_second = container_second.exec("ls /code").output.decode("utf-8").strip()