Repository navigation
chore: add type annotations to DataSourceOVF - #7148
Open
anujsingh-cse wants to merge 2 commits into
Open
anujsingh-cse wants to merge 2 commits into
anujsingh-cse wants to merge 2 commits into
Conversation
Removing cloudinit.sources.DataSourceOVF from the mypy override list in pyproject.toml surfaces six errors. The transports list joins two functions with different signatures into an uncalled object type; annotate it as a list of (name, no-arg callable returning Optional[str]) tuples. The loop also no longer pre-binds name to None: name is only read when one of the calls produced contents, so the loop-bound str is always defined there. seedfound starts as a bool and is then assigned the matched proto string; start it as Optional[str], which keeps the falsy checks identical. find_child builds an empty list of minidom nodes; annotate it. get_properties reads documentElement, which typeshed types as Optional, directly for localName and hasChildNodes. Read it once into a local and fold the impossible None case into the existing "No Environment Node" XmlError, so both reads are narrowed. Refs canonicalGH-5445
canonical#6958 added base64 error-message parameters to test_azure_helper.py with implicitly concatenated strings inside collection literals, which the pinned ruff flags as ISC004. CI for every open PR runs against the merge ref, so this fails check_format across the board. Wrap the two concatenations in parentheses; the string values are unchanged.
anujsingh-cse
force-pushed
the
fix/untyped-defs-ovf-5445
branch
from
October 10, 2026 03:42
86f56e1 to
a8a89c8
Compare
Contributor
Author
|
Rebased onto current main and fixed the check_format failure: the ISC004 lint errors were introduced to main by #6958 (implicitly concatenated strings in the test_azure_helper.py parametrize), and the merge-ref CI inherited them. The two concatenations are now parenthesized (a8a89c8); ruff 0.16.2 passes on the whole tree. The OVF changes themselves were already green. |
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
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.
Proposed Commit Message
Additional Context
Refs GH-5445. Claimed in #5445 (comment).
Removing the module from the override list surfaces six errors:
npholds(transport name, callable)tuples whose two functionshave different signatures, so the join is not callable. Annotated
as
List[Tuple[str, Callable[[], Optional[str]]]](both transportsare zero-arg callable and return content or None). The pre-loop
name = Noneis dropped:nameis only read insideif contents:, which requires a loop iteration to have set both.seedfoundflips from a bool flag to the matched proto string;starting it as
Optional[str]keepsif not seedfound:identical.find_child's result list is annotatedList[minidom.Node].get_propertiesre-readsdocumentElement(Optional per typeshed)twice; it is now read once into a local and the impossible None
case is folded into the existing
"No Environment Node"XmlError.Test Steps
tox -e py3andtox -e check_formatto be confirmed by CI.Merge type