-
Notifications
You must be signed in to change notification settings - Fork 507
(Do not merge yet) Support step context from Python Workflows SDK #5634
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -20,6 +20,8 @@ | |||||||||||||
| from types import LambdaType | ||||||||||||||
| from typing import Any, Never, Protocol, TypedDict, Unpack | ||||||||||||||
|
|
||||||||||||||
| import _cloudflare_compat_flags | ||||||||||||||
|
|
||||||||||||||
| # Get globals modules and import function from the entrypoint-helper | ||||||||||||||
| import _pyodide_entrypoint_helper | ||||||||||||||
| import js | ||||||||||||||
|
|
@@ -1130,28 +1132,60 @@ def __init__(self, js_step): | |||||||||||||
| self._js_step = js_step | ||||||||||||||
| self._memoized_dependencies = {} | ||||||||||||||
| self._in_flight = {} | ||||||||||||||
| self.step_closures = {} | ||||||||||||||
|
|
||||||||||||||
| def do(self, name, depends=None, concurrent=False, config=None): | ||||||||||||||
| def do(self, name=None, *, depends=None, concurrent=False, config=None): | ||||||||||||||
| def decorator(func): | ||||||||||||||
| async def wrapper(): | ||||||||||||||
| # if implicit params are enabled, each param that is not context should be treated as a dependency and resolved | ||||||||||||||
| # In other words, we introspect the declaration and call a function (need to make sure it's a step) with the same name as the corresponding param | ||||||||||||||
| # This new code path should discard depends, as we encourage users to implicitly declare their invariant steps in the signature | ||||||||||||||
| # if the compat flag is disabled, then we just maintain the same legacy behavior | ||||||||||||||
Caio-Nogueira marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
| results_future_list = depends | ||||||||||||||
|
|
||||||||||||||
| if python_from_rpc(_cloudflare_compat_flags)[ | ||||||||||||||
| "python_workflows_implicit_dependencies" | ||||||||||||||
| ]: | ||||||||||||||
|
Comment on lines
+1146
to
+1148
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
| if depends is not None: | ||||||||||||||
| TypeError( | ||||||||||||||
| "Received unexpected parameter depends. This was deprecated and dependencies can be declared using callable names" | ||||||||||||||
| ) | ||||||||||||||
|
Comment on lines
+1150
to
+1152
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Forgot the
Suggested change
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But I think you might try defining different an entirely different method depending on whether this flag is on or not: |
||||||||||||||
| sig = inspect.signature(func) | ||||||||||||||
| results_future_list = [] | ||||||||||||||
| for p in sig.parameters.values(): | ||||||||||||||
| if p.name in self.step_closures: | ||||||||||||||
| results_future_list.append(self.step_closures[p.name]) | ||||||||||||||
| elif p.name == "ctx": | ||||||||||||||
| results_future_list.append(p) | ||||||||||||||
|
|
||||||||||||||
| if concurrent: | ||||||||||||||
| results = await gather( | ||||||||||||||
| *[self._resolve_dependency(dep) for dep in depends or []] | ||||||||||||||
| *[ | ||||||||||||||
| self._resolve_dependency(dep) | ||||||||||||||
| for dep in results_future_list or [] | ||||||||||||||
| ] | ||||||||||||||
| ) | ||||||||||||||
| else: | ||||||||||||||
| results = [ | ||||||||||||||
| await self._resolve_dependency(dep) for dep in depends or [] | ||||||||||||||
| await self._resolve_dependency(dep) | ||||||||||||||
| for dep in results_future_list or [] | ||||||||||||||
| ] | ||||||||||||||
| python_results = [python_from_rpc(result) for result in results] | ||||||||||||||
| return await _do_call(self, name, config, func, *python_results) | ||||||||||||||
| python_results = [ | ||||||||||||||
| result | ||||||||||||||
| if (hasattr(result, "name") and result.name == "ctx") | ||||||||||||||
|
Comment on lines
+1174
to
+1175
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a comment explaining why we do this? |
||||||||||||||
| else python_from_rpc(result) | ||||||||||||||
| for result in results | ||||||||||||||
| ] | ||||||||||||||
| step_name = func.__name__ if name is None else name | ||||||||||||||
| return await _do_call(self, step_name, config, func, *python_results) | ||||||||||||||
|
|
||||||||||||||
| wrapper._step_name = name | ||||||||||||||
| self.step_closures[name] = wrapper | ||||||||||||||
| return wrapper | ||||||||||||||
|
|
||||||||||||||
| return decorator | ||||||||||||||
|
|
||||||||||||||
| def sleep(self, *args, **kwargs): | ||||||||||||||
| # all types should be primitives - no need for explicit translation | ||||||||||||||
| return self._js_step.sleep(*args, **kwargs) | ||||||||||||||
|
|
||||||||||||||
| def sleep_until(self, name, timestamp): | ||||||||||||||
|
|
@@ -1170,7 +1204,9 @@ def wait_for_event(self, name, event_type, /, timeout="24 hours"): | |||||||||||||
| ) | ||||||||||||||
|
|
||||||||||||||
| async def _resolve_dependency(self, dep): | ||||||||||||||
| if dep._step_name in self._memoized_dependencies: | ||||||||||||||
| if hasattr(dep, "name") and dep.name == "ctx": | ||||||||||||||
| return dep | ||||||||||||||
| elif dep._step_name in self._memoized_dependencies: | ||||||||||||||
| return self._memoized_dependencies[dep._step_name] | ||||||||||||||
| elif dep._step_name in self._in_flight: | ||||||||||||||
| return await self._in_flight[dep._step_name] | ||||||||||||||
|
|
@@ -1179,8 +1215,13 @@ async def _resolve_dependency(self, dep): | |||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| async def _do_call(entrypoint, name, config, callback, *results): | ||||||||||||||
| async def _callback(): | ||||||||||||||
| result = callback(*results) | ||||||||||||||
| async def _callback(ctx=None): | ||||||||||||||
| # deconstruct the actual ctx object | ||||||||||||||
| resolved_results = tuple( | ||||||||||||||
| python_from_rpc(ctx) if hasattr(r, "name") and r.name == "ctx" else r | ||||||||||||||
| for r in results | ||||||||||||||
| ) | ||||||||||||||
| result = callback(*resolved_results) | ||||||||||||||
|
|
||||||||||||||
| if inspect.iscoroutine(result): | ||||||||||||||
| result = await result | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| # Copyright (c) 2025 Cloudflare, Inc. | ||
| # Licensed under the Apache 2.0 license found in the LICENSE file or at: | ||
| # https://opensource.org/licenses/Apache-2.0 | ||
|
|
||
| from workers import WorkflowEntrypoint | ||
|
|
||
|
|
||
| class PythonWorkflowDepends(WorkflowEntrypoint): | ||
| # The purpose of this workflow is testing that depends are no longer resolved | ||
| # | ||
| async def run(self, event, step): | ||
| @step.do("step_1") | ||
| async def step_1(): | ||
| # tests backwards compat with workflows that don't have ctx in the step callback | ||
| print("Executing step 1") | ||
| return "foo" | ||
|
|
||
| @step.do("step_2") | ||
| async def step_2(): | ||
| print("Executing step 2") | ||
| return "bar" | ||
|
|
||
| @step.do("step_3", depends=[step_1, step_2], concurrent=True) | ||
| async def step_3(result1=(), result2=()): | ||
| assert result1 == "foo" | ||
| assert result2 == "bar" | ||
| return result1 + result2 | ||
|
|
||
| return await step_3() | ||
|
|
||
|
|
||
| async def test(ctrl, env, ctx): | ||
| pass |
Uh oh!
There was an error while loading. Please reload this page.