Skip to content

Fix set_many examples to use list[FieldUpdate] - #138

Merged
zeevdr merged 11 commits into
mainfrom
fix/133-set-many-field-update
Jun 8, 2026
Merged

Fix set_many examples to use list[FieldUpdate]#138
zeevdr merged 11 commits into
mainfrom
fix/133-set-many-field-update

Conversation

@zeevdr

@zeevdr zeevdr commented Jun 7, 2026

Copy link
Copy Markdown
Member

Summary

  • set_many() examples in quickstart.md, async.md, and the runnable async-client example used a plain dict, but the real signature requires list[FieldUpdate] — calling it as documented raises AttributeError at runtime because the SDK iterates u.field_path.
  • Documentation should match the actual API so readers can copy-paste working code, and a "Bulk writes" section is needed to explain FieldUpdate's optional expected_checksum (optimistic concurrency) and value_description (per-value metadata) fields, which weren't documented anywhere.
  • The examples CI job only ran py_compile (a syntax check), so this runtime bug stayed green; executing the examples against a live server closes that gap for future regressions.

Test plan

  • Fixed sdk/docs/quickstart.md, sdk/docs/async.md, and examples/async-client/main.py to construct [FieldUpdate(...), ...] instead of a dict, matching the SDK's actual set_many(self, tenant_id, updates: list[FieldUpdate], ...) signature and its for u in updates: ... u.field_path iteration.
  • Added a "Bulk writes" subsection to quickstart.md documenting FieldUpdate fields (field_path, value, expected_checksum, value_description) based on the dataclass docstrings in sdk/src/opendecree/types.py.
  • Extended the examples CI job to build and start a live decree server via docker-compose, build the decree CLI to seed example data, and run make test (the existing examples/*/test_*.py smoke tests) — so a broken example like this one fails CI instead of silently passing py_compile.
  • Ran make lint and make test locally (via the pre-commit checklist) — ruff check/format clean, 298 passed / 13 skipped, 100% coverage.

Closes #133

set_many() examples in quickstart.md, async.md, and the runnable
async-client example used a plain dict, but the real signature requires
list[FieldUpdate] and raises AttributeError at runtime when iterated.

- Fix all three call sites to construct FieldUpdate objects.
- Add a "Bulk writes" subsection to quickstart.md documenting FieldUpdate,
  including expected_checksum (optimistic concurrency) and
  value_description (per-value metadata).
- Extend the examples CI job to start a live decree server, seed it, and
  run each example end-to-end (not just py_compile), so a broken example
  like this one fails CI instead of silently passing.

Closes #133
@zeevdr zeevdr added this to the Beta Readiness milestone Jun 7, 2026
@zeevdr zeevdr added size: M Moderate — a day or two, clear scope priority: P0 Blocks alpha or release labels Jun 7, 2026
@zeevdr zeevdr added this to OpenDecree Jun 7, 2026
@codecov

codecov Bot commented Jun 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

zeevdr added 7 commits June 7, 2026 20:19
The decree CLI's global flag for the server address is --server (env
DECREE_SERVER), not --addr. setup.py passed --addr, which cobra rejects
as an unknown flag. Because the root command sets SilenceErrors: true
and main.go exits without printing the error, the failure surfaced as
an empty 'Error seeding: ' message — only visible once CI actually
exercised this path (added in this PR for #133).

Refs #133
The dockerized decree server in CI/local dev runs without TLS, but the
CLI defaults to TLS (--insecure must be explicitly set, mirroring how
the SDK clients and integration tests connect with insecure=True).
Without it, dialServer's TLS handshake fails and the silenced cobra
error surfaces only as an empty 'Error seeding: ' message.

Refs #133
The seeding step was failing for three independent reasons, each
discovered by actually running it against a live server (the new
'Examples' CI job exercises this path for the first time):

1. The CLI's server-address flag is --server (env DECREE_SERVER), not
   --addr — cobra rejected the unknown flag, and because the root
   command sets SilenceErrors/SilenceUsage with no fallback print in
   main.go, the failure surfaced only as an empty 'Error seeding: '.
2. The local server runs in plaintext (INSECURE_LISTEN=1); the CLI
   defaults to TLS, so dialing requires --insecure (mirrors how the
   SDK's own integration tests connect with insecure=True).
3. The server rejects requests with no x-subject header as
   unauthenticated, and the CLI sets none by default — needs --subject.
4. Tenants can only be created against a published schema version, and
   imported schema versions start as unpublished drafts — needs
   --auto-publish.

Also switched the tenant-ID parsing from matching a 'Tenant: <id>'
line (a format the CLI has never produced — its seed output is a
RESOURCE/ID/CREATED/DETAILS table) to --output json, which gives a
stable [][]string the script can parse directly.

Verified end-to-end against a live decree server via docker-compose:
schema import + auto-publish + tenant creation + config import all
succeed and .tenant-id is written correctly.

Refs #133
quickstart and async-client demoed set()/set_many() against bool and
integer fields (app.debug, server.rate_limit). The SDK's
make_string_typed_value() always wraps values as TypedValue(string_value=...),
but the server validates the oneof variant against the field's declared
type with no coercion — so these calls always raised
InvalidArgumentError: expected bool/integer value.

Switch both examples to string fields (app.name, payments.currency),
which exercise the same list[FieldUpdate] syntax (the actual point of
this fix) without hitting the deeper SDK defect. That defect is filed
separately.
The Makefile's EXAMPLES list already included live-config, but no
test_*.py existed for it — so 'make test' (now run by CI's Examples
job against a live server) failed with "file or directory not found:
test_*.py".

The example blocks on a changes() iterator, so the test starts it,
waits for the initial "Current values"/"Watching for changes" output,
then sends SIGINT (which the example's own handler turns into a clean
sys.exit(0)) rather than expecting a returncode of 0.
examples/error-handling/main.py demonstrates set_null()/nullable=True
on app.debug, but the seed schema never declared it nullable — so
set_null() failed live with "field app.debug: value is required (not
nullable)". The example was never exercised against a live server
before this PR added that CI step.
After demonstrating set_null()/nullable=True on app.debug, the example
tried to restore it via client.set(tenant_id, "app.debug", "false") —
which hits the same pre-existing set()/set_many() type-coercion defect
as #139 (make_string_typed_value always sends string_value; the server
requires bool_value for a bool field, raising "expected bool value").

The restore step wasn't load-bearing for the nullable demo (set_null +
get already shows the behavior), so drop it rather than work around the
defect here too.
@zeevdr
zeevdr enabled auto-merge (squash) June 8, 2026 07:13
@zeevdr
zeevdr disabled auto-merge June 8, 2026 07:13
@zeevdr
zeevdr enabled auto-merge (squash) June 8, 2026 10:49
@zeevdr
zeevdr merged commit 1e8c431 into main Jun 8, 2026
13 checks passed
@zeevdr
zeevdr deleted the fix/133-set-many-field-update branch June 8, 2026 10:52
@github-project-automation github-project-automation Bot moved this from Todo to Done in OpenDecree Jun 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: P0 Blocks alpha or release size: M Moderate — a day or two, clear scope

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

docs: set_many examples use dict, need list[FieldUpdate] (crashes)

1 participant