Validate expected types for args for DAG, BaseOperator and TaskGroup

[pankajkoti]

Validates that some commonly used arguments by DAG authors
conform to the expected types. If the provided values do not
match the expected types, a TypeError is raised, resulting in
DAG import errors that appear in the Airflow UI.

Closes: #40187

---

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

[uranusjr]

Generally looks good to me, except I think we don’t need to declare the types in a separate dict, but can use typing.get_type_hints to extract the type hints from __init__. I am not entirely sure, but let’s give it a try.

Most of the entries should be simple, only email needs some extra work to handle the Union type. We can use typing.get_origin(...) is typing.Union to check if the instance is a Union, and use typing.get_args to extract the arguments.

[pankajkoti]

Generally looks good to me, except I think we don’t need to declare the types in a separate dict, but can use typing.get_type_hints to extract the type hints from __init__. I am not entirely sure, but let’s give it a try.

Most of the entries should be simple, only email needs some extra work to handle the Union type. We can use typing.get_origin(...) is typing.Union to check if the instance is a Union, and use typing.get_args to extract the arguments.

I did try to get the type hints using get_type_hints method from the typing module, but it does help for Python < 3.10, for type annotations that use a bitwise or | in the annotations instead of the Union annotation, also did try typing.get_origin(...) is typing.Union but it did not yield expected results. Additionally using type hints directly in the isinstance methods may not work well always e.g. when the annotation is of type Iterable[str]
Hence, I have taken the manual approach 🙂 in this PR

[uranusjr]

That makes sense, getting annotations at runtime (instead of in Mypy) is something Python has been working in recently but I was not sure exactly when it became usable. Looks like we can’t do that yet (maybe two years later…)

Let’s extract the dicts and checks outside the classes, and add a todo comment that says maybe consider refactoring when we require 3.10+ (not sure about the exact version but we don’t need to be exact)

[pankajkoti]

Let’s extract the dicts and checks outside the classes

@uranusjr, could you clarify where exactly we should extract the validation logic? Do you mean move the dicts to be within the same module but outside the classes? The arguments we validate are specific to certain classes (DAG, BaseOperator, TaskGroup) and vary across them. Moreover, since these argument values could be passed directly or via default arguments, we perform the validation check after the instance has been assigned all argument values, following the evaluation of positional/keyword arguments and default arguments. In my opinion, keeping these dicts localized within their respective classes is better. However, if you could explain where and why we should extract them, it would help me understand your perspective better. Also, the helper validation method is already outside the classes in utils which we call from the respective classes.

[uranusjr]

Your correct checks are fine, I just want to move the dicts out of the classes, and add comments to hint about future refactors (and to stop people from refactoring before the supported Python versions allow us to).

[pankajkoti]

@uranusjr okay, moved the dicts out and added comments as suggested. Please review when possible, thanks.

[dstandish]

my suggestions:

• keep it limited to the attrs that the scheduler cares about
• but, if we're going to do all params, use a pre-commit for it, or some existing library that does it more invisibly

[uranusjr]

We might want to add a pre-commit hook to ensure the dicts are up-to-date with the __init__ arguments. But this lgtm.

[pankajkoti]

my suggestions:

• keep it limited to the attrs that the scheduler cares about
• but, if we're going to do all params, use a pre-commit for it, or some existing library that does it more invisibly

I will dwelve into this later to check if we can curtail the attrs list and/or if we have a good library that could help us. For now, would like to merge this and have a preventive workaround for crashes we're seeing users reporting.