Skip to content

feat: Add support for opaque Schema handles in Python - #103

Merged
skuenzli merged 9 commits into
k9securityio:mainfrom
swenger:swenger-schema
Jul 9, 2026
Merged

feat: Add support for opaque Schema handles in Python#103
skuenzli merged 9 commits into
k9securityio:mainfrom
swenger:swenger-schema

Conversation

@swenger

@swenger swenger commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds Schema, Schema.from_str, and Schema.from_json_str, following the same pattern as PolicySet and Entities.

Motivation

The same schema is usually passed to many different function calls, and each call parses it again. Pre-parsing the schema once into the (already existing) Rust Schema object lets us deduplicate this effort.

swenger added 2 commits July 8, 2026 13:25
## Summary

Adds `Schema`, `Schema.from_str`, and `Schema.from_json_str`, following the same pattern as `PolicySet` and `Entities`.

## Motivation

The same schema is usually passed to many different function calls, and each call parses it again. Pre-parsing the schema *once* into the (already existing) Rust `Schema` object lets us deduplicate this effort.
@swenger
swenger marked this pull request as ready for review July 8, 2026 11:32
@Iamrodos

Iamrodos commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

@swenger Looking good. Are you intending updates to the README documentation?

@swenger

swenger commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Looking good.

Thanks :)

Are you intending updates to the README documentation?

Good idea - give me a minute and I'll get back to you.

@swenger

swenger commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Are you intending updates to the README documentation?

Good idea - give me a minute and I'll get back to you.

@Iamrodos how's 82d2275?

@skuenzli

skuenzli commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Thank you for the PR @swenger! I think this is a good idea. I will review my morning.

@skuenzli

skuenzli commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Thanks @swenger — overall this PR looks good and is a strong performance win. The Schema handle follows the PolicySet/Entities handle conventions nicely: frozen, eager ValueError on construction, Source-first extraction in SchemaArg, and the string/dict paths unchanged. Tests pass locally and CI is green.

There are a few issues to resolve first:

  • Per-call Schema::clone() on the hot path. cedar_policy::Schema is not Arc-wrapped; it owns a ValidatorSchema full of HashMaps, so .cloned() is a deep copy. Both is_authorized_batch (schema_opt_for_entities.cloned()) and is_authorized_partial (schema_ref.cloned()) pay it on every call, including the plain-string path where no clone existed before. The handle is still a big win: I measured ~3,100µs down to ~55µs per call with a 300-entity-type schema. But the leftover clone cost grows with schema size (about 200µs/call at 3,000 entity types), and that's exactly the per-call cost this PR sets out to remove. It also runs inside the t_load_entities timer, so it inflates load_entities_duration_micros while parse_schema_duration_micros reads 0. The fix is to pass Option<&Schema> through EntitiesArg::resolve/resolve_partial, make_entities, load_entities, and execute_authorization_request instead of building an owned Option<Schema>; every downstream use only needs a reference. The clone in validate_policies is fine, since Validator::new takes Schema by value.
  • Typed API and code documentation. Please add the Schema class to cedarpy/_internal.pyi and widen the schema parameters there (currently still Optional[str]); typed callers won't see the new API otherwise. Please also mention the handle in the :param schema docstrings of the three authorization functions in cedarpy/__init__.py, matching how the policies and entities params document theirs.
  • Changelog. Please add an entry under [Unreleased] in CHANGELOG.md, following the pattern of the PolicySet/Entities handle entries.

Do you have any questions about the requested changes? Or would you like me to take care of any of them?

@swenger

swenger commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @skuenzli for the detailed feedback! Glad you caught the clones, which I completely missed...

Do you have any questions about the requested changes?

I don't think so, your explanations were great! Do the following commits address your concerns?

skuenzli and others added 2 commits July 9, 2026 13:59
Schema was the only handle that could not render itself: PolicySet.__str__
returns Cedar policy text and Entities.__str__ returns Cedar entities JSON,
but str(schema) fell back to the static repr. cedar_policy::Schema is a
one-way compilation with no render-back API, so the handle now parses to a
SchemaFragment first (which round-trips via to_cedarschema), compiles the
Schema from that fragment, and keeps the fragment for __str__. Rendering
failures report as <unrenderable Schema: ...> rather than raising, matching
Entities.__str__. __repr__ now reports entity-type/action counts, matching
the other handles' count-style reprs.

Also makes the CHANGELOG's str() claim accurate and precise, documents
__str__ in the typed API stub, and adds render/round-trip tests.

Co-Authored-By: Claude <noreply@anthropic.com>
…le changelog entry

Co-Authored-By: Claude <noreply@anthropic.com>
@skuenzli

skuenzli commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Looks great @swenger - thank you!

I upgraded the Schema __str__ and __repr__ implementations to parity with Entities and PolicySet.

I also credited you in the changelog entry.

@skuenzli

skuenzli commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Verified the clone fix locally — the handle path's per-call cost is now flat regardless of schema size. Before the fix it scaled linearly, because each call paid a deep copy of the ValidatorSchema.

Measured with release builds, pre-parsed PolicySet/Entities handles, so schema handling is the only variable:

Schema (as handle) Before fix After fix
none (baseline) 32.6 µs/call 33.0 µs/call
~4 entity types 33.7 µs/call 34.5 µs/call
300 entity types 51.2 µs/call 34.2 µs/call
3,000 entity types 199.5 µs/call 35.4 µs/call

The metrics are honest now too: load_entities_duration_micros previously absorbed the hidden clone (368µs on a call with a pre-parsed Entities handle); it now reads 0. For context, passing the 300-entity-type schema as a string costs ~3,100µs/call, so the handle is roughly a 90× reduction on that workload.

@skuenzli
skuenzli merged commit 5fa17a7 into k9securityio:main Jul 9, 2026
8 checks passed
@swenger

swenger commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @skuenzli for the review and your additions!

Could you tag a release? I'd then package it for conda-forge.

@swenger
swenger deleted the swenger-schema branch July 10, 2026 08:33
@skuenzli

Copy link
Copy Markdown
Contributor

Yes, kicking off the release process now.

skuenzli added a commit that referenced this pull request Jul 10, 2026
Add two Cedar API gotchas: Schema::clone() is a deep copy (borrow
Option<&Schema> on hot paths) and Schema has no render-back API while
SchemaFragment round-trips (how PySchema.__str__ works). Add the
five-surface checklist for public API changes and the shared handle-trio
pattern to Conventions, and a benchmark load-sensitivity note.

Remove the Follow-on work section: GH #62 (supply-chain hardening) and
GH #69 (benchmark process) are both complete and closed. The durable
gate-on-medians finding from #69 moves to the Benchmarks bullets.

Co-Authored-By: Claude <noreply@anthropic.com>
@skuenzli

Copy link
Copy Markdown
Contributor

@swenger the new Schema handle is now available in the new cedar-py v4.8.7 release

@swenger

swenger commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

@swenger the new Schema handle is now available in the new cedar-py v4.8.7 release

Awesome, thanks @skuenzli!

@skuenzli

Copy link
Copy Markdown
Contributor

Thank you @swenger ! Have a great weekend!

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.

3 participants