fix(cmd): enable mypy strict checking for cloudinit.cmd

[jaborvs]

Partially-fixes: GH-5445

As part of my onboarding, I fixed the untyped defs errors found the following files:

• cloudinit.cmd.devel.make_mime
• cloudinit.cmd.devel.net_convert
• cloudinit.cmd.main

Proposed Commit Message

fix(typing): Enable mypy strict checking for cloudinit.cmd

Remove cloudinit.cmd.main, cloudinit.cmd.devel.net_convert and
cloudinit.cmd.devel.make_mime from the check_untyped_defs=false
override list in pyproject.toml and resolve all mypy errors for
each module.

Partially-fixes: GH-5445

Additional Context

N/A

Test Steps

N/A

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

[jaborvs]

There is an extra commit I will remove once the review is completed. This is to fix the error in the "Unit: Alternate Distros / build (pull_request)" job, which fails if the package py3-python-discovery is not installed in the install dependencies step. There is an open PR which implements this fix, pending to be approved and merged (see PR-6864)

[holmanb]

Thanks for this. I left some feedback.

[holmanb]

It is odd that this type is not a string in the first place. Creating a tuple to append to a list of strings is unnecessary.

[holmanb]

That said, this doesn't need to turn into refactoring. I'd just name the later re-assignment something else like "mime_msg".

[jaborvs]

Thanks for the feedback!

Just to clarify my understanding: (content_type, i + 1) does form a tuple as the right operand to %, which is standard for %-formatting with multiple arguments. However, the % operator itself returns a str directly, so what gets appended to errors is already a string, not a tuple. I believe the parentheses around the template string on the left were purely for line wrapping clarity. I have done a quick test to verify the typing and this assumption seems to be correct!

That said, I'm happy to rename the variable from err_msg to mime_msg or to split the formatting logic into several variables if it helps making the code clearer.

What do you think is the best option?

[holmanb]

It would be preferred to avoid changing control flow. Same comment elsewhere in this file.

[jaborvs]

I assume the "elsewhere" applies to the changes in lines 557-633 and 631-637 (let me know if I am missing some other blocks of code).

In this case, the three things that come to mind to solve this mypy error are using if statements, asserts or casts. I believe that the later two options are not very good practices and should be avoided. Therefore, let me explain why I chose to use an if in every situation:

• init.datasource can be None — lines 566, 638: After init.fetch(), datasource is Optional[DataSource], which implies no datasource found is a real runtime case. In case it is None, then it will not have the dsmode attribute. The code must handle this check before attempting to access the attribute, so a None check seems unavoidable to me.
• handler is typed as logging.Handler, not LogExporter — line 954: next(filter(...)) returns logging.Handler. Mypy has no way to know it's a LogExporter without a runtime isinstance check or a cast. I believe that if we want to avoid casting there is no annotation-only fix here.

If you have some other fix in mind I am skipping, please let me know!

[holmanb]

This doesn't seem ideal. Is a way to do this without accessing internal attributes?

[jaborvs]

Changed the logic to avoid accessing internal attributes. To do so, I overrided the add_subparsers method and stored the action in a local class attribute named _subparsers_action.