Repository navigation
fix(abstractions): default the type params of RequestAdapter and ParsableFactory - #787
Conversation
|
hmm, default=any = no limits at all. I'll convert this PR to draft and reopen later |
…ableFactory Generated clients annotate request_adapter: RequestAdapter and dict[str, type[ParsableFactory]] without type arguments, which mypy --strict reports wherever the generated code is checked. That's all 141 strict errors in tests/validation, and msgraph-sdk 1.64.0 has about 26,500 such annotations. Give both type parameters a PEP 696 default at the top of their range. ParsableFactory's U_co defaults to its bound, Parsable, and since it's covariant every ParsableFactory[X] still fits the bare name. RequestAdapter's type parameter is only returned by convert_to_native_async, so it becomes covariant and defaults to object; pylint wants the _co suffix, so it's renamed to RequestType_co. HttpxRequestAdapter.send_collection_async now declares ParsableFactory[ModelType] like the method it overrides, since the bare name no longer means Any. TypeVar only takes a default in the stdlib from 3.13, so it comes from typing_extensions, now a declared dependency of abstractions (it was already installed through opentelemetry-api). tests/validation runs with strict = true so CI catches a regression. Part of microsoft#722.
8a575ec to
ea9b41c
Compare
|
Rename from Ready for review btw |
There was a problem hiding this comment.
🟡 Changes recommended
The public TypeVar rename and unspecialized HTTP adapter introduce typing compatibility regressions.
2 open findings
What changed in this PR
Adds safe defaults to core generic types so generated clients pass strict mypy checks.
Changes:
- Defaults
RequestAdapterandParsableFactorytype parameters. - Enables strict mypy validation.
- Tightens the HTTP collection factory annotation.
| File | Description |
|---|---|
tests/validation/pyproject.toml |
Enables strict mypy checking. |
packages/http/httpx/kiota_http/httpx_request_adapter.py |
Specializes the collection factory annotation. |
packages/abstractions/pyproject.toml |
Adds the TypeVar backport dependency. |
packages/abstractions/kiota_abstractions/serialization/parsable_factory.py |
Defaults the parsable factory type. |
packages/abstractions/kiota_abstractions/request_adapter.py |
Makes the request type covariant and defaulted. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Max Azatian (@HardMax71) would you mind addressing copilot's feedback please? |
…stAdapter[httpx.Request]
|
Vincent Biret (@baywet) addressed |
Vincent Biret (baywet)
left a comment
There was a problem hiding this comment.
Thank you for making the changes!
|




Overview
Generated clients use
RequestAdapterandParsableFactorywithout type arguments. Every request builder takesrequest_adapter: RequestAdapter, and every error map is adict[str, type[ParsableFactory]]. Undermypy --stricteach of those is a "Missing type arguments" error wherever the generated code is checked, which is the usual setup when kiota generates a client into a project. They're all 141 strict errors left in the validation client, and msgraph-sdk 1.64.0 has about 26,500 such annotations.This gives both type parameters a default (PEP 696) at the top of their range, so the bare names stay checked and strict checkers accept them without any generator change or regeneration:
ParsableFactory'sU_codefaults to its bound,Parsable. It's covariant, so everyParsableFactory[X]still fits a bareParsableFactory.RequestAdapter's type parameter only appears as the return type ofconvert_to_native_async, so it becomes covariant and defaults toobject, which every adapter's native request type fits.TypeVaronly takes a default in the stdlib from 3.13, so it comes fromtyping_extensions, which becomes a declared dependency of abstractions. It was already installed through opentelemetry-api.tests/validation gets
strict = true, so the existing validation job fails if generated code picks up a strict error again.Related Issue
Part of #722, added there as its own item.
Notes
pylint wants the
_cosuffix on a covariant TypeVar, soRequestAdapteruses a newRequestType_co. The oldRequestTypestays importable as it was, as #722 requires.HttpxRequestAdapter.send_collection_asynctook a bareParsableFactory, which only type-checked because the bare name meantParsableFactory[Any]. It now declaresParsableFactory[ModelType], like the method it overrides.HttpxRequestAdapternow subclassesRequestAdapter[httpx.Request], the type itsconvert_to_native_asyncreturns. With theobjectdefault, a bare subclass would no longer fit an annotation that asks forRequestAdapter[httpx.Request].As a downstream check, msgraph-sdk-python-core's CI step
mypy src(mypy 1.17.1) still passes against this branch, and its strict run goes from 70 to 60 errors.The floor is typing-extensions 4.4.0, the release that added
default. The abstractions tests pass on Python 3.10 with exactly that version.Since abstractions and http use the bare names themselves,
mypy --strictalso goes from 63 to 51 errors in abstractions and from 127 to 117 in http (mypy 2.4.0). It merges cleanly with #785.Testing Instructions
mypy validation(now strict) passes, also with--python-version 3.10and3.14. On main the strict run reports 141 errors.mypyrun passes for all eight packages.yapf -dr,isort,pylint --disable=Wandpytestpass for abstractions and http.tests/test_typing.pyin packages/http/httpx runs mypy on a snippet that returns anHttpxRequestAdapterasRequestAdapter[httpx.Request]. Without the base-class change it fails withIncompatible return value type.