Преглед на файлове

Reconcile interface return types with implementations

Typing the providers surfaced that several interface return types (derived
from docstrings) did not match what every implementation actually returns.
Correct them so the public API is accurate and providers/base type cleanly
without # type: ignore[override]. Verified per-case across base + all four
providers (aws/azure/gcp/openstack).

resources.py:
- Widen genuinely-optional getters to `... | None`: LabeledCloudResource.label,
  MachineImage.description, MachineImage.min_disk, VMFirewallRule.cidr /
  src_dest_fw_id / src_dest_fw, DnsZone.admin_email.
- Fix data shapes: DnsRecord.data -> list[str] (all providers return a list,
  not str); Volume.attachments -> AttachmentInfo | None (every provider returns
  a single object or None, never a list).
- bool -> None for actions that return nothing in every impl: delete (all
  resources), Instance.reboot, Volume.attach/detach, Router.attach_subnet/
  detach_subnet/attach_gateway/detach_gateway, ObjectLifeCycleMixin.
  wait_till_ready (it calls wait_for but does not return it; wait_for stays
  -> bool since it really returns True).
- Instance.start/stop -> bool | None (AWS returns a success bool, other
  providers return None; honest cross-provider type).
- BucketObject.upload / upload_from_file -> BucketObject | None (the base
  implementation is authoritative; providers were inconsistent).
- Left CloudResource.name and .id as -> str (the only None paths are
  error/degenerate branches, not the normal contract).

services.py: KeyPairService.delete -> None (AWS impl returns None; it was the
only *Service action still declared -> bool).

base/resources.py: drop the 16 now-unnecessary # type: ignore[override]
comments that the interface corrections make redundant (kept one for
BaseBucket.delete, a real signature-arity mismatch with the interface).
Nuwan Goonasekera преди 1 ден
родител
ревизия
3799d2a2d5
променени са 3 файла, в които са добавени 52 реда и са изтрити 94 реда
  1. 20 62
      cloudbridge/base/resources.py
  2. 31 31
      cloudbridge/interfaces/resources.py
  3. 1 1
      cloudbridge/interfaces/services.py

+ 20 - 62
cloudbridge/base/resources.py

@@ -344,10 +344,7 @@ class BaseInstance(BaseCloudResource, BaseObjectLifeCycleMixin, Instance):
                 self.private_ips == other.private_ips and
                 self.image_id == other.image_id)
 
