Skip to content

Conversation

@felixfontein
Copy link
Collaborator

SUMMARY

This time I looked in module_utils' subdirectories :)

ISSUE TYPE
  • Docs Pull Request
  • Refactoring Pull Request
COMPONENT NAME

various module utils

@ansibullbot
Copy link
Collaborator

@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-12 Automatically create a backport for the stable-12 branch labels Dec 3, 2025
@ansibullbot ansibullbot added the tests tests label Dec 3, 2025
@felixfontein felixfontein requested a review from russoz December 3, 2025 21:16
Copy link
Collaborator

@russoz russoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the deco.py file, it LGTM. That one should be an exception, IMO.

Comment on lines +87 to +108
@t.overload
def check_mode_skip_returns(
callable: Callable[t.Concatenate[S, P], T], value: T | None = None
) -> Callable[[Callable[t.Concatenate[S, P], T]], Callable[t.Concatenate[S, P], T]]: ...


@t.overload
def check_mode_skip_returns(
callable: None, value: T
) -> Callable[[Callable[t.Concatenate[S, P], T]], Callable[t.Concatenate[S, P], T]]: ...


@t.overload
def check_mode_skip_returns(
callable: None = None, *, value: T
) -> Callable[[Callable[t.Concatenate[S, P], T]], Callable[t.Concatenate[S, P], T]]: ...


def check_mode_skip_returns(
callable: Callable[t.Concatenate[S, P], T] | None = None, value: T | None = None
) -> Callable[[Callable[t.Concatenate[S, P], T]], Callable[t.Concatenate[S, P], T]]:
def deco(func: Callable[t.Concatenate[S, P], T]) -> Callable[t.Concatenate[S, P], T]:
Copy link
Collaborator

@russoz russoz Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH, I think this is an example where life is better without the type hints. The code is now barely readable, just so that vscode, or pycharm, or copilot, or some other tool, can make a match with a caller. I'd rather choose the human over the machine for this.

Something is bothering me. I have been actually going over this file again and again, until it hit me: strongly-typed language feelings! This level of type hinting transform a "light", fun language as Python into something heavy and, pardon my language, a pain in the rear to code.

And even though the types are not enforced in runtime, if we enforce them at build time, the practical effect to the programmer is to work in a strongly-typed language. This gives me Java, C, C++ feelings.

Of course it is nice to have the tool tell you that this or that is the wrong type, but at what cost? Simple things like def f(a: int, b: str) are easy to read. These signatures above are, to an average reader (I include myself in that category), written in a different language that I need to translate into Python before I can understand what is happening.

I think this file is easier to work with without those type hints.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH, I think this is an example where life is better without the type hints.

I strongly disagree here. Without the type hints, one should never use these decorators, as you will never get any type checks for the functions that are decorated.

I would rather suggest to stop using decorators, or sticking to simpler ones (which need less complex types), than not typing them.

I think this file is easier to work with without those type hints.

Only if we deprecate this file and stop using the decorators in it.

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

Labels

backport-12 Automatically create a backport for the stable-12 branch check-before-release PR will be looked at again shortly before release and merged if possible. cloud docs identity module_utils module_utils net_tools plugins plugin (any type) remote_management storage tests tests WIP Work in progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants