Skip to content

feat(python): add Python contract generator#132

Closed
elzalem wants to merge 4 commits into
split/csharp-generatorfrom
split/python-generator
Closed

feat(python): add Python contract generator#132
elzalem wants to merge 4 commits into
split/csharp-generatorfrom
split/python-generator

Conversation

@elzalem

@elzalem elzalem commented Mar 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR extracts the Python generator work from #126.

Included here:

  • add protoc-gen-py-client
  • add Python golden coverage

Why

The Python generator uses the shared internal/contractmodel introduced in #131, so this PR is intentionally stacked on top of that base instead of duplicating the shared traversal layer.

What Changed

  • add cmd/protoc-gen-py-client
  • add internal/pyclientgen

Tests

  • env GOTOOLCHAIN=auto go test ./internal/pyclientgen
  • env GOTOOLCHAIN=auto go test ./cmd/protoc-gen-py-client

@elzalem
elzalem requested a review from SebastienMelki March 2, 2026 19:15

@SebastienMelki SebastienMelki left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. Documentation: Top-level docs (README/CLAUDE.md updates) explaining the Python generator, plus generator-specific docs covering all supported features, options, and limitations.

  2. 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.

  3. 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 contractmodel also needs its own unit test suite (see #131 review)
  4. Plugin completeness: For something named protoc-gen-py-client, users expect an actual HTTP client — not just dataclasses. Either implement client functionality or rename to protoc-gen-py-contracts to set expectations correctly.

@elzalem
elzalem requested a review from SebastienMelki March 4, 2026 09:10
@SebastienMelki
SebastienMelki requested a review from shavaizknz May 19, 2026 08:38
@SebastienMelki

Copy link
Copy Markdown
Owner

@shavaizknz this pull request adds python support on the client side

@SebastienMelki

Copy link
Copy Markdown
Owner

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 contractmodel / C# work, which has its own design questions we want to discuss separately. I'm going to close this PR and reopen a fresh one based directly on main, following the same pattern as our existing tsclientgen and clientgen generators (calling internal/annotations/ directly rather than going through an intermediate model layer).

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 contractmodel discussion alive on #131 — I'll come back to that thread once Python is out.

Thanks again for the contribution, and sorry about the churn here.

SebastienMelki added a commit that referenced this pull request May 19, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants