Skip to content

Commit 5b3caec

Browse files
l0lawrenceCopilot
andcommitted
fix(http-client-python): address review feedback on TypedDict opt-out
- Remove synthetic internal `models-mode: typeddict`; represent the deprecated value as `models-mode: none` + `generate-typeddict: true`, and drive TypedDict-only behavior via a `generate_typeddict_only` helper instead of a fake models-mode. - Replace generic `**kwargs` in the `_plugin` test helper with an explicit `generate_typeddict: bool` parameter. - Drop msrest from the models-mode ValueError help text (back-compat only). - Reword the `generate-typeddict` description: it adds TypedDict typing for JSON dict input, not an extra request-body overload. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c8b26a94-682b-4350-a424-97093fd1c183
1 parent b6bb261 commit 5b3caec

18 files changed

Lines changed: 140 additions & 104 deletions

File tree

packages/http-client-python/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ Whether to keep the existing `setup.py` when `generate-packaging-files` is `true
134134

135135
**Type:** `boolean`
136136

137-
Whether to generate `TypedDict` types for request bodies. Defaults to `true`. With `models-mode: dpg` this adds `TypedDict` request-body overloads alongside the model classes; with `models-mode: none` it generates `TypedDict`-only types. Set to `false` to opt out of `TypedDict` generation.
137+
Whether to add TypedDict typing for JSON dictionary input in `models-mode: dpg`, instead of accepting only generic JSON. This enriches the typing on the existing overloads rather than adding another request-body overload. Defaults to `true`.
138138

139139
### `keep-pyproject-fields`
140140

packages/http-client-python/emitter/src/lib.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ export const PythonEmitterOptionsSchema: JSONSchemaType<PythonEmitterOptions> =
116116
type: "boolean",
117117
nullable: true,
118118
description:
119-
"Whether to generate `TypedDict` types for request bodies. Defaults to `true`. With `models-mode: dpg` this adds `TypedDict` request-body overloads alongside the model classes; with `models-mode: none` it generates `TypedDict`-only types. Set to `false` to opt out of `TypedDict` generation.",
119+
"Whether to add TypedDict typing for JSON dictionary input in `models-mode: dpg`, instead of accepting only generic JSON. This enriches the typing on the existing overloads rather than adding another request-body overload. Defaults to `true`.",
120120
},
121121
"keep-pyproject-fields": {
122122
type: "object",

packages/http-client-python/generator/pygen/__init__.py

Lines changed: 22 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
from typing import Any, Iterator, Optional, Union
1313

1414
import yaml
15-
from .utils import TYPESPEC_PACKAGE_MODE, VALID_PACKAGE_MODE
15+
from .utils import TYPESPEC_PACKAGE_MODE, VALID_PACKAGE_MODE, is_typeddict_only
1616

1717
from ._version import VERSION
1818

@@ -46,11 +46,19 @@ class OptionsDict(MutableMapping):
4646

4747
def __init__(self, options: Optional[dict[str, Any]] = None) -> None:
4848
self._data = options.copy() if options else {}
49+
# 'models-mode: typeddict' is deprecated. Represent it internally as 'none' models-mode with
50+
# TypedDict generation enabled, so the rest of the codebase only reasons about dpg/msrest/none.
51+
if self._data.get("models-mode") == "typeddict":
52+
self._data["generate-typeddict"] = True
4953
for key in list(self._data):
5054
self._data[key] = self._validate_and_transform(key, self._data[key])
51-
self._normalize_models_mode()
5255
self._validate_combinations()
5356

57+
@property
58+
def generate_typeddict_only(self) -> bool:
59+
"""Whether this is the deprecated TypedDict-only generation ('models-mode: none' + TypedDicts)."""
60+
return is_typeddict_only(self)
61+
5462
def __getitem__(self, key: str) -> Any: # pylint: disable=too-many-return-statements
5563
if key == "head-as-boolean" and self.get("azure-arm"):
5664
# override to always true if azure-arm is set
@@ -133,38 +141,6 @@ def _get_default(self, key: str) -> Any: # pylint: disable=too-many-return-stat
133141
return self.get("flavor") == "azure"
134142
return self.DEFAULTS[key]
135143

136-
def _normalize_models_mode(self) -> None:
137-
"""Reconcile ``models-mode`` with ``generate-typeddict`` for TypeSpec generation.
138-
139-
The user-facing ``models-mode`` values are ``dpg`` and ``none`` (``msrest``
140-
is kept for back-compat). TypedDict output is controlled independently by
141-
``generate-typeddict`` (default ``True``):
142-
143-
* ``dpg`` + ``generate-typeddict`` -> DPG models and TypedDict overloads
144-
* ``dpg`` + no ``generate-typeddict`` -> DPG models only
145-
* ``none`` + ``generate-typeddict`` -> TypedDicts only
146-
* ``none`` + no ``generate-typeddict`` -> nothing
147-
148-
``models-mode: typeddict`` is deprecated; it is still accepted (with a
149-
warning) and behaves as TypedDict-only. Internally, TypedDict-only
150-
generation is represented by ``models-mode == "typeddict"``, so the
151-
``none`` + TypedDicts case is remapped to it here. This only applies to
152-
TypeSpec input; swagger ``models-mode: none`` is left untouched.
153-
"""
154-
if "models-mode" not in self._data:
155-
return
156-
models_mode = self._data["models-mode"]
157-
if models_mode == "typeddict":
158-
_LOGGER.warning(
159-
"'models-mode: typeddict' is deprecated. Use 'models-mode: none' instead "
160-
"(TypedDicts are generated by default; set 'generate-typeddict: false' to opt out)."
161-
)
162-
return
163-
# 'none' is stored as falsy False. For TypeSpec, keep generating TypedDicts
164-
# by default by remapping to the internal typeddict-only mode.
165-
if bool(self._data.get("tsp_file")) and models_mode is False and self.get("generate-typeddict"):
166-
self._data["models-mode"] = "typeddict"
167-
168144
def _validate_combinations(self) -> None:
169145
if not self.get("show-operations") and self.get("builders-visibility") == "embedded":
170146
raise ValueError(
@@ -201,15 +177,22 @@ def _validate_and_transform(self, key: str, value: Any) -> Any:
201177
if key == "builders-visibility" and value not in ["public", "hidden", "embedded"]:
202178
raise ValueError("The value of --builders-visibility must be either 'public', 'hidden', or 'embedded'")
203179

180+
if key == "models-mode" and value == "typeddict":
181+
# Deprecated: keep accepting it for back-compat but store it as 'none' (falsy) with
182+
# TypedDict generation enabled (see OptionsDict.__init__ and generate_typeddict_only).
183+
_LOGGER.warning(
184+
"'models-mode: typeddict' is deprecated. Use 'models-mode: none' with "
185+
"'generate-typeddict: true' (the default) instead."
186+
)
187+
value = False
188+
204189
if key == "models-mode" and value == "none":
205190
value = False # switch to falsy value for easier code writing
206191

207-
if key == "models-mode" and value not in ["msrest", "dpg", "typeddict", False]:
192+
if key == "models-mode" and value not in ["msrest", "dpg", False]:
208193
raise ValueError(
209-
"--models-mode can only be 'msrest', 'dpg', or 'none'. "
210-
"Pass in 'msrest' if you want msrest models, 'dpg' for DPG models, or "
211-
"'none' if you don't want any. TypedDicts are controlled by --generate-typeddict "
212-
"(the deprecated 'typeddict' value is still accepted for back-compat)."
194+
"--models-mode can only be 'dpg' or 'none'. "
195+
"Pass in 'dpg' for DPG models, or 'none' if you don't want any."
213196
)
214197
if key == "package-mode":
215198
if (

packages/http-client-python/generator/pygen/codegen/models/__init__.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,9 @@ def build_type(yaml_data: dict[str, Any], code_model: CodeModel) -> BaseType:
169169
# need to special case model to avoid recursion
170170
if yaml_data["base"] == "typeddict":
171171
model_type = TypedDictModelType # type: ignore
172-
elif yaml_data["base"] == "json" or not code_model.options["models-mode"]:
172+
elif yaml_data["base"] == "json" or (
173+
not code_model.options["models-mode"] and not code_model.generate_typeddict_only
174+
):
173175
model_type = JSONModelType
174176
elif yaml_data["base"] == "dpg":
175177
model_type = DPGModelType # type: ignore

packages/http-client-python/generator/pygen/codegen/models/code_model.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from .utils import NamespaceType
1818
from .._utils import DEFAULT_HEADER_TEXT, DEFAULT_LICENSE_DESCRIPTION
1919
from ... import OptionsDict
20+
from ...utils import is_typeddict_only
2021

2122

2223
def _is_legacy(options) -> bool:
@@ -89,7 +90,7 @@ def __init__(
8990
self.clients: list[Client] = [
9091
Client.from_yaml(client_yaml_data, self) for client_yaml_data in yaml_data["clients"]
9192
]
92-
if self.options["models-mode"] and self.model_types:
93+
if (self.options["models-mode"] or self.generate_typeddict_only) and self.model_types:
9394
self.sort_model_types()
9495
self.named_unions: list[CombinedType] = [
9596
t for t in self.types_map.values() if isinstance(t, CombinedType) and t.name
@@ -174,6 +175,11 @@ def get_unique_models_alias(self, serialize_namespace: str, imported_namespace:
174175
def get_unique_types_alias(self, serialize_namespace: str, imported_namespace: str) -> str:
175176
return self._get_unique_import_alias(serialize_namespace, imported_namespace, "types")
176177

178+
@property
179+
def generate_typeddict_only(self) -> bool:
180+
"""Whether this is the deprecated TypedDict-only generation ('models-mode: none' + TypedDicts)."""
181+
return is_typeddict_only(self.options)
182+
177183
@property
178184
def client_namespace_types(self) -> dict[str, ClientNamespaceType]:
179185
if not self._client_namespace_types:

packages/http-client-python/generator/pygen/codegen/models/enum_type.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ def description(self, *, is_operation_file: bool) -> str:
4040

4141
def type_annotation(self, **kwargs: Any) -> str:
4242
"""The python type used for type annotation"""
43-
if self.code_model.options["models-mode"] == "typeddict":
43+
if self.code_model.generate_typeddict_only:
4444
# A single constant enum value must be
4545
# annotated with its literal value directly (e.g. ``Literal["red"]``).
4646
return f"Literal[{self.value_type.get_declaration(self.value)}]"
@@ -83,7 +83,7 @@ def imports(self, **kwargs: Any) -> FileImport:
8383
file_import = FileImport(self.code_model)
8484
file_import.merge(self.value_type.imports(**kwargs))
8585
file_import.add_submodule_import("typing", "Literal", ImportType.STDLIB, TypingSection.REGULAR)
86-
if self.code_model.options["models-mode"] == "typeddict":
86+
if self.code_model.generate_typeddict_only:
8787
# In typeddict mode the enums module (``_enums.py``) is never generated
8888
return file_import
8989
serialize_namespace = kwargs.get("serialize_namespace", self.code_model.namespace)
@@ -176,15 +176,15 @@ def description(self, *, is_operation_file: bool) -> str:
176176

177177
@property
178178
def is_typeddict_mode(self) -> bool:
179-
return self.code_model.options["models-mode"] == "typeddict"
179+
return self.code_model.generate_typeddict_only
180180

181181
def type_annotation(self, **kwargs: Any) -> str:
182182
"""The python type used for type annotation
183183
184184
:return: The type annotation for this schema
185185
:rtype: str
186186
"""
187-
if self.code_model.options["models-mode"]:
187+
if self.code_model.options["models-mode"] or self.code_model.generate_typeddict_only:
188188
if self.is_typeddict_mode:
189189
# In typeddict mode, enums are Literal aliases defined in types.py
190190
serialize_namespace_type = kwargs.get("serialize_namespace_type")
@@ -221,13 +221,13 @@ def get_declaration(self, value: Any) -> str:
221221
return self.value_type.get_declaration(value)
222222

223223
def docstring_text(self, **kwargs: Any) -> str:
224-
if self.code_model.options["models-mode"]:
224+
if self.code_model.options["models-mode"] or self.code_model.generate_typeddict_only:
225225
return self.name
226226
return self.value_type.type_annotation(**kwargs)
227227

228228
def docstring_type(self, **kwargs: Any) -> str:
229229
"""The python type used for RST syntax input and type annotation."""
230-
if self.code_model.options["models-mode"]:
230+
if self.code_model.options["models-mode"] or self.code_model.generate_typeddict_only:
231231
type_annotation = self.value_type.type_annotation(**kwargs)
232232
enum_type_annotation = f"{self.client_namespace}.models.{self.name}"
233233
return f"{type_annotation} or ~{enum_type_annotation}"
@@ -261,7 +261,7 @@ def from_yaml(cls, yaml_data: dict[str, Any], code_model: "CodeModel") -> "EnumT
261261
def imports(self, **kwargs: Any) -> FileImport:
262262
file_import = FileImport(self.code_model)
263263
file_import.merge(self.value_type.imports(**kwargs))
264-
if self.code_model.options["models-mode"]:
264+
if self.code_model.options["models-mode"] or self.code_model.generate_typeddict_only:
265265
if self.is_typeddict_mode:
266266
# In typeddict mode, enums are Literal aliases in types.py — no Union needed
267267
serialize_namespace_type = kwargs.get("serialize_namespace_type")

packages/http-client-python/generator/pygen/codegen/models/list_type.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ def type_annotation(self, **kwargs: Any) -> str:
3838
self.code_model.options["version-tolerant"]
3939
and self.element_type.is_xml
4040
and not self.code_model.options["models-mode"]
41+
and not self.code_model.generate_typeddict_only
4142
):
4243
# this means we're version tolerant XML, we just return the XML element
4344
return self.element_type.type_annotation(**kwargs)

packages/http-client-python/generator/pygen/codegen/models/model_type.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,7 @@ def __init__(
7777
self.cross_language_definition_id: Optional[str] = self.yaml_data.get("crossLanguageDefinitionId")
7878
self.usage: int = self.yaml_data.get("usage", UsageFlags.Input.value | UsageFlags.Output.value)
7979
self.client_namespace: str = self.yaml_data.get("clientNamespace", code_model.namespace)
80-
self.is_typed_dict_only: bool = (
81-
self.yaml_data.get("typedDictOnly", False) or code_model.options["models-mode"] == "typeddict"
82-
)
80+
self.is_typed_dict_only: bool = self.yaml_data.get("typedDictOnly", False) or code_model.generate_typeddict_only
8381

8482
@property
8583
def is_usage_output(self) -> bool:

packages/http-client-python/generator/pygen/codegen/models/operation.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ def imports( # pylint: disable=too-many-branches, disable=too-many-statements
334334
file_import.merge(
335335
response.imports(async_mode=async_mode, need_import_iobase=self.need_import_iobase, **kwargs)
336336
)
337-
if self.code_model.options["models-mode"]:
337+
if self.code_model.options["models-mode"] or self.code_model.generate_typeddict_only:
338338
for exception in self.exceptions:
339339
file_import.merge(exception.imports(async_mode=async_mode, **kwargs))
340340

@@ -552,7 +552,12 @@ def imports(self, async_mode: bool, **kwargs: Any) -> FileImport:
552552
"distributed_trace_async",
553553
ImportType.SDKCORE,
554554
)
555-
if self.has_response_body and not self.has_optional_return_type and not self.code_model.options["models-mode"]:
555+
if (
556+
self.has_response_body
557+
and not self.has_optional_return_type
558+
and not self.code_model.options["models-mode"]
559+
and not self.code_model.generate_typeddict_only
560+
):
556561
file_import.add_submodule_import("typing", "cast", ImportType.STDLIB)
557562

558563
return file_import

packages/http-client-python/generator/pygen/codegen/models/parameter.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,9 @@ def method_location( # pylint: disable=too-many-return-statements
338338
) -> ParameterMethodLocation:
339339
if not self.in_method_signature:
340340
raise ValueError(f"Parameter '{self.client_name}' is not in the method.")
341-
if self.code_model.options["models-mode"] in ("dpg", "typeddict") and self.in_flattened_body:
341+
if (
342+
self.code_model.options["models-mode"] == "dpg" or self.code_model.generate_typeddict_only
343+
) and self.in_flattened_body:
342344
return ParameterMethodLocation.KEYWORD_ONLY
343345
if self.grouper:
344346
return ParameterMethodLocation.POSITIONAL

0 commit comments

Comments
 (0)