-    # NB: interface declares -> bool, but neither base nor provider
-    # implementations return a value (wait_for raises on failure); annotate
-    # honestly as -> None without altering behavior.
-    def wait_till_ready(  # type: ignore[override]
+    def wait_till_ready(
             self, timeout: int | None = None,
             interval: int | None = None) -> None:
         self.wait_for(
@@ -458,10 +455,7 @@ class BaseMachineImage(
                 self.label == other.label and
                 self.description == other.description)
 
-    # NB: interface declares -> bool but the implementation does not return a
-    # value (wait_for raises on failure); annotate honestly without altering
-    # behavior.
-    def wait_till_ready(  # type: ignore[override]
+    def wait_till_ready(
             self, timeout: int | None = None,
             interval: int | None = None) -> None:
         self.wait_for(
@@ -505,10 +499,7 @@ class BaseVolume(BaseCloudResource, BaseObjectLifeCycleMixin, Volume):
                 self.state == other.state and
                 self.label == other.label)
 
-    # NB: interface declares -> bool but the implementation does not return a
-    # value (wait_for raises on failure); annotate honestly without altering
-    # behavior.
-    def wait_till_ready(  # type: ignore[override]
+    def wait_till_ready(
             self, timeout: int | None = None,
             interval: int | None = None) -> None:
         self.wait_for(
@@ -517,9 +508,7 @@ class BaseVolume(BaseCloudResource, BaseObjectLifeCycleMixin, Volume):
             timeout=timeout,
             interval=interval)
 
-    # NB: interface declares -> bool but VolumeService.delete returns None;
-    # annotate honestly without altering behavior.
-    def delete(self) -> None:  # type: ignore[override]
+    def delete(self) -> None:
         """
         Delete this volume.
         """
@@ -540,10 +529,7 @@ class BaseSnapshot(BaseCloudResource, BaseObjectLifeCycleMixin, Snapshot):
                 self.state == other.state and
                 self.label == other.label)
 
-    # NB: interface declares -> bool but the implementation does not return a
-    # value (wait_for raises on failure); annotate honestly without altering
-    # behavior.
-    def wait_till_ready(  # type: ignore[override]
+    def wait_till_ready(
             self, timeout: int | None = None,
             interval: int | None = None) -> None:
         self.wait_for(
@@ -552,9 +538,7 @@ class BaseSnapshot(BaseCloudResource, BaseObjectLifeCycleMixin, Snapshot):
             timeout=timeout,
             interval=interval)
 
-    # NB: interface declares -> bool but SnapshotService.delete returns None;
-    # annotate honestly without altering behavior.
-    def delete(self) -> None:  # type: ignore[override]
+    def delete(self) -> None:
         """
         Delete this snapshot.
         """
@@ -597,9 +581,7 @@ class BaseKeyPair(BaseCloudResource, KeyPair):
     def material(self, value: str | None) -> None:
         self._private_material = value
 
-    # NB: interface declares -> bool but the implementation does not return a
-    # value; annotate honestly without altering behavior.
-    def delete(self) -> None:  # type: ignore[override]
+    def delete(self) -> None:
         self._provider.security.key_pairs.delete(self)
 
 
@@ -954,18 +936,14 @@ class BaseBucketObject(BaseCloudResource, BucketObject):
             return io.BytesIO(data)
         return data
 
-    # NB: interface declares -> bool, but the base path returns the completed
-    # BucketObject (multipart) or None (single-shot); annotate honestly.
-    def upload(self, data: str | bytes | IO[bytes],  # type: ignore[override]
+    def upload(self, data: str | bytes | IO[bytes],
                config: UploadConfig | None = None) -> BucketObject | None:
         size = self._data_size(data)
         if size is not None and size > self._multipart_threshold(config):
             return self._upload_multipart(self._as_stream(data), config)
         return self._upload_single_shot(data)
 
-    # NB: interface declares -> None, but the base path returns the completed
-    # BucketObject for the multipart branch; annotate honestly.
-    def upload_from_file(  # type: ignore[override]
+    def upload_from_file(
             self, path: str,
             config: UploadConfig | None = None) -> BucketObject | None:
         if os.path.getsize(path) > self._multipart_threshold(config):
@@ -1116,9 +1094,9 @@ class BaseBucket(BaseCloudResource, Bucket):
                 # check from most to least likely mutables
                 self.name == other.name)
 
-    # NB: interface declares delete(delete_contents=False) -> bool, but this
-    # base implementation takes no args and returns None; annotate honestly
-    # without altering behavior.
+    # NB: interface declares delete(delete_contents=False), but this base
+    # implementation takes no args; the override ignore covers that signature
+    # mismatch without altering behavior.
     def delete(self) -> None:  # type: ignore[override]
         """
         Delete this bucket.
@@ -1153,10 +1131,7 @@ class BaseNetwork(BaseCloudResource, BaseObjectLifeCycleMixin, Network):
 
         return prefix1 == prefix2
 
-    # NB: interface declares -> bool but the implementation does not return a
-    # value (wait_for raises on failure); annotate honestly without altering
-    # behavior.
-    def wait_till_ready(  # type: ignore[override]
+    def wait_till_ready(
             self, timeout: int | None = None,
             interval: int | None = None) -> None:
         self.wait_for(
@@ -1165,9 +1140,7 @@ class BaseNetwork(BaseCloudResource, BaseObjectLifeCycleMixin, Network):
             timeout=timeout,
             interval=interval)
 
-    # NB: interface declares -> bool but NetworkService.delete returns None;
-    # annotate honestly without altering behavior.
-    def delete(self) -> None:  # type: ignore[override]
+    def delete(self) -> None:
         self._provider.networking.networks.delete(self)
 
     def __eq__(self, other: object) -> bool:
@@ -1200,10 +1173,7 @@ class BaseSubnet(BaseCloudResource, BaseObjectLifeCycleMixin, Subnet):
         return cast(
             Network, self._provider.networking.networks.get(self.network_id))
 
-    # NB: interface declares -> bool but the implementation does not return a
-    # value (wait_for raises on failure); annotate honestly without altering
-    # behavior.
-    def wait_till_ready(  # type: ignore[override]
+    def wait_till_ready(
             self, timeout: int | None = None,
             interval: int | None = None) -> None:
         self.wait_for(
@@ -1212,9 +1182,7 @@ class BaseSubnet(BaseCloudResource, BaseObjectLifeCycleMixin, Subnet):
             timeout=timeout,
             interval=interval)
 
-    # NB: interface declares -> bool but SubnetService.delete returns None;
-    # annotate honestly without altering behavior.
-    def delete(self) -> None:  # type: ignore[override]
+    def delete(self) -> None:
         self._provider.networking.subnets.delete(self)
 
 
@@ -1232,10 +1200,7 @@ class BaseFloatingIP(BaseCloudResource, BaseObjectLifeCycleMixin, FloatingIP):
         return (FloatingIpState.IN_USE if self.in_use
                 else FloatingIpState.AVAILABLE)
 
-    # NB: interface declares -> bool but the implementation does not return a
-    # value (wait_for raises on failure); annotate honestly without altering
-    # behavior.
-    def wait_till_ready(  # type: ignore[override]
+    def wait_till_ready(
             self, timeout: int | None = None,
             interval: int | None = None) -> None:
         self.wait_for(
@@ -1250,9 +1215,7 @@ class BaseFloatingIP(BaseCloudResource, BaseObjectLifeCycleMixin, FloatingIP):
                 self._provider == other._provider and
                 self.id == other.id)
 
-    # NB: interface declares -> bool but FloatingIPService.delete returns None;
-    # annotate honestly without altering behavior.
-    def delete(self) -> None:  # type: ignore[override]
+    def delete(self) -> None:
         # For OS where the gateway is necessary, we pass the gateway when
         # deleting, for all others we pass None and it will be ignored
         gw: Any = getattr(self, '_gateway_id', None)
@@ -1273,9 +1236,7 @@ class BaseRouter(BaseCloudResource, Router):
                 self._provider == other._provider and
                 self.id == other.id)
 
-    # NB: interface declares -> bool but RouterService.delete returns None;
-    # annotate honestly without altering behavior.
-    def delete(self) -> None:  # type: ignore[override]
+    def delete(self) -> None:
         self._provider.networking.routers.delete(self)
 
 
@@ -1294,10 +1255,7 @@ class BaseInternetGateway(BaseCloudResource, BaseObjectLifeCycleMixin,
                 self._provider == other._provider and
                 self.id == other.id)
 
-    # NB: interface declares -> bool but the implementation does not return a
-    # value (wait_for raises on failure); annotate honestly without altering
-    # behavior.
-    def wait_till_ready(  # type: ignore[override]
+    def wait_till_ready(
             self, timeout: int | None = None,
             interval: int | None = None) -> None:
         self.wait_for(

+ 31 - 31
cloudbridge/interfaces/resources.py

@@ -137,7 +137,7 @@ class CloudResource(object):
 class LabeledCloudResource(CloudResource):
 
     @abstractproperty
-    def label(self) -> str:
+    def label(self) -> str | None:
         """
         Get the resource label.
 
@@ -326,7 +326,7 @@ class ObjectLifeCycleMixin(object):
 
     @abstractmethod
     def wait_till_ready(self, timeout: int | None = None,
-                        interval: int | None = None) -> bool:
+                        interval: int | None = None) -> None:
         """
         Wait till the current object reaches its ready state.
 
@@ -600,7 +600,7 @@ class Instance(ObjectLifeCycleMixin, LabeledCloudResource):
         pass
 
     @abstractmethod
-    def reboot(self) -> bool:
+    def reboot(self) -> None:
         """
         Reboot this instance (using the cloud middleware API).
 
@@ -610,7 +610,7 @@ class Instance(ObjectLifeCycleMixin, LabeledCloudResource):
         pass
 
     @abstractmethod
-    def start(self) -> bool:
+    def start(self) -> bool | None:
         """
         Start this instance (using the cloud middleware API)
 
@@ -620,7 +620,7 @@ class Instance(ObjectLifeCycleMixin, LabeledCloudResource):
         pass
 
     @abstractmethod
-    def stop(self) -> bool:
+    def stop(self) -> bool | None:
         """
         Stop this instance (using the cloud middleware API)
 
@@ -899,7 +899,7 @@ class MachineImage(ObjectLifeCycleMixin, LabeledCloudResource):
     __metaclass__ = ABCMeta
 
     @abstractproperty
-    def description(self) -> str:
+    def description(self) -> str | None:
         """
         Get the image description.
 
@@ -910,7 +910,7 @@ class MachineImage(ObjectLifeCycleMixin, LabeledCloudResource):
         pass
 
     @abstractproperty
-    def min_disk(self) -> int:
+    def min_disk(self) -> int | None:
         """
         Return the minimum size of the disk that's required to boot this image.
 
@@ -922,7 +922,7 @@ class MachineImage(ObjectLifeCycleMixin, LabeledCloudResource):
         pass
 
     @abstractmethod
-    def delete(self) -> bool:
+    def delete(self) -> None:
         """
         Delete this image.
 
@@ -1001,7 +1001,7 @@ class Network(ObjectLifeCycleMixin, LabeledCloudResource):
         pass
 
     @abstractmethod
-    def delete(self) -> bool:
+    def delete(self) -> None:
         """
         Delete this network.
 
@@ -1108,7 +1108,7 @@ class Subnet(ObjectLifeCycleMixin, LabeledCloudResource):
         pass
 
     @abstractmethod
-    def delete(self) -> bool:
+    def delete(self) -> None:
         """
         Delete this subnet.
 
@@ -1171,7 +1171,7 @@ class FloatingIP(ObjectLifeCycleMixin, CloudResource):
         pass
 
     @abstractmethod
-    def delete(self) -> bool:
+    def delete(self) -> None:
         """
         Delete this address.
 
@@ -1238,7 +1238,7 @@ class Router(LabeledCloudResource):
         pass
 
     @abstractmethod
-    def delete(self) -> bool:
+    def delete(self) -> None:
         """
         Delete this router.
 
@@ -1248,7 +1248,7 @@ class Router(LabeledCloudResource):
         pass
 
     @abstractmethod
-    def attach_subnet(self, subnet: Subnet | str) -> bool:
+    def attach_subnet(self, subnet: Subnet | str) -> None:
         """
         Attach this router to a subnet.
 
@@ -1261,7 +1261,7 @@ class Router(LabeledCloudResource):
         pass
 
     @abstractmethod
-    def detach_subnet(self, subnet: Subnet | str) -> bool:
+    def detach_subnet(self, subnet: Subnet | str) -> None:
         """
         Detach this subnet from a network.
 
@@ -1284,7 +1284,7 @@ class Router(LabeledCloudResource):
         pass
 
     @abstractmethod
-    def attach_gateway(self, gateway: Gateway) -> bool:
+    def attach_gateway(self, gateway: Gateway) -> None:
         """
         Attach a gateway to this router.
 
@@ -1297,7 +1297,7 @@ class Router(LabeledCloudResource):
         pass
 
     @abstractmethod
-    def detach_gateway(self, gateway: Gateway) -> bool:
+    def detach_gateway(self, gateway: Gateway) -> None:
         """
         Detach this router from a gateway.
 
@@ -1375,7 +1375,7 @@ class DnsZone(CloudResource):
     __metaclass__ = ABCMeta
 
     @abstractproperty
-    def admin_email(self) -> str:
+    def admin_email(self) -> str | None:
         """
         Email address of this zone's administrator. Some cloud providers do not
         support this field, and therefore, it may be stored in an extra field
@@ -1450,7 +1450,7 @@ class DnsRecord(CloudResource):
         pass
 
     @abstractproperty
-    def data(self) -> str:
+    def data(self) -> list[str]:
         """
         Dns Record data
 
@@ -1615,7 +1615,7 @@ class Volume(ObjectLifeCycleMixin, LabeledCloudResource):
         pass
 
     @abstractproperty
-    def attachments(self) -> list[AttachmentInfo]:
+    def attachments(self) -> AttachmentInfo | None:
         """
         Get attachment information for this volume.
 
@@ -1625,7 +1625,7 @@ class Volume(ObjectLifeCycleMixin, LabeledCloudResource):
         pass
 
     @abstractmethod
-    def attach(self, instance: str | Instance, device: str) -> bool:
+    def attach(self, instance: str | Instance, device: str) -> None:
         """
         Attach this volume to an instance.
 
@@ -1643,7 +1643,7 @@ class Volume(ObjectLifeCycleMixin, LabeledCloudResource):
         pass
 
     @abstractmethod
-    def detach(self, force: bool = False) -> bool:
+    def detach(self, force: bool = False) -> None:
         """
         Detach this volume from an instance.
 
@@ -1682,7 +1682,7 @@ class Volume(ObjectLifeCycleMixin, LabeledCloudResource):
         pass
 
     @abstractmethod
-    def delete(self) -> bool:
+    def delete(self) -> None:
         """
         Delete this volume.
 
@@ -1841,7 +1841,7 @@ class Snapshot(ObjectLifeCycleMixin, LabeledCloudResource):
 #         pass
 
     @abstractmethod
-    def delete(self) -> bool:
+    def delete(self) -> None:
         """
         Delete this snapshot.
 
@@ -1869,7 +1869,7 @@ class KeyPair(CloudResource):
         pass
 
     @abstractmethod
-    def delete(self) -> bool:
+    def delete(self) -> None:
         """
         Delete this key pair.
 
@@ -2118,7 +2118,7 @@ class VMFirewallRule(CloudResource):
         pass
 
     @abstractproperty
-    def cidr(self) -> str:
+    def cidr(self) -> str | None:
         """
         CIDR block this VM firewall is providing access to.
 
@@ -2128,7 +2128,7 @@ class VMFirewallRule(CloudResource):
         pass
 
     @abstractproperty
-    def src_dest_fw_id(self) -> str:
+    def src_dest_fw_id(self) -> str | None:
         """
         VM firewall id given access permissions by this rule.
 
@@ -2138,7 +2138,7 @@ class VMFirewallRule(CloudResource):
         pass
 
     @abstractproperty
-    def src_dest_fw(self) -> VMFirewall:
+    def src_dest_fw(self) -> VMFirewall | None:
         """
         VM firewall given access permissions by this rule.
 
@@ -2364,7 +2364,7 @@ class BucketObject(CloudResource):
 
     @abstractmethod
     def upload(self, source_stream: IO[bytes],
-               config: UploadConfig | None = None) -> bool:
+               config: UploadConfig | None = None) -> BucketObject | None:
         """
         Set the contents of the object to the data read from the source stream.
 
@@ -2380,7 +2380,7 @@ class BucketObject(CloudResource):
 
     @abstractmethod
     def upload_from_file(self, path: str,
-                         config: UploadConfig | None = None) -> None:
+                         config: UploadConfig | None = None) -> BucketObject | None:
         """
         Store the contents of the file pointed by the "path" variable.
 
@@ -2419,7 +2419,7 @@ class BucketObject(CloudResource):
         pass
 
     @abstractmethod
-    def delete(self) -> bool:
+    def delete(self) -> None:
         """
         Delete this object.
 
@@ -2504,7 +2504,7 @@ class Bucket(CloudResource):
         pass
 
     @abstractmethod
-    def delete(self, delete_contents: bool = False) -> bool:
+    def delete(self, delete_contents: bool = False) -> None:
         """
         Delete this bucket.
 

+ 1 - 1
cloudbridge/interfaces/services.py

@@ -1500,7 +1500,7 @@ class KeyPairService(PageableObjectMixin[KeyPair], CloudService):
         pass
 
     @abstractmethod
-    def delete(self, key_pair: KeyPair | str) -> bool:
+    def delete(self, key_pair: KeyPair | str) -> None:
         """
         Delete an existing keypair.