Skip to content

Commit

Permalink
Disambiguate shadowed builtins without API break
Browse files Browse the repository at this point in the history
This suppresses A003 for `list` builtin.

`typing.List`, `typing.Dict`, `typing.Tuple` and `typing.Type` are
deprecated. Removing these annotations breaks the calls to `list` when
they are done within the same class scope, which makes them ambiguous.

Typed returns `list` resolve to the function `list` defined in the class,
shadowing the builtin function. This change is not great but a proper
one would require changing the name of the class function `list` and
breaking the API to be fixed.

Example of where it breaks:

podman/domains/images_manager.py

class ImagesManager(...):

    def list(...):
        ...

    def pull(
        self,
        ...
        ) -> Image | list[Image], [[str]]:
        ...

Here, the typed annotation of `pull` would resolve to the `list` method,
rather than the builtin.

Signed-off-by: Nicola Sella <[email protected]>
  • Loading branch information
inknos committed Dec 11, 2024
1 parent 7a7d5d0 commit ca78098
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 17 deletions.
2 changes: 1 addition & 1 deletion podman/api/http_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,5 +100,5 @@ def _filter_values(mapping: Mapping[str, Any], recursion=False) -> dict[str, Any
return canonical


def encode_auth_header(auth_config: Dict[str, str]) -> str:
def encode_auth_header(auth_config: dict[str, str]) -> str:
return base64.urlsafe_b64encode(json.dumps(auth_config).encode('utf-8'))
10 changes: 6 additions & 4 deletions podman/domain/images_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@

logger = logging.getLogger("podman.images")

builtin_list = list


class ImagesManager(BuildMixin, Manager):
"""Specialized Manager for Image resources."""
Expand Down Expand Up @@ -300,7 +302,7 @@ def push(

@staticmethod
def _push_helper(
decode: bool, body: list[dict[str, Any]]
decode: bool, body: builtin_list[dict[str, Any]]
) -> Iterator[Union[str, dict[str, Any]]]:
"""Helper needed to allow push() to return either a generator or a str."""
for entry in body:
Expand All @@ -316,7 +318,7 @@ def pull(
tag: Optional[str] = None,
all_tags: bool = False,
**kwargs,
) -> Union[Image, list[Image], Iterator[str]]:
) -> Union[Image, builtin_list[Image], Iterator[str]]:
"""Request Podman service to pull image(s) from repository.
Args:
Expand Down Expand Up @@ -459,7 +461,7 @@ def remove(
image: Union[Image, str],
force: Optional[bool] = None,
noprune: bool = False, # pylint: disable=unused-argument
) -> list[dict[Literal["Deleted", "Untagged", "Errors", "ExitCode"], Union[str, int]]]:
) -> builtin_list[dict[Literal["Deleted", "Untagged", "Errors", "ExitCode"], Union[str, int]]]:
"""Delete image from Podman service.
Args:
Expand All @@ -486,7 +488,7 @@ def remove(
results.append({"ExitCode": body["ExitCode"]})
return results

def search(self, term: str, **kwargs) -> list[dict[str, Any]]:
def search(self, term: str, **kwargs) -> builtin_list[dict[str, Any]]:
"""Search Images on registries.
Args:
Expand Down
12 changes: 6 additions & 6 deletions podman/domain/networks.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class Network(PodmanResource):
"""Details and configuration for a networks managed by the Podman service.
Attributes:
attrs (Dict[str, Any]): Attributes of Network reported from Podman service
attrs (dict[str, Any]): Attributes of Network reported from Podman service
"""

@property
Expand All @@ -41,7 +41,7 @@ def id(self): # pylint: disable=invalid-name

@property
def containers(self):
"""List[Container]: Returns list of Containers connected to network."""
"""list[Container]: Returns list of Containers connected to network."""
with suppress(KeyError):
container_manager = ContainersManager(client=self.client)
return [container_manager.get(ident) for ident in self.attrs["Containers"].keys()]
Expand Down Expand Up @@ -71,12 +71,12 @@ def connect(self, container: Union[str, Container], *_, **kwargs) -> None:
container: To add to this Network
Keyword Args:
aliases (List[str]): Aliases to add for this endpoint
driver_opt (Dict[str, Any]): Options to provide to network driver
aliases (list[str]): Aliases to add for this endpoint
driver_opt (dict[str, Any]): Options to provide to network driver
ipv4_address (str): IPv4 address for given Container on this network
ipv6_address (str): IPv6 address for given Container on this network
link_local_ips (List[str]): list of link-local addresses
links (List[Union[str, Containers]]): Ignored
link_local_ips (list[str]): list of link-local addresses
links (list[Union[str, Containers]]): Ignored
Raises:
APIError: when Podman service reports an error
Expand Down
7 changes: 6 additions & 1 deletion podman/domain/pods_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import json
import logging
from typing import Any, Optional, Union
from collections.abc import Iterator

from podman import api
from podman.domain.manager import Manager
Expand All @@ -11,6 +12,8 @@

logger = logging.getLogger("podman.pods")

builtin_list = list


class PodsManager(Manager):
"""Specialized Manager for Pod resources."""
Expand Down Expand Up @@ -129,7 +132,9 @@ def remove(self, pod_id: Union[Pod, str], force: Optional[bool] = None) -> None:
response = self.client.delete(f"/pods/{pod_id}", params={"force": force})
response.raise_for_status()

def stats(self, **kwargs) -> Union[list[dict[str, Any]], [list[dict[str, Any]]]]:
def stats(
self, **kwargs
) -> Union[builtin_list[dict[str, Any]], Iterator[builtin_list[dict[str, Any]]]]:
"""Resource usage statistics for the containers in pods.
Keyword Args:
Expand Down
2 changes: 1 addition & 1 deletion podman/domain/secrets.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def list(self, **kwargs) -> list[Secret]:
"""Report on Secrets.
Keyword Args:
filters (Dict[str, Any]): Ignored.
filters (dict[str, Any]): Ignored.
Raises:
APIError: when error returned by service
Expand Down
4 changes: 2 additions & 2 deletions podman/domain/volumes.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ def create(self, name: Optional[str] = None, **kwargs) -> Volume:
Keyword Args:
driver (str): Volume driver to use
driver_opts (Dict[str, str]): Options to use with driver
labels (Dict[str, str]): Labels to apply to volume
driver_opts (dict[str, str]): Options to use with driver
labels (dict[str, str]): Labels to apply to volume
Raises:
APIError: when service reports error
Expand Down
4 changes: 2 additions & 2 deletions podman/tests/unit/test_api_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ class TestCase:
TestCase(name="empty str", input="", expected=None),
TestCase(name="str", input="reference=fedora", expected='{"reference": ["fedora"]}'),
TestCase(
name="List[str]", input=["reference=fedora"], expected='{"reference": ["fedora"]}'
name="list[str]", input=["reference=fedora"], expected='{"reference": ["fedora"]}'
),
TestCase(
name="Dict[str,str]",
name="dict[str,str]",
input={"reference": "fedora"},
expected='{"reference": ["fedora"]}',
),
Expand Down

0 comments on commit ca78098

Please sign in to comment.