Fix set_many examples to use list[FieldUpdate]#138
Merged
Conversation
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
added
size: M
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
…d-update # Conflicts: # docs/guide/async.md # sdk/docs/quickstart.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
set_many()examples inquickstart.md,async.md, and the runnableasync-clientexample used a plaindict, but the real signature requireslist[FieldUpdate]— calling it as documented raisesAttributeErrorat runtime because the SDK iteratesu.field_path.FieldUpdate's optionalexpected_checksum(optimistic concurrency) andvalue_description(per-value metadata) fields, which weren't documented anywhere.examplesCI job only ranpy_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
sdk/docs/quickstart.md,sdk/docs/async.md, andexamples/async-client/main.pyto construct[FieldUpdate(...), ...]instead of adict, matching the SDK's actualset_many(self, tenant_id, updates: list[FieldUpdate], ...)signature and itsfor u in updates: ... u.field_pathiteration.quickstart.mddocumentingFieldUpdatefields (field_path,value,expected_checksum,value_description) based on the dataclass docstrings insdk/src/opendecree/types.py.examplesCI job to build and start a live decree server via docker-compose, build thedecreeCLI to seed example data, and runmake test(the existingexamples/*/test_*.pysmoke tests) — so a broken example like this one fails CI instead of silently passingpy_compile.make lintandmake testlocally (via the pre-commit checklist) — ruff check/format clean, 298 passed / 13 skipped, 100% coverage.Closes #133