Repository navigation
Generate schemas as on-demand hydration from tuples - #841
Conversation
8ffafa1 to
8dbd922
Compare
| reportUnknownVariableType = false | ||
| reportUnknownMemberType = false | ||
| reportUnknownArgumentType = false | ||
| reportUnknownLambdaType = false |
There was a problem hiding this comment.
Why are these needed and can we have more isolated suppressions (e.g. per-file)? Or does the entire package need them?
There was a problem hiding this comment.
I'll look into fixing, since I'd rather not add suppressions
There was a problem hiding this comment.
suppressions are now inline
| traits[tid] = Trait.new(tid) | ||
| bit >>= 1 | ||
| i += 1 | ||
| _traits_cache[indicator] = traits |
There was a problem hiding this comment.
This is shared across schemas- would it be possible to return a copy or cache something immutable?
There was a problem hiding this comment.
this is already marked private - what is the scenario in which something mutates this incorrectly?
There was a problem hiding this comment.
More of a comment on Schema.traits- the private cached dict escapes through this public field, so mutating one schema can affect unrelated schemas
a39faa4 to
63a6307
Compare
…wide Replace the four reportUnknown* settings in the generated pyproject with inline # pyright: ignore comments emitted only on the recursive/forward-ref schema lines that pyright cannot type (the lambda member targets and the assignment lines holding them). Keeps strict type checking everywhere else in the generated client.
Use a thread-local in-progress map to break reference cycles instead of publishing an empty schema to the shared cache, so a reader never sees a half-built schema and a failed hydration leaves nothing cached. Add schema hydration tests.
c17caa2 to
0fc8991
Compare
| finally: | ||
| del stack[key] | ||
|
|
||
| _cache[key] = (data, schema) |
There was a problem hiding this comment.
this still looks like it can publish part of a recursive graph too early. if A → B → A, B can be cached while A is still being populated and another thread can cache an incomplete members_by_index for A. can we wait until the outermost hydration finishes before publishing anything from the graph?
jonathan343
left a comment
jonathan343
left a comment
There was a problem hiding this comment.
Two quick questions, not really suggestions. Otherwise looks good once Seans concerns are addressed
| } | ||
| } | ||
|
|
||
| private int memberIndex(Shape parent) { |
There was a problem hiding this comment.
nit - can this be shared? Or is it just easier to have the duplication? Commenting since I noticed this is repeated in the deserializer above
There was a problem hiding this comment.
it could be DRYed, yeah
| @cached_property | ||
| def schema(self) -> Schema: | ||
| """The schema of the operation.""" | ||
| from ._schema_compact import hydrate |
There was a problem hiding this comment.
Why are the imports here and not at the top of the file? Want to check my understanding
There was a problem hiding this comment.
it's a cycle-breaker.
stepping back a bit, I don't like how files have multiple exported symbols. I think class Schema should be in Schema.py and class APIOperation should be in APIOperation.py, but I'm leaving that existing style alone for now.
Hold every node of a connected schema graph out of the shared hydrate cache until the outermost frame completes, so a cross-referenced sibling is never published pointing at a half-built node. Extract AbstractMemberGenerator for the writeSchema/memberIndex logic shared by the member ser/deser generators.
3 participants
In the schemas.py file, generate schema declarations as a compact tuples instead of a constructor or factory invocation. This expands lazily to the
Schemaclass object, but only on-use, rather than on import of the client.The result is a reduction in the memory footprint of schemas (delayed init, compact standby mode), and faster startup.
Both the faster startup and the size-on-disk reduction help any caller, and usage on Lambda in particular.
Code example:
effect on clients
picture of schemas.py only