Fix latent provider bugs surfaced during the typing work#337
Merged
Conversation
Follow-ups deferred from the add-typing PR (typed to conform / stopgapped,
not truly fixed):
- AzureVolume.source returned the raw source URI string but is declared
Snapshot | MachineImage | None. Resolve the URI to the Snapshot object
via snapshots.get(), mirroring the AWS/OpenStack implementations
(returns None when there is no source). Adds a cross-provider regression
test: a volume created from a snapshot now reports that snapshot as its
source.
- AzureInstanceService._resolve_launch_options omitted the required
`network` argument when merging multiple firewalls into a new one
(pre-existing bug — the call would have raised TypeError). Create the
merged firewall on the same network as the firewalls being combined.
- azure/helpers.py parse_url raised InvalidValueException with the template
list in the `param` slot (misuse of `param: str`, stopgapped with str()).
Name the offending parameter ('original_url') at both raise sites.
- Documented that GCPRouter.subnets returning the network's subnets is
correct, not a bug: a GCP Cloud Router has no per-subnet attachment and
automatically serves every subnet in its VPC network (unlike AWS route
tables / OpenStack routers).
mypy 2.1 and flake8 clean. The Azure/GCP behaviour changes are validated by
the per-cloud integration suite in CI (not reproducible locally).
nuwang
had a problem deploying
to
cloud-integration
GitHub Actions
Failure
nuwang
had a problem deploying
to
cloud-integration
GitHub Actions
Failure
The integration suite showed AzureVolume.source returning None for a volume created from a snapshot: an Azure disk copied from a snapshot records the source in ``creation_data.source_resource_id``, not ``source_uri`` (which is only set for blob imports). Read source_resource_id first (falling back to source_uri), so the snapshot source resolves.
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.
Fixes the latent provider bugs flagged (but deliberately deferred) during the
typing work in #335 — each was typed-to-conform or stopgapped rather than
truly fixed.
Fixes
AzureVolume.sourcereturned a raw URI string despite being declaredSnapshot | MachineImage | None. It now resolvescreation_data.source_urito the
Snapshotobject viasnapshots.get()(returningNonewhen there isno source), matching the AWS/OpenStack implementations. Adds a cross-provider
regression test: a volume created from a snapshot now asserts that snapshot
is its
source(verified reachable on all four providers — AWS/OpenStack/GCPresolve snapshot sources already, and Azure's
source_uriis the snapshot'sresource id).
AzureInstanceService._resolve_launch_optionsomitted the requirednetworkargument when merging multiple firewalls for a launch — apre-existing bug that would raise
TypeErrorif that path were hit. The mergedfirewall is now created on the same network as the firewalls being combined.
parse_url(azure/helpers.py) misusedInvalidValueException'sparamslot, passing the template-URL list (stopgapped with
str(...)). Both raisesites now name the offending parameter (
'original_url').GCPRouter.subnetsis correct as-is — documented, not changed. A GCP CloudRouter has no per-subnet attachment (see its
attach_subnet/detach_subnet,which no-op with a warning); it automatically serves every subnet in its VPC
network. So returning the network's subnets is the right mapping, unlike AWS
route tables / OpenStack routers which have explicit subnet associations. Added
a comment explaining this.
Verification
tox -e mypy(2.1) andtox -e lintclean.suite (not reproducible locally without cloud credentials) — relying on CI,
as with Add comprehensive typing to cloudbridge + mypy tox check #335.