Skip to content
Merged
Changes from 1 commit
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
136 changes: 135 additions & 1 deletion python_on_whales/components/container/cli_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@
Optional,
Sequence,
Tuple,
TypedDict,
Union,
overload,
)

import pydantic
from typing_extensions import TypeAlias
from typing_extensions import TypeAlias, Unpack
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the original PR this was conditionally imported from typing if the Python version was new enough, but typing_extension does the same already 😊

Copy link

@Dreamsorcerer Dreamsorcerer Sep 26, 2025

Choose a reason for hiding this comment

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

The idea of the version check was to be able to remove typing-extensions as a dependency in versions of Python that don't need it (e.g. https://github.com/aio-libs/multidict/blob/18344e20b7a0c883b48de685174a78b8b33663e7/setup.cfg#L45), plus allow tools like pyupgrade/ruff to automatically remove the old imports when the project stops supporting a particular version of Python.

but typing_extension does the same already

Obviously typing-extensions will have all of these things already, given that's it's sole purpose it to backport typing features to older versions of Python.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Dreamsorcerer maybe we can follow up with another PR? since there's already typing_extensions installed?

Choose a reason for hiding this comment

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

Yes, it's not up to me, just providing some feedback that could be adopted by the project.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah I'd been going with the approach of being explicit about the versions where typing_extensions is actually needed, but I realise it does add clutter and overhead. I can see the argument both ways, but when in doubt going with project consistency is generally a safe bet.

Choose a reason for hiding this comment

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

I'm not sure anyone is complaining about having typing_extensions as a dependency for recent python versions. If someone opens an issue and express a use case where this is annoying we'll take a look at it. Let's not fix a problem that's not bothering anyone. Importing typing_extensions unconditionally is fine for now.


import python_on_whales.components.image.cli_wrapper
import python_on_whales.components.network.cli_wrapper
Expand Down Expand Up @@ -443,6 +444,106 @@ def stop(self, time: Union[int, timedelta] = None) -> None:
ValidContainer = Union[Container, str]


class RunArgs(TypedDict, total=False):
add_hosts: Iterable[Tuple[str, str]]
blkio_weight: Optional[int]
blkio_weight_device: Iterable[str]
cap_add: Iterable[str]
cap_drop: Iterable[str]
cgroup_parent: Optional[str]
cgroupns: Optional[str]
cidfile: Optional[ValidPath]
cpu_period: Optional[int]
cpu_quota: Optional[int]
cpu_rt_period: Optional[int]
cpu_rt_runtime: Optional[int]
cpu_shares: Optional[int]
cpus: Optional[float]
cpuset_cpus: Optional[List[int]]
cpuset_mems: Optional[List[int]]
devices: Iterable[str]
device_cgroup_rules: Iterable[str]
device_read_bps: Iterable[str]
device_read_iops: Iterable[str]
device_write_bps: Iterable[str]
device_write_iops: Iterable[str]
content_trust: bool
dns: Iterable[str]
dns_options: Iterable[str]
dns_search: Iterable[str]
domainname: Optional[str]
entrypoint: Optional[str]
envs: Mapping[str, str]
env_files: Union[ValidPath, Iterable[ValidPath]]
env_host: bool
expose: Union[int, Iterable[int]]
gpus: Union[int, str, None]
groups_add: Iterable[str]
healthcheck: bool
health_cmd: Optional[str]
health_interval: Union[None, int, timedelta]
health_retries: Optional[int]
health_start_period: Union[None, int, timedelta]
health_timeout: Union[None, int, timedelta]
hostname: Optional[str]
init: bool
interactive: bool
ip: Optional[str]
ip6: Optional[str]
ipc: Optional[str]
isolation: Optional[str]
kernel_memory: Union[int, str, None]
labels: Mapping[str, str]
label_files: Iterable[ValidPath]
link: Iterable[ValidContainer]
link_local_ip: Iterable[str]
log_driver: Optional[str]
log_options: Iterable[str]
mac_address: Optional[str]
memory: Union[int, str, None]
memory_reservation: Union[int, str, None]
memory_swap: Union[int, str, None]
memory_swappiness: Optional[int]
mounts: Iterable[List[str]]
name: Optional[str]
networks: Iterable[python_on_whales.components.network.cli_wrapper.ValidNetwork]
network_aliases: Iterable[str]
oom_kill: bool
oom_score_adj: Optional[int]
pid: Optional[str]
pids_limit: Optional[int]
platform: Optional[str]
pod: Optional[python_on_whales.components.pod.cli_wrapper.ValidPod]
preserve_fds: Optional[int]
privileged: bool
publish: Iterable[ValidPortMapping]
publish_all: bool
pull: str
read_only: bool
restart: Optional[str]
remove: bool
runtime: Optional[str]
security_options: Iterable[str]
shm_size: Union[int, str, None]
sig_proxy: bool
stop_signal: Optional[Union[int, str]]
stop_timeout: Optional[int]
storage_options: Iterable[str]
sysctl: Mapping[str, str]
systemd: Optional[Union[bool, Literal["always"]]]
tmpfs: Iterable[ValidPath]
tty: bool
tz: Optional[str]
ulimit: Iterable[str]
user: Optional[str]
userns: Optional[str]
uts: Optional[str]
volumes: Iterable[python_on_whales.components.volume.cli_wrapper.VolumeDefinition]
volume_driver: Optional[str]
volumes_from: Iterable[ValidContainer]
workdir: Optional[ValidPath]


class ContainerCLI(DockerCLICaller):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
Expand Down Expand Up @@ -1358,6 +1459,39 @@ def remove(

run(full_cmd)

@overload
def run(
self,
image: python_on_whales.components.image.cli_wrapper.ValidImage,
command: Sequence[str] = ...,
*,
detach: Literal[True],
stream: bool = ...,
**kwargs: Unpack[RunArgs],
) -> Container: ...

@overload
def run(
self,
image: python_on_whales.components.image.cli_wrapper.ValidImage,
command: Sequence[str] = ...,
*,
detach: Literal[False] = ...,
stream: Literal[True],
**kwargs: Unpack[RunArgs],
) -> Iterable[Tuple[str, bytes]]: ...

@overload
def run(
self,
image: python_on_whales.components.image.cli_wrapper.ValidImage,
command: Sequence[str] = ...,
*,
detach: Literal[False] = ...,
stream: Literal[False] = ...,
**kwargs: Unpack[RunArgs],
) -> str: ...

def run(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason the non-overload method doesn't use **kwargs: Unpack[RunArgs]? If it doesn't there's still duplication and a need to update two places at once.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@LewisGaul yes, unfortunately it is needed for docs, I didn't explain it properly in my PR message, but basically since it's not possible to give default args to kwargs we can't really avoid the duplication

self,
image: python_on_whales.components.image.cli_wrapper.ValidImage,
Expand Down
Loading