Reconcile remaining cross-provider inconsistencies surfaced by typing

Guiding principle: the interface should be abstract enough that every
implementation conforms to it -- so fix the implementations rather than widen
the interface or paper over with casts/ignores.

- Instance.start/stop -> None (was bool | None). Drop AWS's success-bool return
  so the abstract action contract matches every provider (azure/gcp already
  return None).
- BucketObject.upload / upload_from_file -> BucketObject (was BucketObject |
  None). Conform base (_upload_single_shot/_upload_from_file_single_shot) and
  AWS to return the BucketObject the content was written to.
- AWSImageService.find / AWSSnapshotService.find now return a
  ClientPagedResultList like every other find(), instead of a plain list cast
  to ResultList.
- Two latent AWS bugs fixed (not preserved): AWSRouter.attach_gateway /
  detach_gateway now return None as the interface declares (the impls no longer
  return boto3's bool/dict); AWSGatewayService.delete logs gw.id, not self.id
  (a service has no id).
- Bucket.delete(delete_contents=False): add the param to BaseBucket.delete to
  match the interface, AND honor it -- when True, delete the bucket's objects
  first (no provider overrides delete, so this fixes the previously-broken
  documented flag everywhere).

Verified: mypy strict green (43 files), flake8 clean, mock suite 97 passed.

cloudbridge/base/resources.py

@@ -846,7 +846,7 @@ class BaseBucketObject(BaseCloudResource, BucketObject):
             "BucketObject subclasses must implement the bucket property")
 
     def _upload_single_shot(
-            self, data: str | bytes | IO[bytes]) -> BucketObject | None:
+            self, data: str | bytes | IO[bytes]) -> BucketObject:
         # Provider-implemented single-shot (non-multipart) upload.
         raise NotImplementedError(
             "BucketObject subclasses must implement _upload_single_shot")
@@ -938,7 +938,7 @@ class BaseBucketObject(BaseCloudResource, BucketObject):
         return data
 
     def upload(self, data: str | bytes | IO[bytes],
-               config: UploadConfig | None = None) -> BucketObject | None:
+               config: UploadConfig | None = None) -> BucketObject:
         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)
@@ -946,7 +946,7 @@ class BaseBucketObject(BaseCloudResource, BucketObject):
 
     def upload_from_file(
             self, path: str,
-            config: UploadConfig | None = None) -> BucketObject | None:
+            config: UploadConfig | None = None) -> BucketObject:
         if os.path.getsize(path) > self._multipart_threshold(config):
             with open(path, 'rb') as f:
                 return self._upload_multipart(f, config)
@@ -1059,7 +1059,7 @@ class BaseBucketObject(BaseCloudResource, BucketObject):
         return bytes(buffer)
 
     def _upload_from_file_single_shot(
-            self, path: str) -> BucketObject | None:
+            self, path: str) -> BucketObject:
         """
         Default small-file upload: read the file and hand it to the provider's
         single-shot upload. Providers with a more efficient native file upload
@@ -1095,13 +1095,13 @@ class BaseBucket(BaseCloudResource, Bucket):
                 # check from most to least likely mutables
                 self.name == other.name)
 
-    # 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]
+    def delete(self, delete_contents: bool = False) -> None:
         """
         Delete this bucket.
         """
+        if delete_contents:
+            for obj in self.objects:
+                obj.delete()
         # BucketService.delete is implemented by every provider but is not
         # declared on the public typed interface, hence the ignore.
         self._provider.storage.buckets.delete(self.id)  # type: ignore[attr-defined]

cloudbridge/interfaces/resources.py

@@ -610,22 +610,16 @@ class Instance(ObjectLifeCycleMixin, LabeledCloudResource):
         pass
 
     @abstractmethod
-    def start(self) -> bool | None:
+    def start(self) -> None:
         """
         Start this instance (using the cloud middleware API)
