Skip to content

chore: add type annotations to DataSourceOVF - #7148

Open
anujsingh-cse wants to merge 2 commits into
canonical:mainfrom
anujsingh-cse:fix/untyped-defs-ovf-5445
Open

anujsingh-cse wants to merge 2 commits into
canonical:mainfrom
anujsingh-cse:fix/untyped-defs-ovf-5445

Conversation

@anujsingh-cse

Copy link
Copy Markdown
Contributor

Proposed Commit Message

chore: add type annotations to DataSourceOVF

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 GH-5445

Additional Context

Refs GH-5445. Claimed in #5445 (comment).

Removing the module from the override list surfaces six errors:

DataSourceOVF.py:70: error: Cannot call function of unknown type  [operator]
DataSourceOVF.py:78: error: Argument 1 to "append" of "list" has
                incompatible type "str | None"; expected "str"  [arg-type]
DataSourceOVF.py:89: error: Incompatible types in assignment
                (expression has type "str", variable has type "bool")  [assignment]
DataSourceOVF.py:350: error: Need type annotation for "ret"  [var-annotated]
DataSourceOVF.py:361: error: Item "None" of "Element | None"
                has no attribute "localName"  [union-attr]
DataSourceOVF.py:364: error: Item "None" of "Element | None"
                has no attribute "hasChildNodes"  [union-attr]
  • np holds (transport name, callable) tuples whose two functions
    have different signatures, so the join is not callable. Annotated
    as List[Tuple[str, Callable[[], Optional[str]]]] (both transports
    are zero-arg callable and return content or None). The pre-loop
    name = None is dropped: name is only read inside
    if contents:, which requires a loop iteration to have set both.
  • seedfound flips from a bool flag to the matched proto string;
    starting it as Optional[str] keeps if not seedfound: identical.
  • find_child's result list is annotated List[minidom.Node].
  • get_properties re-reads documentElement (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

$ python -m mypy cloudinit/ tests/ tools/
(0 errors in the target module with the override removed; no new
 errors in modules that import it)

$ pytest tests/unittests/sources/test_ovf.py -q
21 passed, 9 failed — the same 9 failures occur on untouched main
on this Windows sandbox (device-path/platform tests); no regressions
from this change.

$ ruff check / black --check / isort --check-only / pylint
All green

tox -e py3 and tox -e check_format to be confirmed by CI.

Merge type

  • Squash merge using "Proposed Commit Message"
  • Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

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
anujsingh-cse force-pushed the fix/untyped-defs-ovf-5445 branch from 86f56e1 to a8a89c8 Compare October 10, 2026 03:42
@anujsingh-cse

Copy link
Copy Markdown
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant