Add comprehensive typing to cloudbridge + mypy tox check#335
Merged
Conversation
Strongly type CloudBridge's public API so downstream users get a
well-typed interface, even though the internal providers remain untyped.
Because the interface layer expresses the entire return-type web
(create_provider() -> CloudProvider -> ComputeService -> InstanceService
-> ResultList[Instance] -> Instance), annotating it plus shipping a PEP
561 py.typed marker makes the whole API typed for consumers regardless of
provider internals.
- Annotate the full interface layer (interfaces/{resources,services,
subservices,provider,exceptions}.py), factory.py and __init__.py.
- Make PageableObjectMixin and ResultList generic so list()/iteration are
typed (ResultList[Instance], etc.); add `from __future__ import
annotations` and TYPE_CHECKING blocks to break import cycles.
- Ship cloudbridge/py.typed via [tool.setuptools.package-data].
- Add a strict [tool.mypy] baseline over the whole package, with base.*
and providers.* temporarily exempted (ignore_errors) as a gradual
ratchet -- remove a module from that list as it gets typed.
- Add a lean `mypy` tox env (skip_install) to envlist and run it in the
CI lint job; add mypy>=1.11 to the dev extra.
Interface behaviour is unchanged: @abstractproperty / __metaclass__ are
kept as-is (no modernization), so runtime abstractness of the untyped
providers is unaffected.
Address imprecise annotations surfaced in review: - factory: type the provider registry precisely. provider_list is now defaultdict[str, dict[str, type[CloudProvider]]] and list_providers / get_provider_class return type[CloudProvider] instead of Any. The issubclass() guard in register_provider_class narrows the registered class, so the previous cast() in get_provider_class is now redundant and removed. (register_provider_class keeps `cls: type` on purpose -- it is a filter that receives arbitrary classes and decides whether to register.) - provider: the vestigial ContainerProvider/DeploymentProvider stubs no longer use Any; create_container/delete_container -> None and deploy(target: Instance) -> None (Instance is the only concrete type the docstring names; there is no Container class). - Configuration now subclasses dict[str, Any] instead of bare dict. Enable `strict = true` (still scoped to the interface layer + factory; base/providers stay exempt via ignore_errors), with two documented opt-outs that don't fit this codebase: implicit_reexport (interfaces/ __init__.py re-exports without __all__) and disallow_untyped_decorators (cross-class property setters + the deprecation library's decorators). Note: even strict does not forbid explicit Any, so the remaining Any uses (**kwargs, dict[str, Any] for to_json/extra_data/config) are intentional.
Annotate every callable in cloudbridge/base/ (resources, services, subservices, provider, helpers, middleware). base orchestrates through the already-typed interface layer and never touches the cloud SDKs directly, so it is held to the SAME full strict bar as the public API (providers, which DO read from untyped SDK objects, will get a more lenient pragmatic tier later). mypy config: base.* is no longer exempt -- it falls under the global strict baseline. providers.* stays exempt (ignore_errors) until typed. Notable points (all verified green by mypy strict + the mock test suite, and behaviour-preserving unless noted): - The pagination helpers are now generic: BaseResultList[T], Server/Client PagedResultList[T], BasePageableObjectMixin[T]; element type propagates to data()/__iter__()/list(). Each base service subscribes the mixin with its resource type (e.g. BasePageableObjectMixin[Instance]). - _upload_single_shot() and BucketObject.bucket were always called by base but never declared; added as explicit raising stubs (providers override them) to make the implicit abstract contract type-visible. No runtime change. - BaseSubnetSubService.get now guards `if sn and ...` so a missing subnet returns None per its documented `-> Subnet | None` contract instead of raising AttributeError (forced by the optional type). - 27 `# type: ignore` mark genuine interface/implementation contract gaps: 18 [override] where interface docstrings declared bool (e.g. delete/ wait_till_ready) but implementations return None, and [attr-defined] for provider-internal members (e.g. _bucket_objects) not on the public interfaces. Candidate for a follow-up that reconciles the interface return types.
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).
Annotate every callable in cloudbridge/providers/aws/ (provider, resources,
services, helpers, subservices; ~367 callables). AWS is the first provider
typed and validates the per-provider pattern for the rest.
mypy config: providers wrap untyped cloud SDKs, so providers.aws.* gets a
pragmatic tier (disallow_untyped_defs etc. ON, but warn_return_any and
disallow_untyped_calls OFF) so reading attributes off Any-typed boto3 objects
doesn't require a cast on every getter. providers.* stays exempt
(ignore_errors) until each is typed. base/ is finalised under the full strict
baseline (the temporary disallow_untyped_calls relaxation used while annotating
base is removed).
base/resources.py: the ResultList paging helpers (BaseResultList /
ClientPagedResultList) now accept Sequence[T] instead of list[T], so a
provider passing list[AWSVMType] where list[VMType] is expected no longer trips
list invariance. Benefits every provider.
Notable AWS specifics:
- `from __future__ import annotations` in resources.py/services.py so `list[...]`
annotations don't collide with the `list` methods these classes define.
- AWS-specific provider members (ec2_conn/s3_conn/session) reached via
cast("AWSCloudProvider", self._provider) through a TYPE_CHECKING import
(avoids a runtime import cycle).
- Property getter/setter pairs reordered so the @label.setter immediately
follows the getter (an untyped @tenacity.retry method in between was severing
mypy's property link) - no behaviour change, and avoids type: ignore there.
- Two pre-existing latent bugs preserved and flagged in-place for a later fix:
AWSGatewayService.delete logs self.id (a service has no id), and
AWSRouter.detach_gateway returns a raw boto3 dict where the interface is None.
Verified: mypy strict green (43 files), flake8 clean, and the mock-provider
test suite (mock subclasses AWS) passes 97/5-skipped with no regression.
Annotate MockAWSCloudProvider (it subclasses AWSCloudProvider) and add cloudbridge.providers.mock.* to the pragmatic mypy tier alongside aws.
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.
Apply the user's decisions for inconsistencies surfaced while typing providers, following the principle "the interface is abstract enough that impls conform to it" (fix the impl, don't widen/paper-over): - VMFirewallRule.direction -> TrafficDirection (was str). The create() param and callers already use the TrafficDirection enum; AWS stored the enum at runtime anyway. Conform AWS (__init__ takes TrafficDirection; getter returns it) and drop the now-needless arg-type ignores. - VMFirewall.description -> str | None (optional metadata, like MachineImage.description). Widen interface + base; drop AWS's override ignore. - Fatal id getters stay -> str but RAISE when the value is genuinely absent rather than returning/casting None: AWSDnsZone.id raises if the HostedZone Id is missing; AWSDnsRecord.zone_id follows from it. - AWSMachineImage.name -> str: on an SDK read error, return "" (preserves the existing don't-crash behavior) instead of None.
Annotate cloudbridge/providers/openstack/ (~341 callables) and add
providers.openstack.* to the pragmatic mypy tier. Conform implementations to
the interface per the established decisions:
- Instance.start/stop -> None; Router.attach_subnet/detach_subnet -> None (drop
bool returns; honor the Subnet | str arg); BucketObject.upload/upload_from_file
return the BucketObject; BucketObject/DnsRecord/FloatingIPService.delete -> None;
ImageService.find wraps its result in ClientPagedResultList.
- VMFirewallRule.direction returns the TrafficDirection enum.
- Fatal id getters raise ProviderInternalException when absent (Instance.zone_id
/subnet_id, Router.id) instead of returning None.
- GatewaySubService.get_or_create raises when no external network is available
(was returning None against an -> InternetGateway contract).
- Remove the dead inspect.getargspec fallback in _clean_options (getargspec was
removed in Python 3.11; the project requires 3.13).
OpenStack-specific provider members (nova/neutron/swift/keystone/os_conn) are
reached via cast("OpenStackCloudProvider", self._provider) through a
TYPE_CHECKING import. Verified: mypy strict/pragmatic green (43 files), flake8
clean, modules import under the OpenStack SDK env, and the AWS mock suite still
passes 97/5-skipped.
Annotate cloudbridge/providers/gcp/ (~415 callables) and add providers.gcp.*
to the pragmatic mypy tier. Conform implementations to the corrected interface
and apply the user's decisions for the new inconsistencies GCP surfaced.
Interface/base widenings (genuinely-optional values that providers expose):
- VMFirewallRule.protocol -> str | None (a rule may cover all protocols).
- Instance.key_pair_id -> str | None (an instance may have no key pair).
- AttachmentInfo.device -> str | None (GCP disks expose no device name);
BaseAttachmentInfo accepts device: str | None.
- Router.subnets -> Iterable[Subnet] (was list[Subnet]) so a provider may
return its SubnetSubService; AWS/OpenStack lists remain valid covariant
returns.
GCP conformance:
- Required getters raise ProviderInternalException when absent
(Instance.vm_type, Instance.image_id) rather than returning None.
- create_image and all *Service.create error paths raise instead of returning
None (a create returns the resource or raises).
- Implement GCPInstance.start() (mirrors stop()); it was missing, which had
forced type: ignore[abstract] at the instantiation sites.
- start/stop/reboot/delete/attach*/detach* -> None; upload* -> BucketObject;
find() wraps in ClientPagedResultList; DnsRecord.data -> list[str];
Volume.attachments -> AttachmentInfo | None.
- GCP-specific provider members reached via cast("GCPCloudProvider", ...).
Verified: mypy green (43 files), flake8 clean, GCP modules import under the GCP
SDK env. (No AWS runtime change this round; interface/base edits are
annotation-only.)
Annotate cloudbridge/providers/azure/ (~475 callables incl. azure_client.py) and add providers.azure.* to the pragmatic mypy tier. With this, all five providers are fully typed. Azure conforms to the decided contracts (start implemented; direction -> TrafficDirection; required id getters raise; creates raise on error; upload -> BucketObject; find -> ResultList; optional getters -> | None) and the agents fixed several latent bugs surfaced by typing (AzureBucket.exists called a nonexistent self.get; log.exception(err.message) would AttributeError on a ValueError; AzureInstanceService.delete guarded the unresolved id). Make the type-check environment correct now that the providers (heavy SDK users) are typed: - tox [testenv:mypy] installs the full deps (requirements.txt) instead of skip_install. Without the cloud SDKs, mypy infers SDK values as Any and reports spurious redundant-cast / unused-ignore errors that don't occur in a real dev env. - Pin mypy>=2.1,<3 in the dev extra so CI and developers use the same mypy; mypy 2.x narrows `list[X] | list[Y]` after isinstance checks, which made five vm_firewalls casts (aws/gcp/openstack services) redundant -- those casts are removed (runtime no-ops). Verified: tox -e mypy (mypy 2.1.0, with deps) green across all 43 files; flake8 clean.
All five providers are now typed, so the gradual-typing ratchet is fully consumed: replace the `cloudbridge.providers.*` ignore_errors exemption + the per-provider pragmatic overrides with a single pragmatic-tier override over cloudbridge.providers.* (annotations required via the strict baseline; warn_return_any / disallow_untyped_calls off for the untyped-SDK boundary). Also add flake8-import-order to the CI lint env so import ordering (kept clean throughout this work) is enforced going forward, matching the dev extra. mypy strict green across all 43 files; tox -e lint and the mock suite pass.
Address review feedback on the "pattern of unnecessary casts" in base/, keeping provider-internal implementation details off the public interface: - get_env: precise @Overloads mirroring os.environ.get, replacing Any/Any. - region_name: widen interface + base to `str | None` (matches the underlying value); drop the cast. Guard the one AWS call site that assumed non-None with an explicit ProviderInternalException. - Region.default_zone: declare on the interface so zone_name can read it directly, removing the getattr + double-cast in BaseCloudProvider. - Pageable generics: bound `T` to CloudResource (interface + base) so paging code reads `.id` without casting. - _bucket_objects / _get_config_value stay OFF the public interface (they are implementation details). Instead: * declare _bucket_objects as an abstract member on BaseStorageService; * make BaseCloudResource._provider a covariant override returning BaseCloudProvider, so base internals are visible without per-call casts; * unify all four _bucket_objects access sites on one meaningful downcast to BaseStorageService (removes the Any-hop in subservices and every `# type: ignore[attr-defined]`). - InstanceService.delete / BucketService.delete: declare on the public interface for consistency with the eight other services that already do, letting Instance.delete()/Bucket.delete() drop their casts.
nuwang
had a problem deploying
to
cloud-integration
GitHub Actions
Failure
nuwang
had a problem deploying
to
cloud-integration
GitHub Actions
Failure
Two behavioral regressions surfaced by the per-cloud integration tests: - GCP VM firewall creation broke. `vm_firewalls.create` adds the implicit default-egress rule via `create_with_priority`, but that rule matches `is_dummy_rule()` and is deliberately hidden by `list()`/`find()`, so the find-after-create returns nothing. The typed refactor had turned that "not found" case from `return None` into a raise, so firewall creation (and everything depending on it) failed. Restore `create_with_priority` returning `None` for the unfindable dummy rule (its callers ignore the return); the public, interface-facing `create()` still raises for a genuinely missing rule. - `test_instance_start_stop_methods` asserted a truthy return from `stop()`/`start()`, but those now return `None` per the interface. The test already verifies the real effect via `wait_for` + state assertions, so the obsolete return-value assertions are removed. mypy 2.1 and flake8 clean. GCP/AWS integration paths are validated by CI (not reproducible locally without cloud credentials).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Adds comprehensive type annotations across cloudbridge and an automated
mypycheck (new tox env + CI), so downstream users get a strongly-typedpublic API — every object returned through the interface is typed, even
though the underlying cloud SDKs (boto3, azure-*, google-api, openstacksdk)
are not.
The interface layer already expresses the complete return-type web
(
create_provider() -> CloudProvider,.compute.instances.list() -> ResultList[Instance],Instance.reboot() -> bool, …). Typing that layer andshipping a PEP 561
py.typedmarker makes the whole API typed for usersregardless of provider internals.
Approach
interfaces/andbase/layers are held to fullmypy strict. The
providers/layer runs under a pragmatic tier(annotations required, but
warn_return_any/disallow_untyped_callsrelaxed) because it wraps untyped SDK objects.
PageableObjectMixin[T]/ResultList[T]withTbound toCloudResource, so services and result lists are typed per-element.py.typedmarker shipped via package-data (PEP 561).@abstractproperty/__metaclass__ = ABCMetaare kept as-is.Tooling / CI
tox -e mypyenv (installs deps; mypy pinned>=2.1,<3).tox -e lintnow also enforces import order (flake8-import-order).tox -e lintthentox -e mypy.Behaviour reconciliations
Typing surfaced real cross-provider inconsistencies. Following the guiding
principle that the interface should be abstract enough to match every
implementation, providers were conformed to the interface rather than the
interface widened — e.g.
delete/attach/detach/start/stopreturnNoneconsistently; firewall-ruledirectionreturns theTrafficDirectionenum; fatal missing-id paths raise
ProviderInternalExceptioninstead ofreturning
None;Router.subnetsisIterable[Subnet]; GCPInstance.startimplemented to mirror
stop. Provider-internal members (_bucket_objects,_get_config_value) are kept off the public interface and accessed throughbase-layer types.
Verification
tox -e mypy(mypy 2.1) green across all 43 typed modules.tox -e lintclean (flake8 + import order).tox -e py3.13-mockgreen (mock subclasses AWS, so this exercises theAWS + base behaviour paths).
reveal_typeonprovider.compute.instances.list()anditerated
Instancemembers resolves through the interface types.Follow-ups (separate PRs)
deprecationdependency + deprecated code.Volume.sourcesemantics, GCPRouter.subnetsscope) — tracked for adedicated fix.