-
-        :rtype: ``bool``
-        :return: ``True`` if the starting was successful; ``False`` otherwise.
         """
         pass
 
     @abstractmethod
-    def stop(self) -> bool | None:
+    def stop(self) -> None:
         """
         Stop this instance (using the cloud middleware API)
-
-        :rtype: ``bool``
-        :return: ``True`` if the stopping was successful; ``False`` otherwise.
         """
         pass
 

cloudbridge/providers/aws/resources.py

@@ -41,6 +41,7 @@ from cloudbridge.base.resources import BaseVMFirewallRule
 from cloudbridge.base.resources import BaseVMType
 from cloudbridge.base.resources import BaseVolume
 from cloudbridge.interfaces.resources import AttachmentInfo
+from cloudbridge.interfaces.resources import BucketObject
 from cloudbridge.interfaces.resources import FloatingIP
 from cloudbridge.interfaces.resources import Gateway
 from cloudbridge.interfaces.resources import GatewayState
@@ -343,21 +344,11 @@ class AWSInstance(BaseInstance):
     def reboot(self) -> None:
         self._ec2_instance.reboot()
 
-    def start(self) -> bool:
-        response = self._ec2_instance.start()
-        states = ['pending', 'running']
-        if response['StartingInstances'][0]['CurrentState']['Name'] in states:
-            return True
-        else:
-            return False
+    def start(self) -> None:
+        self._ec2_instance.start()
 
-    def stop(self) -> bool:
-        response = self._ec2_instance.stop()
-        states = ['stopping', 'stopped']
-        if response['StoppingInstances'][0]['CurrentState']['Name'] in states:
-            return True
-        else:
-            return False
+    def stop(self) -> None:
+        self._ec2_instance.stop()
 
     @property
     def image_id(self) -> str:
@@ -925,15 +916,14 @@ class AWSBucketObject(BaseBucketObject):
     def iter_content(self) -> Iterable[bytes]:
         return self.BucketObjIterator(self._obj.get().get('Body'))
 
-    def _upload_single_shot(self, data: str | bytes | IO[bytes]) -> None:
+    def _upload_single_shot(self,
+                            data: str | bytes | IO[bytes]) -> BucketObject:
         self._obj.put(Body=data)
+        return self
 
-    # The base driver returns the completed BucketObject; boto3's
-    # upload_fileobj performs the multipart upload in place and returns
-    # nothing, so this override returns None.
-    def _upload_multipart(  # type: ignore[override]
+    def _upload_multipart(
             self, stream: IO[bytes],
-            config: UploadConfig | None = None) -> None:
+            config: UploadConfig | None = None) -> BucketObject:
         # boto3's TransferManager uploads parts concurrently with a thread-safe
         # client, so the transparent multipart path delegates to it rather than
         # CloudBridge's generic clone-pool driver.
@@ -942,9 +932,10 @@ class AWSBucketObject(BaseBucketObject):
             multipart_chunksize=self._multipart_part_size(config),
             max_concurrency=self._multipart_max_concurrency(config))
         self._obj.upload_fileobj(stream, Config=transfer_config)
+        return self
 
     def upload_from_file(self, path: str,
-                         config: UploadConfig | None = None) -> None:
+                         config: UploadConfig | None = None) -> BucketObject:
         # boto3's upload_file streams large files in parts via its
         # TransferManager. Drive it with CloudBridge's multipart knobs so that
         # upload_from_file and upload() honour the same configuration rather
@@ -954,6 +945,7 @@ class AWSBucketObject(BaseBucketObject):
             multipart_chunksize=self._multipart_part_size(config),
             max_concurrency=self._multipart_max_concurrency(config))
         self._obj.upload_file(path, Config=transfer_config)
+        return self
 
     def delete(self) -> None:
         self._obj.delete()
@@ -1289,23 +1281,16 @@ class AWSRouter(BaseRouter):
         return [AWSSubnet(cast("AWSCloudProvider", self._provider), rta.subnet)
                 for rta in self._route_table.associations if rta.subnet]
 
-    # AWS returns a bool, but the Router interface declares attach_gateway as
-    # returning None; preserve the AWS-specific bool return.
-    def attach_gateway(self, gateway: Gateway) -> bool:  # type: ignore[override]
+    def attach_gateway(self, gateway: Gateway) -> None:
         gw_id = (gateway.id if isinstance(gateway, AWSInternetGateway)
                  else gateway)
-        if self._route_table.create_route(
-                DestinationCidrBlock='0.0.0.0/0', GatewayId=gw_id):
-            return True
-        return False
-
-    # AWS returns the raw boto3 response dict, but the Router interface declares
-    # detach_gateway as returning None; preserve the AWS-specific dict return.
-    def detach_gateway(  # type: ignore[override]
-            self, gateway: Gateway) -> dict[str, Any]:
+        self._route_table.create_route(
+            DestinationCidrBlock='0.0.0.0/0', GatewayId=gw_id)
+
+    def detach_gateway(self, gateway: Gateway) -> None:
         gw_id = (gateway.id if isinstance(gateway, AWSInternetGateway)
                  else gateway)
-        return cast("AWSCloudProvider", self._provider).ec2_conn.meta.client \
+        cast("AWSCloudProvider", self._provider).ec2_conn.meta.client \
             .detach_internet_gateway(
                 InternetGatewayId=gw_id, VpcId=self._route_table.vpc_id)
 

cloudbridge/providers/aws/services.py

@@ -493,7 +493,7 @@ class AWSSnapshotService(BaseSnapshotService):
         # Filter by description or label
         label = kwargs.get('label', None)
 
-        obj_list = []
+        obj_list: list[Snapshot] = []
         if label:
             log.debug("Searching for AWS Snapshot with label %s", label)
             obj_list.extend(self.svc.find(filters={'tag:Name': label},
@@ -501,8 +501,8 @@ class AWSSnapshotService(BaseSnapshotService):
         else:
             obj_list = list(self)
         filters = ['label']
-        return cast("ResultList[Snapshot]",
-                    cb_helpers.generic_find(filters, kwargs, obj_list))
+        return ClientPagedResultList(
+            self.provider, cb_helpers.generic_find(filters, kwargs, obj_list))
 
     @dispatch(event="provider.storage.snapshots.list",
               priority=BaseSnapshotService.STANDARD_EVENT_PRIORITY)
@@ -800,16 +800,14 @@ class AWSImageService(BaseImageService):
 
         # The original list is made by combining both searches by "tag:Name"
         # and "AMI name" to allow for searches of public images
+        obj_list: list[MachineImage] = []
         if label:
             log.debug("Searching for AWS Image Service %s", label)
-            obj_list: list[MachineImage] = []
             obj_list.extend(
                 self.svc.find(filters={'name': label}, **extra_args))
             obj_list.extend(
                 self.svc.find(filters={'tag:Name': label}, **extra_args))
-            return cast("ResultList[MachineImage]", obj_list)
-        else:
-            return cast("ResultList[MachineImage]", [])
+        return ClientPagedResultList(self.provider, obj_list)
 
     # Intentionally extends the base list() with a leading filter_by_owner
     # parameter (matches the interface's documented arguments-differ override).
@@ -1526,10 +1524,7 @@ class AWSGatewayService(BaseGatewayService):
                 # pylint:disable=protected-access
                 gw._gateway.detach_from_vpc(VpcId=gw.network_id)
         except ClientError as e:
-            # NB: self.id is a pre-existing latent bug (a service has no id);
-            # preserved as-is for this typing pass. Only runs on an error path.
-            log.warn("Error deleting gateway {0}: {1}".format(
-                self.id, e))  # type: ignore[attr-defined]
+            log.warn("Error deleting gateway {0}: {1}".format(gw.id, e))
         # pylint:disable=protected-access
         gw._gateway.delete()