test: add pylint linter (#248)

* chore: add pylint linter

* refactor: fix linting errors

* docs: fix incomplete docstring

Makefile

@@ -6,6 +6,7 @@ venv:
 	venv/bin/pip install -e .[docs,test]
 
 lint: venv
+	venv/bin/pylint hcloud
 	venv/bin/mypy hcloud
 
 test: venv

hcloud/_client.py

@@ -42,7 +42,7 @@ def __init__(
         poll_interval: int = 1,
         timeout: float | tuple[float, float] | None = None,
     ):
-        """Create an new Client instance
+        """Create a new Client instance
 
         :param token: Hetzner Cloud API token
         :param api_endpoint: Hetzner Cloud API endpoint

hcloud/actions/client.py

@@ -27,6 +27,7 @@ def wait_until_finished(self, max_retries: int = 100) -> None:
         while self.status == Action.STATUS_RUNNING:
             if max_retries > 0:
                 self.reload()
+                # pylint: disable=protected-access
                 time.sleep(self._client._client.poll_interval)
                 max_retries = max_retries - 1
             else:

hcloud/core/client.py

@@ -45,6 +45,7 @@ def _iter_pages(  # type: ignore[no-untyped-def]
 
     def _get_first_by(self, **kwargs):  # type: ignore[no-untyped-def]
         assert hasattr(self, "get_list")
+        # pylint: disable=no-member
         entities, _ = self.get_list(**kwargs)
         return entities[0] if entities else None
 

hcloud/core/domain.py

@@ -6,6 +6,9 @@ class BaseDomain:
 
     @classmethod
     def from_dict(cls, data: dict):  # type: ignore[no-untyped-def]
+        """
+        Build the domain object from the data dict.
+        """
         supported_data = {k: v for k, v in data.items() if k in cls.__slots__}
         return cls(**supported_data)
 
@@ -22,12 +25,14 @@ class DomainIdentityMixin:
 
     @property
     def id_or_name(self) -> int | str:
+        """
+        Return the first defined value, and fails if none is defined.
+        """
         if self.id is not None:
             return self.id
-        elif self.name is not None:
+        if self.name is not None:
             return self.name
-        else:
-            raise ValueError("id or name must be set")
+        raise ValueError("id or name must be set")
 
 
 class Pagination(BaseDomain):
@@ -65,6 +70,9 @@ def __init__(self, pagination: Pagination | None = None):
 
     @classmethod
     def parse_meta(cls, response: dict) -> Meta | None:
+        """
+        If present, extract the meta details from the response and return a meta object.
+        """
         meta = None
         if response and "meta" in response:
             meta = cls()

hcloud/firewalls/client.py

@@ -39,29 +39,32 @@ def __init__(self, client: FirewallsClient, data: dict, complete: bool = True):
 
         applied_to = data.get("applied_to", [])
         if applied_to:
+            # pylint: disable=import-outside-toplevel
             from ..servers import BoundServer
 
-            ats = []
-            for a in applied_to:
-                if a["type"] == FirewallResource.TYPE_SERVER:
-                    ats.append(
+            data_applied_to = []
+            for firewall_resource in applied_to:
+                if firewall_resource["type"] == FirewallResource.TYPE_SERVER:
+                    data_applied_to.append(
                         FirewallResource(
-                            type=a["type"],
+                            type=firewall_resource["type"],
                             server=BoundServer(
-                                client._client.servers, a["server"], complete=False
+                                client._client.servers,
+                                firewall_resource["server"],
+                                complete=False,
                             ),
                         )
                     )
-                elif a["type"] == FirewallResource.TYPE_LABEL_SELECTOR:
-                    ats.append(
+                elif firewall_resource["type"] == FirewallResource.TYPE_LABEL_SELECTOR:
+                    data_applied_to.append(
                         FirewallResource(
-                            type=a["type"],
+                            type=firewall_resource["type"],
                             label_selector=FirewallResourceLabelSelector(
-                                selector=a["label_selector"]["selector"]
+                                selector=firewall_resource["label_selector"]["selector"]
                             ),
                         )
                     )
-            data["applied_to"] = ats
+            data["applied_to"] = data_applied_to
 
         super().__init__(client, data, complete)
 

hcloud/firewalls/domain.py

@@ -108,6 +108,9 @@ def __init__(
         self.description = description
 
     def to_payload(self) -> dict[str, Any]:
+        """
+        Generates the request payload from this domain object.
+        """
         payload: dict[str, Any] = {
             "direction": self.direction,
             "protocol": self.protocol,
@@ -151,6 +154,9 @@ def __init__(
         self.label_selector = label_selector
 
     def to_payload(self) -> dict[str, Any]:
+        """
+        Generates the request payload from this domain object.
+        """
         payload: dict[str, Any] = {"type": self.type}
         if self.server is not None:
             payload["server"] = {"id": self.server.id}

hcloud/floating_ips/client.py

@@ -19,6 +19,7 @@ class BoundFloatingIP(BoundModelBase):
     model = FloatingIP
 
     def __init__(self, client: FloatingIPsClient, data: dict, complete: bool = True):
+        # pylint: disable=import-outside-toplevel
         from ..servers import BoundServer
 
         server = data.get("server")

hcloud/hcloud.py

@@ -8,4 +8,5 @@
     stacklevel=2,
 )
 
+# pylint: disable=wildcard-import,wrong-import-position,unused-wildcard-import
 from ._client import *  # noqa

hcloud/helpers/labels.py

@@ -18,10 +18,10 @@ def validate(labels: dict[str, str]) -> bool:
 
         :return:  bool
         """
-        for k, v in labels.items():
-            if LabelValidator.KEY_REGEX.match(k) is None:
+        for key, value in labels.items():
+            if LabelValidator.KEY_REGEX.match(key) is None:
                 return False
-            if LabelValidator.VALUE_REGEX.match(v) is None:
+            if LabelValidator.VALUE_REGEX.match(value) is None:
                 return False
         return True
 
@@ -32,9 +32,15 @@ def validate_verbose(labels: dict[str, str]) -> tuple[bool, str]:
 
         :return:  bool, str
         """
-        for k, v in labels.items():
-            if LabelValidator.KEY_REGEX.match(k) is None:
-                return False, f"label key {k} is not correctly formatted"
-            if LabelValidator.VALUE_REGEX.match(v) is None:
-                return False, f"label value {v} (key: {k}) is not correctly formatted"
+        for key, value in labels.items():
+            if LabelValidator.KEY_REGEX.match(key) is None:
+                return (
+                    False,
+                    f"label key {key} is not correctly formatted",
+                )
+            if LabelValidator.VALUE_REGEX.match(value) is None:
+                return (
+                    False,
+                    f"label value {value} (key: {key}) is not correctly formatted",
+                )
         return True, ""

hcloud/images/client.py

@@ -16,6 +16,7 @@ class BoundImage(BoundModelBase):
     model = Image
 
     def __init__(self, client: ImagesClient, data: dict):
+        # pylint: disable=import-outside-toplevel
         from ..servers import BoundServer
 
         created_from = data.get("created_from")

hcloud/images/domain.py

@@ -71,6 +71,7 @@ class Image(BaseDomain, DomainIdentityMixin):
         "deprecated",
     )
 
+    # pylint: disable=too-many-locals
     def __init__(
         self,
         id: int | None = None,

hcloud/load_balancers/client.py

@@ -39,6 +39,7 @@ class BoundLoadBalancer(BoundModelBase):
 
     model = LoadBalancer
 
+    # pylint: disable=too-many-branches,too-many-locals
     def __init__(self, client: LoadBalancersClient, data: dict, complete: bool = True):
         algorithm = data.get("algorithm")
         if algorithm:

hcloud/load_balancers/domain.py

@@ -69,6 +69,7 @@ class LoadBalancer(BaseDomain):
         "included_traffic",
     )
 
+    # pylint: disable=too-many-locals
     def __init__(
         self,
         id: int,
@@ -137,7 +138,11 @@ def __init__(
         self.health_check = health_check
         self.http = http
 
+    # pylint: disable=too-many-branches
     def to_payload(self) -> dict[str, Any]:
+        """
+        Generates the request payload from this domain object.
+        """
         payload: dict[str, Any] = {}
 
         if self.protocol is not None:
@@ -334,6 +339,9 @@ def __init__(
         self.health_status = health_status
 
     def to_payload(self) -> dict[str, Any]:
+        """
+        Generates the request payload from this domain object.
+        """
         payload: dict[str, Any] = {
             "type": self.type,
         }

hcloud/networks/client.py

@@ -26,6 +26,7 @@ def __init__(self, client: NetworksClient, data: dict, complete: bool = True):
             routes = [NetworkRoute.from_dict(route) for route in routes]
             data["routes"] = routes
 
+        # pylint: disable=import-outside-toplevel
         from ..servers import BoundServer
 
         servers = data.get("servers", [])

hcloud/primary_ips/client.py

@@ -17,6 +17,7 @@ class BoundPrimaryIP(BoundModelBase):
     model = PrimaryIP
 
     def __init__(self, client: PrimaryIPsClient, data: dict, complete: bool = True):
+        # pylint: disable=import-outside-toplevel
         from ..datacenters import BoundDatacenter
 
         datacenter = data.get("datacenter", {})

hcloud/servers/client.py

@@ -47,6 +47,7 @@ class BoundServer(BoundModelBase):
 
     model = Server
 
+    # pylint: disable=too-many-locals
     def __init__(self, client: ServersClient, data: dict, complete: bool = True):
         datacenter = data.get("datacenter")
         if datacenter is not None:
@@ -540,6 +541,7 @@ def get_by_name(self, name: str) -> BoundServer | None:
         """
         return self._get_first_by(name=name)
 
+    # pylint: disable=too-many-branches,too-many-locals
     def create(
         self,
         name: str,

hcloud/servers/domain.py

@@ -104,6 +104,7 @@ class Server(BaseDomain):
         "placement_group",
     )
 
+    # pylint: disable=too-many-locals
     def __init__(
         self,
         id: int,

hcloud/volumes/client.py

@@ -23,6 +23,7 @@ def __init__(self, client: VolumesClient, data: dict, complete: bool = True):
         if location is not None:
             data["location"] = BoundLocation(client._client.locations, location)
 
+        # pylint: disable=import-outside-toplevel
         from ..servers import BoundServer
 
         server = data.get("server")
@@ -254,7 +255,7 @@ def create(
         if size <= 0:
             raise ValueError("size must be greater than 0")
 
-        if not (bool(location) ^ bool(server)):
+        if not bool(location) ^ bool(server):
             raise ValueError("only one of server or location must be provided")
 
         data: dict[str, Any] = {"name": name, "size": size}

pyproject.toml

@@ -12,3 +12,29 @@ source = ["hcloud"]
 [build-system]
 requires = ["setuptools"]
 build-backend = "setuptools.build_meta"
+
+[tool.pylint.main]
+py-version = "3.8"
+recursive = true
+jobs = 0
+
+[tool.pylint.reports]
+output-format = "colorized"
+
+[tool.pylint.basic]
+good-names = ["i", "j", "k", "ex", "_", "ip", "id"]
+
+[tool.pylint."messages control"]
+disable = [
+  "fixme",
+  "line-too-long",
+  "missing-class-docstring",
+  "missing-module-docstring",
+  "redefined-builtin",
+  # Consider disabling line-by-line
+  "too-few-public-methods",
+  "too-many-public-methods",
+  "too-many-arguments",
+  "too-many-instance-attributes",
+  "too-many-lines",
+]

setup.py

@@ -51,6 +51,7 @@
         ],
         "test": [
             "coverage>=7.3,<7.4",
+            "pylint>=2.17.4,<2.18",
             "pytest>=7.4,<7.5",
             "mypy>=1.5,<1.6",
             "types-python-dateutil",