[anujsingh-cse]

Proposed Commit Message

chore: add type annotations to cloudinit.distros.opensuse

Removing cloudinit.distros.opensuse and
tests.unittests.distros.test_opensuse from the mypy override list in
pyproject.toml surfaces errors in four places, all of them repeated
instances of two problems.

opensuse.preferred_ntp_clients fills the empty _preferred_ntp_clients
attribute, which the base class declares without a type. Annotate it
as Optional[List[str]] in cloudinit/distros/__init__.py, matching what
the base property, ubuntu, and opensuse all store.

test_opensuse.py builds its fixture through distros.fetch, whose
return type is the base Distro; update_method and read_only_root are
opensuse-specific attributes set in the subclass __init__, so the
assignments mypy flags are on a base-class-typed fixture. Annotate the
fixture as Any.

Refs GH-5445

Additional Context

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

Removing both modules from the override list surfaces 23 errors, which
collapse into two problems:

• opensuse.py:270 and opensuse.py:276 (plus the same shape in
  alpine.py:204, still overridden): the subclass property fills
  _preferred_ntp_clients, which the base class declares as a bare
  _preferred_ntp_clients = None at cloudinit/distros/__init__.py:145.
  mypy therefore types the attribute as None and rejects every
  subclass assignment of a list. The base property
  (__init__.py:654), ubuntu.py:54, and opensuse.py all store
  list[str] into it, so the declared type is Optional[List[str]].
  Annotating the declaration in the base class fixes every subclass at
  once; no other module assigns or reads the private attribute.
• test_opensuse.py (update_method x18, read_only_root x3): the
  fixture is built through distros.fetch, annotated as returning
  Type[Distro] (the base class), while update_method and
  read_only_root are opensuse-specific attributes set in the subclass
  __init__. The tests reset that state by assignment, so the fixture
  is annotated Any rather than changing what is fetched or patched.

No signatures change, so there is no effect on callers.

Test Steps

$ python -m mypy cloudinit/ tests/ tools/
(0 errors in the target modules with the overrides removed)

$ pytest tests/unittests/distros/test_opensuse.py -q
14 passed

$ ruff check / black --check / isort --check-only / pylint (changed files)
All green

tox -e py3 and tox -e check_format to be confirmed by CI
(developed on Windows where the full tox suite cannot run).

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>)

[blackboxsw]

Thank you for continuing to contribute @anujsingh-cse to upstream type annotations. To ensure we are getting more value from type hints in upstream commits, we would like to continue focusing on not only quieting mypy warnings, but also adding type annotations to public and private functions, methods, input parameters and return values defined throughout the module. Generally we want to avoid ANY where possible as well as that doesn't help us validate the specific assignment values to a given local variable.

Please provide a bit more type annotation coverage throughout this file and keep in mind the additional commentary on #5445 (comment)

[anujsingh-cse]

Follow-up in df98faf: every method in the module is now annotated — constructor, apply_locale, install_packages, package_command, set_timezone, update_package_sources, the hostname helpers, _set_update_method, and preferred_ntp_clients — following the GH-5445 guidance on the linked comment.

Concrete types instead of Any:

• package_command(command: str, args: Optional[Union[str, List[str]]], pkgs: Optional[PackageList]) — both accepted args spellings and the shared distro alias.
• _read_hostname -> Optional[str]; its IOError handler now assigns hostname = None instead of leaving the local unbound before the if not hostname check.
• self.update_method: Optional[str] — None until _set_update_method decides.
• preferred_ntp_clients -> List[str] — the lazy fill guarantees a concrete list.

No asserts, casts, or Any. mypy is clean with the override removed, ruff/black/isort pass, pylint is 10.00/10, and all 14 test_opensuse tests plus the distros hostname tests pass.

[anujsingh-cse]

The PR: Lint failure on this head was the ISC004 regression #6958 introduced to main (test_azure_helper.py implicit string concatenation), inherited by every merge-ref CI run.

Branch rebased onto current main (which brings #7083 and the #6958 lines) and the identical parenthesization from #7098/#7148 carried in a504567, so check_format is green on the merge-ref; it becomes a no-op once either of those merges.

Re-verified on the rebased head with the pinned toolchain: ruff check . passes on the whole tree, mypy clean for cloudinit/distros/opensuse.py, black/isort clean, and all 14 test_opensuse tests pass.

[nryanl]

Hi, I left some comments. In general:

Please review the commentary provided by @blackboxsw and the issue comment linked in his comment, particularly the section about Any / Union.

Your changes to tests/unittests/sources/test_azure_helper.py are also present in #7148 and several other of your type annotation PRs. This was addressed in a separate PR. Please avoid making changes outside the scope of the current PR and also opening identical changes in multiple PRs. You will need to rebase your branch to current main.

Please also avoid using git push --force after you have received review. It forces the reviewer to re-review your entire changeset each time.

[nryanl]

Generally we want to avoid the use of Any, because it silently disables static type checking for that variable/argument and any data interacting with it. Can we do better with cfg here?

[anujsingh-cse]

Kept Dict[str, Any] for cfg deliberately. It is the already-parsed, heterogeneous config dict passed to Distro.__init__; the base signature (cloudinit/distros/__init__.py:263) is itself untyped, and this module only writes cfg["ssh_svcname"] before forwarding the same dict. Keys and value types vary with datasource/user config, so a TypedDict cannot describe it (it would reject the arbitrary extra keys), and Mapping[str, object] would force a cast/ignore at the base call. Every other parameter and return in the module is concrete; this is the only Any left. The one supporting change outside the module is Distro._preferred_ntp_clients: Optional[List[str]], required so the concrete preferred_ntp_clients -> List[str] override type-checks.

[nryanl]

This comment is redundant/unnecessary.

[anujsingh-cse]

Removed in f52a7f5: self.update_method: Optional[str] = None now stands on its own.

[nryanl]

This change is outside the scope of the PR, which is to add type annotations to the module.

[anujsingh-cse]

Reverted in f52a7f5. The condition is back to the original not self.update_method == "transactional", and the PR diff no longer touches that line.

[nryanl]

Same comment as above regarding Any.

[anujsingh-cse]

The fixture now uses the concrete class instead of Any: distro: Distro = Distro("opensuse", {}, None), imported from cloudinit.distros.opensuse.

[nryanl]

These changes were addressed in a separate PR and are out of scope of type annotations. Please avoid making additional changes.

[anujsingh-cse]

Dropped. git diff upstream/main...HEAD -- tests/unittests/sources/test_azure_helper.py is empty on this branch and on the other six annotation branches. Main already has the parenthesization from #7157, so merging main brings it without a branch-specific change.

[anujsingh-cse]

All five threads are addressed in f52a7f5, and I have replied to each inline comment. No force-push after review: the branch merges current main and the fixes are ordinary commits.

• cfg stays Dict[str, Any]: it is the heterogeneous config dict forwarded to the untyped base Distro.__init__; every other signature in the module is concrete.
• Redundant comment removed; the out-of-scope != rewrite reverted; the test fixture now uses Distro, not Any.
• The duplicate tests/unittests/sources/test_azure_helper.py change is gone from all seven annotation PRs (main carries it via test(azure): parenthesize concatenated error messages #7157).

The remaining diff is cloudinit/distros/opensuse.py, the two pyproject mypy exemptions, the test fixture type, and Distro._preferred_ntp_clients: Optional[List[str]] (required by the preferred_ntp_clients -> List[str] override).

Gates on this head: black 26.5.1 and ruff clean, mypy clean, 14 opensuse tests pass.