From 12e634d8b9e50e9004d3a90e1eb5aae0704a05ba Mon Sep 17 00:00:00 2001 From: jo Date: Thu, 20 Jul 2023 17:00:35 +0200 Subject: [PATCH 1/7] refactor: simplify client _get_all implementation The `get_list` function returns a named tuple that always has the following structure `(result, meta)`. Since this is a tuple, we can unpack it without using the named attributes. This allows us to remove the `results_list_attribute_name` used to access the data in the tuple. --- hcloud/core/client.py | 17 +++++------------ tests/unit/core/test_client.py | 26 -------------------------- 2 files changed, 5 insertions(+), 38 deletions(-) diff --git a/hcloud/core/client.py b/hcloud/core/client.py index 3224ac6b..57406715 100644 --- a/hcloud/core/client.py +++ b/hcloud/core/client.py @@ -34,24 +34,20 @@ def _add_meta_to_result( def _get_all( self, list_function, # type: function - results_list_attribute_name, # type: str *args, **kwargs, ): # type (...) -> List[BoundModelBase] - - page = 1 - results = [] + page = 1 while page: - page_result = list_function( + result, meta = list_function( page=page, per_page=self.max_per_page, *args, **kwargs ) - result = getattr(page_result, results_list_attribute_name) if result: results.extend(result) - meta = page_result.meta + if ( meta and meta.pagination @@ -66,17 +62,14 @@ def _get_all( def get_all(self, *args, **kwargs): # type: (...) -> List[BoundModelBase] - self._is_list_attribute_implemented() - return self._get_all( - self.get_list, self.results_list_attribute_name, *args, **kwargs - ) + return self._get_all(self.get_list, *args, **kwargs) def get_actions(self, *args, **kwargs): # type: (...) -> List[BoundModelBase] if not hasattr(self, "get_actions_list"): raise ValueError("this endpoint does not support get_actions method") - return self._get_all(self.get_actions_list, "actions", *args, **kwargs) + return self._get_all(self.get_actions_list, *args, **kwargs) class GetEntityByNameMixin: diff --git a/tests/unit/core/test_client.py b/tests/unit/core/test_client.py index 80bab5e3..cf5f500c 100644 --- a/tests/unit/core/test_client.py +++ b/tests/unit/core/test_client.py @@ -205,32 +205,6 @@ def json_content_function(p): (23, 3, "sweet", 50), ] - def test_raise_exception_if_list_attribute_is_not_implemented( - self, client_class_with_actions_constructor - ): - def json_content_function(p): - return { - "actions": [10 + p, 20 + p], - "meta": { - "pagination": { - "page": p, - "per_page": 11, - "next_page": p + 1 if p < 3 else None, - } - }, - } - - candies_client = client_class_with_actions_constructor(json_content_function) - - with pytest.raises(NotImplementedError) as exception_info: - candies_client.get_all() - - error = exception_info.value - assert ( - str(error) - == "in order to get results list, 'results_list_attribute_name' attribute of CandiesClient has to be specified" - ) - class TestGetEntityByNameMixin: @pytest.fixture() From c87407358aaf709c461f12a55d366aae715cde3c Mon Sep 17 00:00:00 2001 From: jo Date: Thu, 20 Jul 2023 17:08:23 +0200 Subject: [PATCH 2/7] refactor: rewrite Meta.parse_meta method --- hcloud/core/domain.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/hcloud/core/domain.py b/hcloud/core/domain.py index 302ca9ec..eab4b935 100644 --- a/hcloud/core/domain.py +++ b/hcloud/core/domain.py @@ -63,14 +63,15 @@ def __init__(self, pagination=None): self.pagination = pagination @classmethod - def parse_meta(cls, json_content): + def parse_meta(cls, response: dict) -> Meta | None: meta = None - if json_content and "meta" in json_content: + if response and "meta" in response: meta = cls() - pagination_json = json_content["meta"].get("pagination") - if pagination_json: - pagination = Pagination(**pagination_json) - meta.pagination = pagination + try: + meta.pagination = Pagination(**response["meta"]["pagination"]) + except KeyError: + pass + return meta From 8911cb4e540b94ed2cb304bca1fa092afaeffc81 Mon Sep 17 00:00:00 2001 From: jo Date: Thu, 20 Jul 2023 17:24:53 +0200 Subject: [PATCH 3/7] refactor: simplify GetEntityByNameMixin Unpack the page result tuple instead of accessing the data by attr name. --- hcloud/core/client.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/hcloud/core/client.py b/hcloud/core/client.py index 57406715..d73f00a4 100644 --- a/hcloud/core/client.py +++ b/hcloud/core/client.py @@ -79,11 +79,8 @@ class GetEntityByNameMixin: def get_by_name(self, name): # type: (str) -> BoundModelBase - self._is_list_attribute_implemented() - response = self.get_list(name=name) - entities = getattr(response, self.results_list_attribute_name) - entity = entities[0] if entities else None - return entity + entities, _ = self.get_list(name=name) + return entities[0] if entities else None class BoundModelBase: From 56bb9de6278c73b57ee5e62f3f89bfdd5d8984d9 Mon Sep 17 00:00:00 2001 From: jo Date: Thu, 20 Jul 2023 17:54:46 +0200 Subject: [PATCH 4/7] refactor: don't use dynamic page results namedtuple --- hcloud/actions/client.py | 9 ++++++++- hcloud/certificates/client.py | 15 ++++++++++---- hcloud/core/client.py | 11 ----------- hcloud/core/domain.py | 9 --------- hcloud/datacenters/client.py | 10 +++++++++- hcloud/firewalls/client.py | 15 ++++++++++---- hcloud/floating_ips/client.py | 15 ++++++++++---- hcloud/images/client.py | 15 ++++++++++---- hcloud/isos/client.py | 9 ++++++++- hcloud/load_balancer_types/client.py | 12 +++++++++++- hcloud/load_balancers/client.py | 17 +++++++++++----- hcloud/locations/client.py | 10 +++++++++- hcloud/networks/client.py | 17 +++++++++++----- hcloud/placement_groups/client.py | 10 +++++++++- hcloud/primary_ips/client.py | 10 +++++++++- hcloud/server_types/client.py | 10 +++++++++- hcloud/servers/client.py | 15 ++++++++++---- hcloud/ssh_keys/client.py | 12 ++++++++++-- hcloud/volumes/client.py | 15 ++++++++++---- tests/unit/core/test_client.py | 18 +++++++++++++---- tests/unit/core/test_domain.py | 29 +--------------------------- 21 files changed, 187 insertions(+), 96 deletions(-) diff --git a/hcloud/actions/client.py b/hcloud/actions/client.py index 314a8059..951bf141 100644 --- a/hcloud/actions/client.py +++ b/hcloud/actions/client.py @@ -1,8 +1,10 @@ from __future__ import annotations import time +from typing import NamedTuple from ..core.client import BoundModelBase, ClientEntityBase +from ..core.domain import Meta from .domain import Action, ActionFailedException, ActionTimeoutException @@ -29,6 +31,11 @@ def wait_until_finished(self, max_retries=100): raise ActionFailedException(action=self) +class ActionsPageResult(NamedTuple): + actions: list[BoundAction] + meta: Meta | None + + class ActionsClient(ClientEntityBase): results_list_attribute_name = "actions" @@ -77,7 +84,7 @@ def get_list( actions = [ BoundAction(self, action_data) for action_data in response["actions"] ] - return self._add_meta_to_result(actions, response) + return ActionsPageResult(actions, Meta.parse_meta(response)) def get_all(self, status=None, sort=None): # type: (Optional[List[str]], Optional[List[str]]) -> List[BoundAction] diff --git a/hcloud/certificates/client.py b/hcloud/certificates/client.py index 548feb7e..283109eb 100644 --- a/hcloud/certificates/client.py +++ b/hcloud/certificates/client.py @@ -1,8 +1,10 @@ from __future__ import annotations -from ..actions.client import BoundAction +from typing import NamedTuple + +from ..actions.client import ActionsPageResult, BoundAction from ..core.client import BoundModelBase, ClientEntityBase, GetEntityByNameMixin -from ..core.domain import add_meta_to_result +from ..core.domain import Meta from .domain import ( Certificate, CreateManagedCertificateResponse, @@ -83,6 +85,11 @@ def retry_issuance(self): return self._client.retry_issuance(self) +class CertificatesPageResult(NamedTuple): + certificates: list[BoundCertificate] + meta: Meta | None + + class CertificatesClient(ClientEntityBase, GetEntityByNameMixin): results_list_attribute_name = "certificates" @@ -138,7 +145,7 @@ def get_list( for certificate_data in response["certificates"] ] - return self._add_meta_to_result(certificates, response) + return CertificatesPageResult(certificates, Meta.parse_meta(response)) def get_all(self, name=None, label_selector=None): # type: (Optional[str], Optional[str]) -> List[BoundCertificate] @@ -287,7 +294,7 @@ def get_actions_list( BoundAction(self._client.actions, action_data) for action_data in response["actions"] ] - return add_meta_to_result(actions, response, "actions") + return ActionsPageResult(actions, Meta.parse_meta(response)) def get_actions(self, certificate, status=None, sort=None): # type: (Certificate, Optional[List[str]], Optional[List[str]]) -> List[BoundAction] diff --git a/hcloud/core/client.py b/hcloud/core/client.py index d73f00a4..4cd61489 100644 --- a/hcloud/core/client.py +++ b/hcloud/core/client.py @@ -1,7 +1,5 @@ from __future__ import annotations -from .domain import add_meta_to_result - class ClientEntityBase: max_per_page = 50 @@ -22,15 +20,6 @@ def _is_list_attribute_implemented(self): ) ) - def _add_meta_to_result( - self, - results, # type: List[BoundModelBase] - response, # type: json - ): - # type: (...) -> PageResult - self._is_list_attribute_implemented() - return add_meta_to_result(results, response, self.results_list_attribute_name) - def _get_all( self, list_function, # type: function diff --git a/hcloud/core/domain.py b/hcloud/core/domain.py index eab4b935..8b0a2574 100644 --- a/hcloud/core/domain.py +++ b/hcloud/core/domain.py @@ -1,7 +1,5 @@ from __future__ import annotations -from collections import namedtuple - class BaseDomain: __slots__ = () @@ -73,10 +71,3 @@ def parse_meta(cls, response: dict) -> Meta | None: pass return meta - - -def add_meta_to_result(result, json_content, attr_name): - # type: (List[BoundModelBase], json, string) -> PageResult - class_name = f"PageResults{attr_name.capitalize()}" - PageResults = namedtuple(class_name, [attr_name, "meta"]) - return PageResults(**{attr_name: result, "meta": Meta.parse_meta(json_content)}) diff --git a/hcloud/datacenters/client.py b/hcloud/datacenters/client.py index 9166648a..03a31f68 100644 --- a/hcloud/datacenters/client.py +++ b/hcloud/datacenters/client.py @@ -1,6 +1,9 @@ from __future__ import annotations +from typing import NamedTuple + from ..core.client import BoundModelBase, ClientEntityBase, GetEntityByNameMixin +from ..core.domain import Meta from ..locations.client import BoundLocation from ..server_types.client import BoundServerType from .domain import Datacenter, DatacenterServerTypes @@ -43,6 +46,11 @@ def __init__(self, client, data): super().__init__(client, data) +class DatacentersPageResult(NamedTuple): + datacenters: list[BoundDatacenter] + meta: Meta | None + + class DatacentersClient(ClientEntityBase, GetEntityByNameMixin): results_list_attribute_name = "datacenters" @@ -90,7 +98,7 @@ def get_list( for datacenter_data in response["datacenters"] ] - return self._add_meta_to_result(datacenters, response) + return DatacentersPageResult(datacenters, Meta.parse_meta(response)) def get_all(self, name=None): # type: (Optional[str]) -> List[BoundDatacenter] diff --git a/hcloud/firewalls/client.py b/hcloud/firewalls/client.py index a3c76d13..0150eac5 100644 --- a/hcloud/firewalls/client.py +++ b/hcloud/firewalls/client.py @@ -1,8 +1,10 @@ from __future__ import annotations -from ..actions.client import BoundAction +from typing import NamedTuple + +from ..actions.client import ActionsPageResult, BoundAction from ..core.client import BoundModelBase, ClientEntityBase, GetEntityByNameMixin -from ..core.domain import add_meta_to_result +from ..core.domain import Meta from .domain import ( CreateFirewallResponse, Firewall, @@ -134,6 +136,11 @@ def remove_from_resources(self, resources): return self._client.remove_from_resources(self, resources) +class FirewallsPageResult(NamedTuple): + firewalls: list[BoundFirewall] + meta: Meta | None + + class FirewallsClient(ClientEntityBase, GetEntityByNameMixin): results_list_attribute_name = "firewalls" @@ -177,7 +184,7 @@ def get_actions_list( BoundAction(self._client.actions, action_data) for action_data in response["actions"] ] - return add_meta_to_result(actions, response, "actions") + return ActionsPageResult(actions, Meta.parse_meta(response)) def get_actions( self, @@ -249,7 +256,7 @@ def get_list( for firewall_data in response["firewalls"] ] - return self._add_meta_to_result(firewalls, response) + return FirewallsPageResult(firewalls, Meta.parse_meta(response)) def get_all(self, label_selector=None, name=None, sort=None): # type: (Optional[str], Optional[str], Optional[List[str]]) -> List[BoundFirewall] diff --git a/hcloud/floating_ips/client.py b/hcloud/floating_ips/client.py index 16a1dd52..d41e90b2 100644 --- a/hcloud/floating_ips/client.py +++ b/hcloud/floating_ips/client.py @@ -1,8 +1,10 @@ from __future__ import annotations -from ..actions.client import BoundAction +from typing import NamedTuple + +from ..actions.client import ActionsPageResult, BoundAction from ..core.client import BoundModelBase, ClientEntityBase, GetEntityByNameMixin -from ..core.domain import add_meta_to_result +from ..core.domain import Meta from ..locations.client import BoundLocation from .domain import CreateFloatingIPResponse, FloatingIP @@ -119,6 +121,11 @@ def change_dns_ptr(self, ip, dns_ptr): return self._client.change_dns_ptr(self, ip, dns_ptr) +class FloatingIPsPageResult(NamedTuple): + floating_ips: list[BoundFloatingIP] + meta: Meta | None + + class FloatingIPsClient(ClientEntityBase, GetEntityByNameMixin): results_list_attribute_name = "floating_ips" @@ -164,7 +171,7 @@ def get_actions_list( BoundAction(self._client.actions, action_data) for action_data in response["actions"] ] - return add_meta_to_result(actions, response, "actions") + return ActionsPageResult(actions, Meta.parse_meta(response)) def get_actions( self, @@ -234,7 +241,7 @@ def get_list( for floating_ip_data in response["floating_ips"] ] - return self._add_meta_to_result(floating_ips, response) + return FloatingIPsPageResult(floating_ips, Meta.parse_meta(response)) def get_all(self, label_selector=None, name=None): # type: (Optional[str], Optional[str]) -> List[BoundFloatingIP] diff --git a/hcloud/images/client.py b/hcloud/images/client.py index 7e099154..8a72816c 100644 --- a/hcloud/images/client.py +++ b/hcloud/images/client.py @@ -1,8 +1,10 @@ from __future__ import annotations -from ..actions.client import BoundAction +from typing import NamedTuple + +from ..actions.client import ActionsPageResult, BoundAction from ..core.client import BoundModelBase, ClientEntityBase, GetEntityByNameMixin -from ..core.domain import add_meta_to_result +from ..core.domain import Meta from .domain import Image @@ -89,6 +91,11 @@ def change_protection(self, delete=None): return self._client.change_protection(self, delete) +class ImagesPageResult(NamedTuple): + images: list[BoundImage] + meta: Meta | None + + class ImagesClient(ClientEntityBase, GetEntityByNameMixin): results_list_attribute_name = "images" @@ -132,7 +139,7 @@ def get_actions_list( BoundAction(self._client.actions, action_data) for action_data in response["actions"] ] - return add_meta_to_result(actions, response, "actions") + return ActionsPageResult(actions, Meta.parse_meta(response)) def get_actions( self, @@ -224,7 +231,7 @@ def get_list( response = self._client.request(url="/images", method="GET", params=params) images = [BoundImage(self, image_data) for image_data in response["images"]] - return self._add_meta_to_result(images, response) + return ImagesPageResult(images, Meta.parse_meta(response)) def get_all( self, diff --git a/hcloud/isos/client.py b/hcloud/isos/client.py index 9d0fcddb..2e84c670 100644 --- a/hcloud/isos/client.py +++ b/hcloud/isos/client.py @@ -1,8 +1,10 @@ from __future__ import annotations +from typing import NamedTuple from warnings import warn from ..core.client import BoundModelBase, ClientEntityBase, GetEntityByNameMixin +from ..core.domain import Meta from .domain import Iso @@ -10,6 +12,11 @@ class BoundIso(BoundModelBase): model = Iso +class IsosPageResult(NamedTuple): + isos: list[BoundIso] + meta: Meta | None + + class IsosClient(ClientEntityBase, GetEntityByNameMixin): results_list_attribute_name = "isos" @@ -72,7 +79,7 @@ def get_list( response = self._client.request(url="/isos", method="GET", params=params) isos = [BoundIso(self, iso_data) for iso_data in response["isos"]] - return self._add_meta_to_result(isos, response) + return IsosPageResult(isos, Meta.parse_meta(response)) def get_all( self, diff --git a/hcloud/load_balancer_types/client.py b/hcloud/load_balancer_types/client.py index 12e5edd3..1ac9ff1b 100644 --- a/hcloud/load_balancer_types/client.py +++ b/hcloud/load_balancer_types/client.py @@ -1,6 +1,9 @@ from __future__ import annotations +from typing import NamedTuple + from ..core.client import BoundModelBase, ClientEntityBase, GetEntityByNameMixin +from ..core.domain import Meta from .domain import LoadBalancerType @@ -8,6 +11,11 @@ class BoundLoadBalancerType(BoundModelBase): model = LoadBalancerType +class LoadBalancerTypesPageResult(NamedTuple): + load_balancer_types: list[BoundLoadBalancerType] + meta: Meta | None + + class LoadBalancerTypesClient(ClientEntityBase, GetEntityByNameMixin): results_list_attribute_name = "load_balancer_types" @@ -53,7 +61,9 @@ def get_list(self, name=None, page=None, per_page=None): BoundLoadBalancerType(self, load_balancer_type_data) for load_balancer_type_data in response["load_balancer_types"] ] - return self._add_meta_to_result(load_balancer_types, response) + return LoadBalancerTypesPageResult( + load_balancer_types, Meta.parse_meta(response) + ) def get_all(self, name=None): # type: (Optional[str]) -> List[BoundLoadBalancerType] diff --git a/hcloud/load_balancers/client.py b/hcloud/load_balancers/client.py index 4c913154..9ad7e46e 100644 --- a/hcloud/load_balancers/client.py +++ b/hcloud/load_balancers/client.py @@ -1,9 +1,11 @@ from __future__ import annotations -from ..actions.client import BoundAction +from typing import NamedTuple + +from ..actions.client import ActionsPageResult, BoundAction from ..certificates.client import BoundCertificate from ..core.client import BoundModelBase, ClientEntityBase, GetEntityByNameMixin -from ..core.domain import add_meta_to_result +from ..core.domain import Meta from ..load_balancer_types.client import BoundLoadBalancerType from ..locations.client import BoundLocation from ..networks.client import BoundNetwork @@ -310,6 +312,11 @@ def change_type(self, load_balancer_type): return self._client.change_type(self, load_balancer_type) +class LoadBalancersPageResult(NamedTuple): + load_balancers: list[BoundLoadBalancer] + meta: Meta | None + + class LoadBalancersClient(ClientEntityBase, GetEntityByNameMixin): results_list_attribute_name = "load_balancers" @@ -360,11 +367,11 @@ def get_list( url="/load_balancers", method="GET", params=params ) - ass_load_balancers = [ + load_balancers = [ BoundLoadBalancer(self, load_balancer_data) for load_balancer_data in response["load_balancers"] ] - return self._add_meta_to_result(ass_load_balancers, response) + return LoadBalancersPageResult(load_balancers, Meta.parse_meta(response)) def get_all(self, name=None, label_selector=None): # type: (Optional[str], Optional[str]) -> List[BoundLoadBalancer] @@ -550,7 +557,7 @@ def get_actions_list( BoundAction(self._client.actions, action_data) for action_data in response["actions"] ] - return add_meta_to_result(actions, response, "actions") + return ActionsPageResult(actions, Meta.parse_meta(response)) def get_actions(self, load_balancer, status=None, sort=None): # type: (LoadBalancer, Optional[List[str]], Optional[List[str]]) -> List[BoundAction] diff --git a/hcloud/locations/client.py b/hcloud/locations/client.py index 97b5fdfd..473b098d 100644 --- a/hcloud/locations/client.py +++ b/hcloud/locations/client.py @@ -1,6 +1,9 @@ from __future__ import annotations +from typing import NamedTuple + from ..core.client import BoundModelBase, ClientEntityBase, GetEntityByNameMixin +from ..core.domain import Meta from .domain import Location @@ -8,6 +11,11 @@ class BoundLocation(BoundModelBase): model = Location +class LocationsPageResult(NamedTuple): + locations: list[BoundLocation] + meta: Meta | None + + class LocationsClient(ClientEntityBase, GetEntityByNameMixin): results_list_attribute_name = "locations" @@ -46,7 +54,7 @@ def get_list(self, name=None, page=None, per_page=None): BoundLocation(self, location_data) for location_data in response["locations"] ] - return self._add_meta_to_result(locations, response) + return LocationsPageResult(locations, Meta.parse_meta(response)) def get_all(self, name=None): # type: (Optional[str]) -> List[BoundLocation] diff --git a/hcloud/networks/client.py b/hcloud/networks/client.py index 46d25f5e..14f294fc 100644 --- a/hcloud/networks/client.py +++ b/hcloud/networks/client.py @@ -1,8 +1,10 @@ from __future__ import annotations -from ..actions.client import BoundAction +from typing import NamedTuple + +from ..actions.client import ActionsPageResult, BoundAction from ..core.client import BoundModelBase, ClientEntityBase, GetEntityByNameMixin -from ..core.domain import add_meta_to_result +from ..core.domain import Meta from .domain import Network, NetworkRoute, NetworkSubnet @@ -153,6 +155,11 @@ def change_protection(self, delete=None): return self._client.change_protection(self, delete=delete) +class NetworksPageResult(NamedTuple): + networks: list[BoundNetwork] + meta: Meta | None + + class NetworksClient(ClientEntityBase, GetEntityByNameMixin): results_list_attribute_name = "networks" @@ -198,10 +205,10 @@ def get_list( response = self._client.request(url="/networks", method="GET", params=params) - ass_networks = [ + networks = [ BoundNetwork(self, network_data) for network_data in response["networks"] ] - return self._add_meta_to_result(ass_networks, response) + return NetworksPageResult(networks, Meta.parse_meta(response)) def get_all(self, name=None, label_selector=None): # type: (Optional[str], Optional[str]) -> List[BoundNetwork] @@ -359,7 +366,7 @@ def get_actions_list( BoundAction(self._client.actions, action_data) for action_data in response["actions"] ] - return add_meta_to_result(actions, response, "actions") + return ActionsPageResult(actions, Meta.parse_meta(response)) def get_actions(self, network, status=None, sort=None): # type: (Network, Optional[List[str]], Optional[List[str]]) -> List[BoundAction] diff --git a/hcloud/placement_groups/client.py b/hcloud/placement_groups/client.py index ca78bc82..f7ce3e8b 100644 --- a/hcloud/placement_groups/client.py +++ b/hcloud/placement_groups/client.py @@ -1,7 +1,10 @@ from __future__ import annotations +from typing import NamedTuple + from ..actions.client import BoundAction from ..core.client import BoundModelBase, ClientEntityBase, GetEntityByNameMixin +from ..core.domain import Meta from .domain import CreatePlacementGroupResponse, PlacementGroup @@ -29,6 +32,11 @@ def delete(self): return self._client.delete(self) +class PlacementGroupsPageResult(NamedTuple): + placement_groups: list[BoundPlacementGroup] + meta: Meta | None + + class PlacementGroupsClient(ClientEntityBase, GetEntityByNameMixin): results_list_attribute_name = "placement_groups" @@ -92,7 +100,7 @@ def get_list( for placement_group_data in response["placement_groups"] ] - return self._add_meta_to_result(placement_groups, response) + return PlacementGroupsPageResult(placement_groups, Meta.parse_meta(response)) def get_all(self, label_selector=None, name=None, sort=None): # type: (Optional[str], Optional[str], Optional[List[str]]) -> List[BoundPlacementGroup] diff --git a/hcloud/primary_ips/client.py b/hcloud/primary_ips/client.py index d33f0f20..93ab40ae 100644 --- a/hcloud/primary_ips/client.py +++ b/hcloud/primary_ips/client.py @@ -1,7 +1,10 @@ from __future__ import annotations +from typing import NamedTuple + from ..actions.client import BoundAction from ..core.client import BoundModelBase, ClientEntityBase, GetEntityByNameMixin +from ..core.domain import Meta from .domain import CreatePrimaryIPResponse, PrimaryIP @@ -84,6 +87,11 @@ def change_dns_ptr(self, ip, dns_ptr): return self._client.change_dns_ptr(self, ip, dns_ptr) +class PrimaryIPsPageResult(NamedTuple): + primary_ips: list[BoundPrimaryIP] + meta: Meta | None + + class PrimaryIPsClient(ClientEntityBase, GetEntityByNameMixin): results_list_attribute_name = "primary_ips" @@ -139,7 +147,7 @@ def get_list( for primary_ip_data in response["primary_ips"] ] - return self._add_meta_to_result(primary_ips, response) + return PrimaryIPsPageResult(primary_ips, Meta.parse_meta(response)) def get_all(self, label_selector=None, name=None): # type: (Optional[str], Optional[str]) -> List[BoundPrimaryIP] diff --git a/hcloud/server_types/client.py b/hcloud/server_types/client.py index 70e29e69..fa6a3da9 100644 --- a/hcloud/server_types/client.py +++ b/hcloud/server_types/client.py @@ -1,6 +1,9 @@ from __future__ import annotations +from typing import NamedTuple + from ..core.client import BoundModelBase, ClientEntityBase, GetEntityByNameMixin +from ..core.domain import Meta from .domain import ServerType @@ -8,6 +11,11 @@ class BoundServerType(BoundModelBase): model = ServerType +class ServerTypesPageResult(NamedTuple): + server_types: list[BoundServerType] + meta: Meta | None + + class ServerTypesClient(ClientEntityBase, GetEntityByNameMixin): results_list_attribute_name = "server_types" @@ -48,7 +56,7 @@ def get_list(self, name=None, page=None, per_page=None): BoundServerType(self, server_type_data) for server_type_data in response["server_types"] ] - return self._add_meta_to_result(server_types, response) + return ServerTypesPageResult(server_types, Meta.parse_meta(response)) def get_all(self, name=None): # type: (Optional[str]) -> List[BoundServerType] diff --git a/hcloud/servers/client.py b/hcloud/servers/client.py index 99fe53b6..2022f533 100644 --- a/hcloud/servers/client.py +++ b/hcloud/servers/client.py @@ -1,8 +1,10 @@ from __future__ import annotations -from ..actions.client import BoundAction +from typing import NamedTuple + +from ..actions.client import ActionsPageResult, BoundAction from ..core.client import BoundModelBase, ClientEntityBase, GetEntityByNameMixin -from ..core.domain import add_meta_to_result +from ..core.domain import Meta from ..datacenters.client import BoundDatacenter from ..firewalls.client import BoundFirewall from ..floating_ips.client import BoundFloatingIP @@ -409,6 +411,11 @@ def remove_from_placement_group(self): return self._client.remove_from_placement_group(self) +class ServersPageResult(NamedTuple): + servers: list[BoundServer] + meta: Meta | None + + class ServersClient(ClientEntityBase, GetEntityByNameMixin): results_list_attribute_name = "servers" @@ -462,7 +469,7 @@ def get_list( ass_servers = [ BoundServer(self, server_data) for server_data in response["servers"] ] - return self._add_meta_to_result(ass_servers, response) + return ServersPageResult(ass_servers, Meta.parse_meta(response)) def get_all(self, name=None, label_selector=None, status=None): # type: (Optional[str], Optional[str], Optional[List[str]]) -> List[BoundServer] @@ -625,7 +632,7 @@ def get_actions_list( BoundAction(self._client.actions, action_data) for action_data in response["actions"] ] - return add_meta_to_result(actions, response, "actions") + return ActionsPageResult(actions, Meta.parse_meta(response)) def get_actions(self, server, status=None, sort=None): # type: (Server, Optional[List[str]], Optional[List[str]]) -> List[BoundAction] diff --git a/hcloud/ssh_keys/client.py b/hcloud/ssh_keys/client.py index 66c13fa5..afb747e4 100644 --- a/hcloud/ssh_keys/client.py +++ b/hcloud/ssh_keys/client.py @@ -1,6 +1,9 @@ from __future__ import annotations +from typing import NamedTuple + from ..core.client import BoundModelBase, ClientEntityBase, GetEntityByNameMixin +from ..core.domain import Meta from .domain import SSHKey @@ -27,6 +30,11 @@ def delete(self): return self._client.delete(self) +class SSHKeysPageResult(NamedTuple): + ssh_keys: list[BoundSSHKey] + meta: Meta | None + + class SSHKeysClient(ClientEntityBase, GetEntityByNameMixin): results_list_attribute_name = "ssh_keys" @@ -77,10 +85,10 @@ def get_list( response = self._client.request(url="/ssh_keys", method="GET", params=params) - ass_ssh_keys = [ + ssh_keys = [ BoundSSHKey(self, server_data) for server_data in response["ssh_keys"] ] - return self._add_meta_to_result(ass_ssh_keys, response) + return SSHKeysPageResult(ssh_keys, Meta.parse_meta(response)) def get_all(self, name=None, fingerprint=None, label_selector=None): # type: (Optional[str], Optional[str], Optional[str]) -> List[BoundSSHKey] diff --git a/hcloud/volumes/client.py b/hcloud/volumes/client.py index bd9dff55..2c21d1ec 100644 --- a/hcloud/volumes/client.py +++ b/hcloud/volumes/client.py @@ -1,8 +1,10 @@ from __future__ import annotations -from ..actions.client import BoundAction +from typing import NamedTuple + +from ..actions.client import ActionsPageResult, BoundAction from ..core.client import BoundModelBase, ClientEntityBase, GetEntityByNameMixin -from ..core.domain import add_meta_to_result +from ..core.domain import Meta from ..locations.client import BoundLocation from .domain import CreateVolumeResponse, Volume @@ -111,6 +113,11 @@ def change_protection(self, delete=None): return self._client.change_protection(self, delete) +class VolumesPageResult(NamedTuple): + volumes: list[BoundVolume] + meta: Meta | None + + class VolumesClient(ClientEntityBase, GetEntityByNameMixin): results_list_attribute_name = "volumes" @@ -158,7 +165,7 @@ def get_list( volumes = [ BoundVolume(self, volume_data) for volume_data in response["volumes"] ] - return self._add_meta_to_result(volumes, response) + return VolumesPageResult(volumes, Meta.parse_meta(response)) def get_all(self, label_selector=None, status=None): # type: (Optional[str], Optional[List[str]]) -> List[BoundVolume] @@ -277,7 +284,7 @@ def get_actions_list( BoundAction(self._client.actions, action_data) for action_data in response["actions"] ] - return add_meta_to_result(actions, response, "actions") + return ActionsPageResult(actions, Meta.parse_meta(response)) def get_actions(self, volume, status=None, sort=None): # type: (Union[Volume, BoundVolume], Optional[List[str]], Optional[List[str]]) -> List[BoundAction] diff --git a/tests/unit/core/test_client.py b/tests/unit/core/test_client.py index cf5f500c..09419648 100644 --- a/tests/unit/core/test_client.py +++ b/tests/unit/core/test_client.py @@ -1,11 +1,13 @@ from __future__ import annotations +from typing import Any, NamedTuple from unittest import mock import pytest +from hcloud.actions.client import ActionsPageResult from hcloud.core.client import BoundModelBase, ClientEntityBase, GetEntityByNameMixin -from hcloud.core.domain import BaseDomain, add_meta_to_result +from hcloud.core.domain import BaseDomain, Meta class TestBoundModelBase: @@ -84,6 +86,10 @@ class TestClientEntityBase: @pytest.fixture() def client_class_constructor(self): def constructor(json_content_function): + class CandiesPageResult(NamedTuple): + candies: list[Any] + meta: Meta + class CandiesClient(ClientEntityBase): results_list_attribute_name = "candies" @@ -92,7 +98,7 @@ def get_list(self, status, page=None, per_page=None): results = [ (r, page, status, per_page) for r in json_content["candies"] ] - return self._add_meta_to_result(results, json_content) + return CandiesPageResult(results, Meta.parse_meta(json_content)) return CandiesClient(mock.MagicMock()) @@ -107,7 +113,7 @@ def get_actions_list(self, status, page=None, per_page=None): results = [ (r, page, status, per_page) for r in json_content["actions"] ] - return add_meta_to_result(results, json_content, "actions") + return ActionsPageResult(results, Meta.parse_meta(json_content)) return CandiesClient(mock.MagicMock()) @@ -210,13 +216,17 @@ class TestGetEntityByNameMixin: @pytest.fixture() def client_class_constructor(self): def constructor(json_content_function): + class CandiesPageResult(NamedTuple): + candies: list[Any] + meta: Meta + class CandiesClient(ClientEntityBase, GetEntityByNameMixin): results_list_attribute_name = "candies" def get_list(self, name, page=None, per_page=None): json_content = json_content_function(page) results = json_content["candies"] - return self._add_meta_to_result(results, json_content) + return CandiesPageResult(results, Meta.parse_meta(json_content)) return CandiesClient(mock.MagicMock()) diff --git a/tests/unit/core/test_domain.py b/tests/unit/core/test_domain.py index e9e06a35..97cb6f74 100644 --- a/tests/unit/core/test_domain.py +++ b/tests/unit/core/test_domain.py @@ -3,13 +3,7 @@ import pytest from dateutil.parser import isoparse -from hcloud.core.domain import ( - BaseDomain, - DomainIdentityMixin, - Meta, - Pagination, - add_meta_to_result, -) +from hcloud.core.domain import BaseDomain, DomainIdentityMixin, Meta, Pagination class TestMeta: @@ -46,27 +40,6 @@ def test_parse_meta_json_ok(self): assert result.pagination.last_page == 10 assert result.pagination.total_entries == 100 - def test_add_meta_to_result(self): - json_content = { - "meta": { - "pagination": { - "page": 2, - "per_page": 10, - "previous_page": 1, - "next_page": 3, - "last_page": 10, - "total_entries": 100, - } - } - } - result = add_meta_to_result([1, 2, 3], json_content, "id_list") - assert result.id_list == [1, 2, 3] - assert result.meta.pagination.page == 2 - assert result.meta.pagination.per_page == 10 - assert result.meta.pagination.next_page == 3 - assert result.meta.pagination.last_page == 10 - assert result.meta.pagination.total_entries == 100 - class SomeDomain(BaseDomain, DomainIdentityMixin): __slots__ = ("id", "name") From 35e1e10cfd5e5f1cdb1083bca16782af877ec9ab Mon Sep 17 00:00:00 2001 From: jo Date: Thu, 20 Jul 2023 18:03:28 +0200 Subject: [PATCH 5/7] refactor: simplify ImagesClient.get_by_name_and_architecture --- hcloud/images/client.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hcloud/images/client.py b/hcloud/images/client.py index 8a72816c..cccac20d 100644 --- a/hcloud/images/client.py +++ b/hcloud/images/client.py @@ -298,8 +298,7 @@ def get_by_name_and_architecture(self, name, architecture): Used to identify the image. :return: :class:`BoundImage ` """ - response = self.get_list(name=name, architecture=[architecture]) - entities = getattr(response, self.results_list_attribute_name) + entities, _ = self.get_list(name=name, architecture=[architecture]) entity = entities[0] if entities else None return entity From 7c5485b8fb15162247889c8a4349db898f8115d2 Mon Sep 17 00:00:00 2001 From: jo Date: Thu, 20 Jul 2023 18:04:52 +0200 Subject: [PATCH 6/7] refactor: remove unused results_list_attribute_name attr --- hcloud/actions/client.py | 2 -- hcloud/certificates/client.py | 2 -- hcloud/core/client.py | 8 -------- hcloud/datacenters/client.py | 2 -- hcloud/firewalls/client.py | 2 -- hcloud/floating_ips/client.py | 2 -- hcloud/images/client.py | 2 -- hcloud/isos/client.py | 2 -- hcloud/load_balancer_types/client.py | 2 -- hcloud/load_balancers/client.py | 2 -- hcloud/locations/client.py | 2 -- hcloud/networks/client.py | 2 -- hcloud/placement_groups/client.py | 2 -- hcloud/primary_ips/client.py | 2 -- hcloud/server_types/client.py | 2 -- hcloud/servers/client.py | 2 -- hcloud/ssh_keys/client.py | 2 -- hcloud/volumes/client.py | 2 -- tests/unit/core/test_client.py | 4 ---- 19 files changed, 46 deletions(-) diff --git a/hcloud/actions/client.py b/hcloud/actions/client.py index 951bf141..ef262384 100644 --- a/hcloud/actions/client.py +++ b/hcloud/actions/client.py @@ -37,8 +37,6 @@ class ActionsPageResult(NamedTuple): class ActionsClient(ClientEntityBase): - results_list_attribute_name = "actions" - def get_by_id(self, id): # type: (int) -> BoundAction """Get a specific action by its ID. diff --git a/hcloud/certificates/client.py b/hcloud/certificates/client.py index 283109eb..2f60bd1e 100644 --- a/hcloud/certificates/client.py +++ b/hcloud/certificates/client.py @@ -91,8 +91,6 @@ class CertificatesPageResult(NamedTuple): class CertificatesClient(ClientEntityBase, GetEntityByNameMixin): - results_list_attribute_name = "certificates" - def get_by_id(self, id): # type: (int) -> BoundCertificate """Get a specific certificate by its ID. diff --git a/hcloud/core/client.py b/hcloud/core/client.py index 4cd61489..d68c8301 100644 --- a/hcloud/core/client.py +++ b/hcloud/core/client.py @@ -12,14 +12,6 @@ def __init__(self, client): """ self._client = client - def _is_list_attribute_implemented(self): - if self.results_list_attribute_name is None: - raise NotImplementedError( - "in order to get results list, 'results_list_attribute_name' attribute of {} has to be specified".format( - self.__class__.__name__ - ) - ) - def _get_all( self, list_function, # type: function diff --git a/hcloud/datacenters/client.py b/hcloud/datacenters/client.py index 03a31f68..76e3c707 100644 --- a/hcloud/datacenters/client.py +++ b/hcloud/datacenters/client.py @@ -52,8 +52,6 @@ class DatacentersPageResult(NamedTuple): class DatacentersClient(ClientEntityBase, GetEntityByNameMixin): - results_list_attribute_name = "datacenters" - def get_by_id(self, id): # type: (int) -> BoundDatacenter """Get a specific datacenter by its ID. diff --git a/hcloud/firewalls/client.py b/hcloud/firewalls/client.py index 0150eac5..118fc850 100644 --- a/hcloud/firewalls/client.py +++ b/hcloud/firewalls/client.py @@ -142,8 +142,6 @@ class FirewallsPageResult(NamedTuple): class FirewallsClient(ClientEntityBase, GetEntityByNameMixin): - results_list_attribute_name = "firewalls" - def get_actions_list( self, firewall, # type: Firewall diff --git a/hcloud/floating_ips/client.py b/hcloud/floating_ips/client.py index d41e90b2..f6d56113 100644 --- a/hcloud/floating_ips/client.py +++ b/hcloud/floating_ips/client.py @@ -127,8 +127,6 @@ class FloatingIPsPageResult(NamedTuple): class FloatingIPsClient(ClientEntityBase, GetEntityByNameMixin): - results_list_attribute_name = "floating_ips" - def get_actions_list( self, floating_ip, # type: FloatingIP diff --git a/hcloud/images/client.py b/hcloud/images/client.py index cccac20d..55729a0f 100644 --- a/hcloud/images/client.py +++ b/hcloud/images/client.py @@ -97,8 +97,6 @@ class ImagesPageResult(NamedTuple): class ImagesClient(ClientEntityBase, GetEntityByNameMixin): - results_list_attribute_name = "images" - def get_actions_list( self, image, # type: Image diff --git a/hcloud/isos/client.py b/hcloud/isos/client.py index 2e84c670..fba3930e 100644 --- a/hcloud/isos/client.py +++ b/hcloud/isos/client.py @@ -18,8 +18,6 @@ class IsosPageResult(NamedTuple): class IsosClient(ClientEntityBase, GetEntityByNameMixin): - results_list_attribute_name = "isos" - def get_by_id(self, id): # type: (int) -> BoundIso """Get a specific ISO by its id diff --git a/hcloud/load_balancer_types/client.py b/hcloud/load_balancer_types/client.py index 1ac9ff1b..7dff45a3 100644 --- a/hcloud/load_balancer_types/client.py +++ b/hcloud/load_balancer_types/client.py @@ -17,8 +17,6 @@ class LoadBalancerTypesPageResult(NamedTuple): class LoadBalancerTypesClient(ClientEntityBase, GetEntityByNameMixin): - results_list_attribute_name = "load_balancer_types" - def get_by_id(self, id): # type: (int) -> load_balancer_types.client.BoundLoadBalancerType """Returns a specific Load Balancer Type. diff --git a/hcloud/load_balancers/client.py b/hcloud/load_balancers/client.py index 9ad7e46e..85e73d82 100644 --- a/hcloud/load_balancers/client.py +++ b/hcloud/load_balancers/client.py @@ -318,8 +318,6 @@ class LoadBalancersPageResult(NamedTuple): class LoadBalancersClient(ClientEntityBase, GetEntityByNameMixin): - results_list_attribute_name = "load_balancers" - def get_by_id(self, id): # type: (int) -> BoundLoadBalancer """Get a specific Load Balancer diff --git a/hcloud/locations/client.py b/hcloud/locations/client.py index 473b098d..c21c6a75 100644 --- a/hcloud/locations/client.py +++ b/hcloud/locations/client.py @@ -17,8 +17,6 @@ class LocationsPageResult(NamedTuple): class LocationsClient(ClientEntityBase, GetEntityByNameMixin): - results_list_attribute_name = "locations" - def get_by_id(self, id): # type: (int) -> locations.client.BoundLocation """Get a specific location by its ID. diff --git a/hcloud/networks/client.py b/hcloud/networks/client.py index 14f294fc..6a93c0cd 100644 --- a/hcloud/networks/client.py +++ b/hcloud/networks/client.py @@ -161,8 +161,6 @@ class NetworksPageResult(NamedTuple): class NetworksClient(ClientEntityBase, GetEntityByNameMixin): - results_list_attribute_name = "networks" - def get_by_id(self, id): # type: (int) -> BoundNetwork """Get a specific network diff --git a/hcloud/placement_groups/client.py b/hcloud/placement_groups/client.py index f7ce3e8b..5a97c7c5 100644 --- a/hcloud/placement_groups/client.py +++ b/hcloud/placement_groups/client.py @@ -38,8 +38,6 @@ class PlacementGroupsPageResult(NamedTuple): class PlacementGroupsClient(ClientEntityBase, GetEntityByNameMixin): - results_list_attribute_name = "placement_groups" - def get_by_id(self, id): # type: (int) -> BoundPlacementGroup """Returns a specific Placement Group object diff --git a/hcloud/primary_ips/client.py b/hcloud/primary_ips/client.py index 93ab40ae..717406a5 100644 --- a/hcloud/primary_ips/client.py +++ b/hcloud/primary_ips/client.py @@ -93,8 +93,6 @@ class PrimaryIPsPageResult(NamedTuple): class PrimaryIPsClient(ClientEntityBase, GetEntityByNameMixin): - results_list_attribute_name = "primary_ips" - def get_by_id(self, id): # type: (int) -> BoundPrimaryIP """Returns a specific Primary IP object. diff --git a/hcloud/server_types/client.py b/hcloud/server_types/client.py index fa6a3da9..ed90dbb6 100644 --- a/hcloud/server_types/client.py +++ b/hcloud/server_types/client.py @@ -17,8 +17,6 @@ class ServerTypesPageResult(NamedTuple): class ServerTypesClient(ClientEntityBase, GetEntityByNameMixin): - results_list_attribute_name = "server_types" - def get_by_id(self, id): # type: (int) -> BoundServerType """Returns a specific Server Type. diff --git a/hcloud/servers/client.py b/hcloud/servers/client.py index 2022f533..b9f7455e 100644 --- a/hcloud/servers/client.py +++ b/hcloud/servers/client.py @@ -417,8 +417,6 @@ class ServersPageResult(NamedTuple): class ServersClient(ClientEntityBase, GetEntityByNameMixin): - results_list_attribute_name = "servers" - def get_by_id(self, id): # type: (int) -> BoundServer """Get a specific server diff --git a/hcloud/ssh_keys/client.py b/hcloud/ssh_keys/client.py index afb747e4..0c89d6b6 100644 --- a/hcloud/ssh_keys/client.py +++ b/hcloud/ssh_keys/client.py @@ -36,8 +36,6 @@ class SSHKeysPageResult(NamedTuple): class SSHKeysClient(ClientEntityBase, GetEntityByNameMixin): - results_list_attribute_name = "ssh_keys" - def get_by_id(self, id): # type: (int) -> BoundSSHKey """Get a specific SSH Key by its ID diff --git a/hcloud/volumes/client.py b/hcloud/volumes/client.py index 2c21d1ec..687f145d 100644 --- a/hcloud/volumes/client.py +++ b/hcloud/volumes/client.py @@ -119,8 +119,6 @@ class VolumesPageResult(NamedTuple): class VolumesClient(ClientEntityBase, GetEntityByNameMixin): - results_list_attribute_name = "volumes" - def get_by_id(self, id): # type: (int) -> volumes.client.BoundVolume """Get a specific volume by its id diff --git a/tests/unit/core/test_client.py b/tests/unit/core/test_client.py index 09419648..6ea2cd63 100644 --- a/tests/unit/core/test_client.py +++ b/tests/unit/core/test_client.py @@ -91,8 +91,6 @@ class CandiesPageResult(NamedTuple): meta: Meta class CandiesClient(ClientEntityBase): - results_list_attribute_name = "candies" - def get_list(self, status, page=None, per_page=None): json_content = json_content_function(page) results = [ @@ -221,8 +219,6 @@ class CandiesPageResult(NamedTuple): meta: Meta class CandiesClient(ClientEntityBase, GetEntityByNameMixin): - results_list_attribute_name = "candies" - def get_list(self, name, page=None, per_page=None): json_content = json_content_function(page) results = json_content["candies"] From fc408a7c05d5532ead6c8e5cd3ea4f610d769e5b Mon Sep 17 00:00:00 2001 From: jo Date: Fri, 21 Jul 2023 17:18:41 +0200 Subject: [PATCH 7/7] add comment about page result tuple unpacking --- hcloud/core/client.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hcloud/core/client.py b/hcloud/core/client.py index d68c8301..b0bde8c1 100644 --- a/hcloud/core/client.py +++ b/hcloud/core/client.py @@ -23,6 +23,8 @@ def _get_all( page = 1 while page: + # The *PageResult tuples MUST have the following structure + # `(result: List[Bound*], meta: Meta)` result, meta = list_function( page=page, per_page=self.max_per_page, *args, **kwargs )