Conversation
cf3013c to
69d2a75
Compare
There was a problem hiding this comment.
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" |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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( |
There was a problem hiding this comment.
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
No description provided.