Attempt to add memo functions to durable context for TS and Py SDKs#2921
Attempt to add memo functions to durable context for TS and Py SDKs#2921mnafees wants to merge 6 commits intonafees/go-sdk-memofrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| async def aio_memo( | ||
| self, fn: Callable[[], Awaitable[Any]], deps: list[Any] | ||
| ) -> Any: |
There was a problem hiding this comment.
this is going to be a bit of a tricky one to get right, I think 🤔. we probably want both memo and aio_memo, where memo takes a regular sync function and aio_memo takes an async one. In this case, we should have some generic like TMemo = TypeVar("TMemo"), and then have this signature be:
async def aio_memo(
self, fn: Callable[[], Awaitable[TMemo]], deps: list[Any]
) -> TMemo:and then for sync memo, it'd be:
def memo(
self, fn: Callable[[], TMemo], deps: list[Any]
) -> TMemo:
|
|
||
| key = _compute_memo_key(self.action.action_id, deps) | ||
|
|
||
| resp = self.durable_event_listener.get_durable_event_log( |
There was a problem hiding this comment.
this needs to be wrapped in asyncio.to_thread so it doesn't block
| ) | ||
|
|
||
| if resp.found: | ||
| return json.loads(resp.data) |
| data = json.dumps(result).encode() | ||
|
|
||
| self.durable_event_listener.create_durable_event_log( | ||
| external_id=self.workflow_run_id, | ||
| key=key, | ||
| data=data, | ||
| ) |
There was a problem hiding this comment.
and to_thread for these ones :)
| def _compute_memo_key(step_name: str, deps: list[Any]) -> str: | ||
| h = hashlib.sha256() | ||
| h.update(step_name.encode()) | ||
| h.update(json.dumps(deps).encode()) |
There was a problem hiding this comment.
this seems a bit risky to me because people can put in whatever they want, but I'm not sure how better to do it. we might want json.dumps(deps, default=str) just in case to try to avoid errors from things that aren't serializable
|
|
||
| if resp.found: | ||
| return json.loads(resp.data) | ||
| return json.loads(resp.data) # type: ignore[no-any-return] |
There was a problem hiding this comment.
it's a small thing, but I have a slight preference for this (we do it elsewhere) because it's clearer when reading the code what's going on:
return cast(TMemo, json.loads(resp.data))
|
|
||
| result = await fn() | ||
|
|
||
| data = json.dumps(result).encode() |
There was a problem hiding this comment.
to_thread for the json.dumps too :P
one thing I'm curious about here too is what happens if the function e.g. returns a pydantic model or a dataclass or something we can't call json.dumps on. I think we probably should support dataclasses, Pydantic, etc. but it makes the (de)serialization and typing trickier here, but it also makes the DX much better
2 participants
Description
Fixes # (issue)
Type of change
What's Changed