Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type Annotations in ansible-core #202

Closed
nitzmahone opened this issue Nov 30, 2021 · 5 comments
Closed

Type Annotations in ansible-core #202

nitzmahone opened this issue Nov 30, 2021 · 5 comments

Comments

@nitzmahone
Copy link
Member

nitzmahone commented Nov 30, 2021

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?

  • Ensure reasonable consistency in application of type annotations in core codebase
  • Ensure that type annotations are being validated by sanity tests
  • Balance the usefulness of type annotations (call-site validation, self-documentation, IDE completion) with ease of development and maintenance

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

  • A type annotation validator (eg, mypy as in the sample sanity test PR above)

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
Copy link
Member

sivel commented Nov 30, 2021

+1 from me

@felixfontein
Copy link

+1

2 similar comments
@maxamillion
Copy link

+1

@briantist
Copy link

+1

@nitzmahone
Copy link
Member Author

nitzmahone commented Oct 14, 2022

Closing since this was implemented by ansible/ansible@3d5637b (and related followups)

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

No branches or pull requests

6 participants