ansible / proposals

Repository for sharing and tracking progress on enhancement proposals for Ansible.
Creative Commons Zero v1.0 Universal
92 stars 19 forks source link

Type Annotations in ansible-core #202

Closed nitzmahone closed 1 year ago

nitzmahone commented 2 years ago

Proposal: Type Annotations in Ansible Core

Author: Matt Davis - @nitzmahone

Date: 2021-11-17

Motivation

Since the ansible-core controller fully dropped support for older versions of Python in 2.12, new Python language features like inline type annotations have become available to us. Before we open the floodgates to type annotations being splattered across the core codebase, it's necessary to define some policy around how type annotations will be used, and especially how they will be validated.

Problems

What problems exist that this proposal will solve?

Solution proposal

General guidelines

1) At minimum, any net-new public Python API added to ansible-core from 2.13 on should have type annotations. 2) Type annotations should be added to existing public API surfaces only after consensus has been reached on the value of doing so, and if the resultant type annotations are not overly complex. Adding pedantically-correct type annotations to existing APIs that were not designed for it can result in unmaintainable or unreadable code. A careful balance between maintainability and correctness needs to be struck for annotations of pre-existing APIs. 3) Each release of ansible-core will specify a major.minor release of mypy to validate its annotations. Other validators were considered (pytype, pyright, pyre), but mypy currently strikes the best balance of stability, performance, and ease-of-use in our testing. 4) Type validation ignore entries should be per-Python version (TBD whether annotations are validated for multiple Python versions- the current sample PR tests all supported controlller/target Python versions) 5) For validation, require pinned type stubs for defined/documented public ansible-core deps (eg, Jinja2, pyyaml, requests, packaging, toml). Use a template mechanism to manage updates to pinned versions across major releases of core (the sample PR implements this).

Style

1) Imports: import typing as t where possible for brevity (balance typing import boilerplate with terse annotations) 2) Function/method annotations take priority. Call-site/var type annotations can be added where most calls are to well-annotated code and the annotations provide significant benefit without impeding readability/maintainability.

Controller code

1) Use PEP484/PEP526 language-integrated inline type annotations, since the controller is always Python3.8+ 2) The PEP563 deferred annotations future should be imported to improve runtime performance. Consider adding this to the required boilerplate for controller code.

Non-controller code (module API, modules, module_utils, collection loader)

1) Use inline PEP484 type comments on new module APIs. 2) Avoid use of stub files unless type annotations become extremely unwieldy; we should be worried more about moving docs out of target-side code before worrying about the additional code bloat from type annotations.

ansible-test code

The ansible-test codebase is already heavily type-annotated, and the annotation sample PR above includes validation.

Dependencies

Testing

Type annotations should always be validated when present; sanity testing is a necessary pre-requisite to applying new type annotations.

Documentation

When these policies are finalized, they should be documented in the ansible-core development guidelines.

Non-Goals

Collections support

Bounding the problem-space properly for the ansible-core codebase will be difficult enough, and may take several releases to get the proper tooling and procedures in place. Supporting collections with the initial tooling adds a significant amount of complexity we're not currently staffed to deal with (multiple Ansible versions, broader Python/validator version scopes, multiple profiles).

Type-annotating the entire ansible-core codebase

Our initial approach should be to tread lightly and discover problems by applying type annotations to new public APIs and high-value areas, not to blanket apply type annotations to the entire codebase. Massive unsolicited type annotation PRs should not be accepted.

Runtime type checks

Runtime type-checking in Python is in relative infancy; our initial application should be focused on validation at development-time, not strict runtime enforcement.

Docstring integration

Language-standard type annotations, while a form of documentation, were generally designed to be used inline for consumption by type checkers and IDE completion. Defining an overall policy for docstrings is orthogonal, since the stated goals cannot be met with legacy type annotations in docstrings.

sivel commented 2 years ago

+1 from me

felixfontein commented 2 years ago

+1

maxamillion commented 2 years ago

+1

briantist commented 2 years ago

+1

nitzmahone commented 1 year ago

Closing since this was implemented by https://github.com/ansible/ansible/commit/3d5637beecbbdcd1bc11a85d180de2087ecf12ee (and related followups)