feat(python): add Python contract generator#132
Conversation
SebastienMelki
left a comment
There was a problem hiding this comment.
Code Review — PR #132
@elzalem thanks for the Python generator! Since this builds on contractmodel from #131, many of the foundational issues carry over. Here's the Python-specific feedback.
internal/pyclientgen/generator.go
1. Unused package flag
The package flag is registered via flags.String("package", "", "Override Python package output path") but never read or used anywhere in the generator. Either implement it or remove it — dead flags confuse users.
2. All fields are T | None = None — no required/optional distinction
Every field is generated as optional (| None = None), making required fields indistinguishable from optional ones. Proto3 optional keyword and required semantics should produce different Python types. At minimum, non-optional fields should not default to None.
3. services dict is stringly-typed
The service metadata is generated as a deeply nested dict[str, dict[str, dict[str, str]]]. This is not Pythonic and provides no type safety. Consider generating typed dataclasses or at least TypedDict definitions for the service registry. It also lacks HTTP method/path information — for a plugin called protoc-gen-py-client, users would expect actual HTTP client functionality.
4. No to_dict() / from_dict() serialization
The generated dataclasses are pure data holders with no serialization methods. For a client generator, users need to convert to/from JSON dicts for HTTP requests. Without this, the generated code is incomplete as a "client" — it's just a set of data classes.
5. Enum values use raw proto names
Enum values like STATE_UNSPECIFIED are used directly. While this is technically valid Python, it doesn't match Python naming conventions (which would be STATE_UNSPECIFIED for constants, which happens to match — but the class name ItemItem_State is odd). The naming comes from contractmodel's __ separator issue.
6. No sebuf annotation support
Same as #131 — none of the project's custom annotations are handled. int64_encoding, enum_encoding, timestamp_format, nullable, etc. are all ignored.
internal/pyclientgen/golden_test.go
7. Same minimal test proto as C#
The test proto is identical to the one in #131 — single file, basic types only. Same gaps apply: no optional, no oneof, no complex maps, no well-known types beyond Struct, no sebuf annotations, no multi-file imports.
8. diffStrings() duplicated again
Same helper copy-pasted from csharpgen tests.
Missing Requirements
Same as #131 — this PR needs the following before it's ready:
-
Documentation: Top-level docs (README/CLAUDE.md updates) explaining the Python generator, plus generator-specific docs covering all supported features, options, and limitations.
-
Examples: A working example in the
examples/directory demonstrating the Python generator across all supported use cases — including different field types, nested messages, enums, maps, well-known types, and how to use the generated code in a real Python project. -
Extensive testing: We need:
- Unit tests for Python-specific functions (
pythonType,pythonScalar,pythonBaseType,initFiles) - Multiple golden test protos covering edge cases (optional fields, oneof, complex maps, deeply nested messages, multiple services, well-known types)
- The shared
contractmodelalso needs its own unit test suite (see #131 review)
- Unit tests for Python-specific functions (
-
Plugin completeness: For something named
protoc-gen-py-client, users expect an actual HTTP client — not just dataclasses. Either implement client functionality or rename toprotoc-gen-py-contractsto set expectations correctly.
|
@shavaizknz this pull request adds python support on the client side |
|
Hey @elzalem — really appreciate the work here, thank you for putting this together. We need the Python client shipped today for an internal deadline and we don't want to block it behind the I'll cherry-pick your test protos and serialization helpers (bytes encoding, snake_case conversion, path/query param logic, oneof discriminator sketch) into the new PR with credit to you. Let's keep the C# generator and Thanks again for the contribution, and sorry about the churn here. |
…or) (#172) * feat(py-client): scaffold protoc-gen-py-client plugin Stand up cmd/protoc-gen-py-client and internal/pyclientgen mirroring the tsclientgen layout: one generated _client.py per .proto source, stdlib-only output, dataclasses + IntEnum + Protocol-typed transport. Field rendering, JSON-mapping annotations, and RPC method bodies land in subsequent commits. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(py-client): render message dataclasses with JSON-mapping annotations Replace the message scaffold with full @DataClass field rendering plus to_dict / from_dict serialization. Honors int64_encoding, enum_encoding + enum_value, bytes_encoding, timestamp_format, nullable, empty_behavior, unwrap (root + map-value), flatten + flatten_prefix, and oneof discriminator configurations. Adds a Python type-mapping helper module and JSON encode / decode expression builders so each field collapses to one or two lines in the generated to_dict / from_dict. Enum decoding emits a per-enum helper that accepts string (proto name or custom enum_value) or int wire forms and raises on unknown values. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(py-client): implement RPC methods, options, and typed error hierarchy Flesh out the client class with real request/response handling: - path parameter substitution via urllib.parse.quote - query parameter encoding via urlencode(doseq=True), with proper guards for string/bool/numeric/repeated fields - header building from default + per-call + typed service/method header options generated from sebuf.http.service_headers and method_headers annotations - transport invocation through the injectable HttpTransport protocol with per-call timeout fallback - response parsing using each message's generated from_dict - content-type negotiation surface (JSON implemented, proto raises NotImplementedError until a follow-up adds binary protobuf encoding) - SSE streaming methods detected via HttpConfig.stream and emit NotImplementedError pointing at the follow-up issue Replace the error stub with full per-*Error-message exception classes. Each class subclasses ApiError, exposes proto fields as constructor kwargs, and ships a populate() classmethod that builds an instance from a parsed JSON dict. An _ERROR_CLASSES registry indexed by required JSON key set lets the client's _raise_for_status pick the most specific exception for a response. Add a constants.go module with Python type names and well-known type proto-name constants to satisfy goconst, and tighten every switch with the appropriate nolint:exhaustive pragmas where the default branch is intentional. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(py-client): add Phase 2 handoff doc for testing and docs work Generator implementation is complete on this branch (3 commits). This doc hands off the remaining test, demo, docs, and PR-opening work to the next agent. Includes file-by-file pointers to the patterns to mirror, the lint command tuned for go.mod 1.26, a note about the pre-existing openapiv3 test failure on main, and the rationale for not cherry-picking from PR #132. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(py-client): add golden tests + fix WKT/empty-set/pass generator bugs Adds 15 per-feature test protos mirroring tsclientgen's testdata (plus a new errors.proto exercising the per-*Error exception class generation that is unique to py-client) and a golden test harness that also runs `python3 -c "import ast; ast.parse(...)"` on each generated file to catch syntactic regressions a string-compare cannot. Capturing the goldens surfaced four generator bugs, all fixed here: - error.go: empty set literal was emitted as `{}`, which is an empty dict — violated the registry's `set[str]` type and would have crashed if the runtime guard ever fell through. - message.go: empty messages emitted `pass` followed by methods, which is semantically incorrect and noisy. The methods alone keep the class body non-empty. - types.go: Timestamp WKT fields with unix-seconds / unix-millis / date formats were typed as `int` / `str`, but encoding.go always calls `.timestamp()` / `.strftime()`, assuming `datetime`. Aligned on `datetime` for every timestamp_format — the format only affects the wire encoding, not the user-facing type. - message.go: WKT message-kind fields (Timestamp, Duration, FieldMask, Any, Empty, Struct, scalar wrappers) routed through the scalar to_dict path were emitted unconditionally, even though they are always nullable in proto3. Guard them like proto3 `optional` scalars so the encoder never sees a `None` default and raises AttributeError. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(py-client): add helper unit tests + extract repeated string constants Covers the pure helpers that the golden tests exercise only indirectly: - snakeCase: CamelCase → snake_case method-name conversion - headerOptionName: HTTP header → Python kwarg, with keyword-collision escape (X-Class → class_) - escapePyKeyword: hard + soft Python 3.10 keywords - formatPyStringSet: empty input emits set(), not the dict-literal {} - stripOptional, camelToSnake, isInvalidIdentifier: small string utils Also lifts three repeated literals into constants (pyFalse, pyEmptySet, pyListStr) so goconst is happy and there is one place to change the emitted Python idiom for each. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(py-client): preserve original proto enum value names for wire parity The generator was stripping the enum-name prefix and lowercasing each variant ("PRIORITY_HIGH" -> "high"). That was an ergonomic improvement on paper but broke cross-generator wire compatibility: the Go server emits enums via protojson default ("PRIORITY_HIGH"), while _encode_enum_X falls back to IntEnum.name, which the renaming had turned into "high". A Python client talking to a Go server was always going to misparse enum-typed fields. Keep the proto value name verbatim ("PRIORITY_HIGH") so .name and the wire format agree. Users write Priority.PRIORITY_HIGH which is also PEP 8-conformant (UPPER_CASE for enum members). Removes the now-orphaned camelToSnake/isInvalidIdentifier helpers and their unit tests. Verified end-to-end against the python-client-demo Go server: enums round-trip correctly across CRUD, query filtering, and the unwrap response path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(py-client): add examples/python-client-demo end-to-end demo Mirrors examples/ts-client-demo section-by-section so a reader can compare the two client surfaces directly. Shares the proto + Go HTTP server with the TS demo (NoteService — CRUD over Notes with enums, maps, optional fields, headers, query params, validation, unwrap response, and a typed NotFoundError). The Python client demonstrates: - Section 1: NoteServiceClientOptions with typed kwargs for service headers (api_key, tenant_id) and a default_headers escape hatch - Section 2: every HTTP verb (GET/POST/PUT/PATCH/DELETE) with path params, request bodies, and method-level headers via call options - Section 3: query parameter encoding for ListNotes (status/priority/sort/limit/offset) - Section 4: header layering (service options vs call options vs per-call headers dict, and per-call override of a service header) - Section 5: ValidationError parsing on min_len / max_len / missing required header — same buf.validate rules as the TS demo - Section 6: typed NotFoundError exception subclass (not a generic ApiError) chosen by the _ERROR_CLASSES registry from response shape - Section 7: custom HttpTransport injection (logging middleware) and the unwrap response path (NoteList.notes flattens on the wire) Verified end-to-end against the Go server: `make demo` runs the full suite cleanly with no failures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(py-client): add docs/python-generation.md, register plugin in README + CLAUDE.md Adds the dedicated Python client reference and wires protoc-gen-py-client into the toolkit overview in README.md and CLAUDE.md (now six plugins, not five). docs/python-generation.md covers: generator output (dataclasses, IntEnum, transport Protocol, error hierarchy, options, client class), transport injection, URL building (path + query params), header layering, ApiError/ValidationError/typed *Error exceptions, every JSON-mapping annotation (with focus on Timestamp/int64/bytes/oneof — same wire format as the Go and TS generators), Python keyword escaping, the SSE NotImplementedError stub, known limitations, and a link to examples/python-client-demo. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: remove Python client handoff docs Both planning docs were time-limited handoffs between agents working on this branch (PYTHON_CLIENT_REWRITE.md → the rewrite plan after PR #132 was closed; PY_CLIENT_HANDOFF.md → the Phase 2 testing/docs/PR handoff). The work they tracked is now landed on this branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(py-client): emit enums before error classes to avoid NameError When an *Error message has an enum-typed field, the generated default expression — code: Reason = Reason.X — is evaluated at class-definition time, so the enum class must already be declared. The previous file ordering emitted writeErrors before the enum loop, raising NameError at import time for any error that referenced an enum. Reorder the file so enums are written before writeErrors. Messages already trailed both blocks and don't need adjustment — message-typed defaults are always None, so forward references in them are safe. Add a regression case to testdata/proto/errors.proto (EventError + RejectionReason) matching the exact shape @yashagarwal-sarwa reported on #172, and upgrade the golden test to actually execute each generated file via importlib (ast.parse only checks syntax, not runtime NameErrors). The new import check also registers the module in sys.modules so @DataClass machinery can resolve string annotations from `from __future__ import annotations`. Reported-by: @yashagarwal-sarwa Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(py-client): add from_dict() to generated *Error classes When an *Error message is embedded as a field on another message, the parent's generated from_dict calls EventError.from_dict(...) — but error classes only had populate() and to_dict(), so the call raised AttributeError at runtime. Add a from_dict classmethod on every *Error class that delegates to populate() with neutral status/body/headers. This keeps the error class shape interchangeable with regular messages for serialization purposes, which is what the parent message's deserializer assumes. Extend errors.proto with EventResult { EventError error } as a regression case matching the exact shape @yashagarwal-sarwa reported on #172. The import test (added in 0166439) catches the AttributeError on the next regen attempt. Reported-by: @yashagarwal-sarwa Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(py-client): TIMESTAMP_FORMAT_DATE decode, flatten wire names, root-map unwrap Three generator bugs surfaced by the new examples/python-encoding-demo end-to-end round-trip against a real Go server: 1. decodeTimestampExpr for TIMESTAMP_FORMAT_DATE returned the raw "YYYY-MM-DD" string instead of a datetime, even though the field type is datetime. Now parses with datetime.strptime so the assigned value matches the declared annotation. 2. The Python flatten encoder iterated nested.to_dict().items() and prefix-tagged each key — which used JSON names (camelCase). The Go HTTP plugin's flatten encoder uses proto names (snake_case), so the Python side emitted `author_zipCode` while the server emits `author_zip_code`, breaking round-trips. Rewritten to emit one wire key per nested field using the field's proto name, with the matching decoder reading those keys directly. Encoder + decoder now agree with the Go server byte for byte. 3. annotations.FindUnwrapField is documented as a list-only helper, but py-client's root-unwrap codepaths called it for messages whose unwrap field is a map. That silently produced empty `to_dict() -> {}` / `from_dict() -> cls()` on every root-map unwrap message. Added a local findRootUnwrapField that doesn't filter on IsList(); kept the shared helper unchanged so other generators stay untouched. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(py-client): add python-encoding-demo + python-errors-demo Two new end-to-end examples that round-trip every protoc-gen-py-client feature (except SSE, tracked as #167) against a real Go server. Each example follows the established repo pattern — single focus, Go server + Python client + `make demo` target. examples/python-encoding-demo (51 assertions) Round-trips every JSON-mapping annotation: enum_value override, timestamp_format (RFC3339/UNIX_S/UNIX_MS/DATE), int64_encoding STRING+NUMBER, bytes_encoding base64+HEX, flatten+flatten_prefix, oneof_config nested + flattened variants, all three unwrap variants (root repeated, root map, map-value), Python keyword field-name escaping (`from`/`class`/`return`), and repeated query parameters. Each annotation lives on its own message because the Go HTTP plugin emits one MarshalJSON method per (message, annotation) and would produce duplicate methods otherwise. Writing this demo surfaced three real generator bugs that were invisible to the golden tests (they pass ast.parse and import-time exec but never check wire-format compatibility with the Go side). Fixes shipped in the preceding commit. examples/python-errors-demo (41 assertions) Covers every error surface: ValidationError parsed from a buf.validate body, registry-based disambiguation across NotFoundError / ConflictError / RateLimitError, and an *Error embedded as a field on a regular response (the BatchCreateItemResult pattern from @yashagarwal-sarwa's #172 — exercises the FieldValidationError. from_dict alias that lives alongside populate()). CLAUDE.md updated to list both examples in the project-structure section. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
This PR extracts the Python generator work from #126.
Included here:
protoc-gen-py-clientWhy
The Python generator uses the shared
internal/contractmodelintroduced in #131, so this PR is intentionally stacked on top of that base instead of duplicating the shared traversal layer.What Changed
cmd/protoc-gen-py-clientinternal/pyclientgenTests
env GOTOOLCHAIN=auto go test ./internal/pyclientgenenv GOTOOLCHAIN=auto go test ./cmd/protoc-gen-py-client