Skip to content

Chris/schy 45 datastreamreplicator for python#58

Open
cbrady wants to merge 16 commits intomainfrom
chris/schy-45-datastreamreplicator-for-python
Open

Chris/schy 45 datastreamreplicator for python#58
cbrady wants to merge 16 commits intomainfrom
chris/schy-45-datastreamreplicator-for-python

Conversation

@cbrady
Copy link
Collaborator

@cbrady cbrady commented Mar 25, 2026

No description provided.

@cbrady cbrady force-pushed the chris/schy-45-datastreamreplicator-for-python branch from cf3013c to 69d2a75 Compare March 26, 2026 00:30
@cbrady cbrady marked this pull request as ready for review March 26, 2026 01:17
@cbrady cbrady requested a review from a team as a code owner March 26, 2026 01:17
Copy link
Collaborator

@bpapillon bpapillon left a comment

Choose a reason for hiding this comment

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

Can we avoid checking in the wasm file? I remember we had a similar convo about this during one of the last SDKs, my though was that since we have another repo responsible for producing that wasm binary, we should be able to pull it in at build time.

Related - we should probably figure out a way to do some kind of build test, as I'm not certain how the packaging and publishing system here determines which files to include. We need to make sure that the wasm file, however we pull it in, ends up getting shipped with the published Python package.

pyproject.toml Outdated
pydantic = ">= 1.9.2"
pydantic-core = ">=2.18.2"
typing_extensions = ">= 4.0.0"
websockets = ">=10.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

wasmtime is marked as optional, but websockets are not

I would think those two dependencies are required under similar circumstances, i.e. they should either both be optional or neither be optional

that being said, wasmtime appears to be quite a large dep while websockets appears to be quite a small one, so you may have already considered this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah i hadn't considered that because it didn't add anything substantial to the package, but might as well because they are tied todether.




def _build_cache_key(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This was kind of a pre-existing problem, I think, but we should probably improve this cache key function t be more deterministic as some of the other implementations are

https://github.com/SchematicHQ/schematic-api/blob/main/docs/docs/client-libraries/sdk-spec.md#cache-key-format

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