Introduce ActivityDefinition as a wrapper for Activity Fns #29
Conversation
natemort
force-pushed
the
activity_definition
branch
2 times, most recently
from
868b5d0 to
1735a9b
Compare
In order to use activity functions within a Workflow we need to have a very explicit name for the function that we can determine without the registry, as the activity may not even be present within the same registry as the workflow. Remove the concept of aliases, and mandate a decorator on all Activity functions. For ease of use, continue to support the decorator via the registry to both register and decorate the activity at the same time. In addition, rather than simply storing the activity function or setting attributes on it to track that it's registered, introduce a wrapping Callable. Within the context of a Workflow we can reinterpret invocations of the activity function as the execution of the activity itself. Signed-off-by: Nate Mortensen <natemort@uber.com>
natemort
force-pushed
the
activity_definition
branch
from
1735a9b to
edf65c0
Compare
| type_hint: Type | None | ||
| default_value: Any | None | ||
|
|
||
| class ExecutionStrategy(Enum): |
There was a problem hiding this comment.
nit: naming, are we going to have more than 2 types of enum here? if not, perhaps a naming of isAsync might be more descriptive here.
There was a problem hiding this comment.
I think there's a chance we might add multiprocessing support in the future.
| name = opts["name"] | ||
|
|
||
| strategy = ExecutionStrategy.THREAD_POOL | ||
| if inspect.iscoroutinefunction(fn) or inspect.iscoroutinefunction(fn.__call__): # type: ignore |
There was a problem hiding this comment.
if fn is enforced as a callable, what's the difference between those two if conditions?
There was a problem hiding this comment.
It covers some really weird edge cases. This PR to CPython actually has good examples: https://github.com/python/cpython/pull/99247/files
class Cl:
async def __call__(self):
pass
self.assertFalse(inspect.iscoroutinefunction(Cl))
# instances with async def __call__ are NOT recognised.
self.assertFalse(inspect.iscoroutinefunction(Cl()))
I don't really know why it works this way, but it does. The only likely scenario for someone to run into an issue with this is if they have other decorators on the function.
3 participants
In order to use activity functions within a Workflow we need to have a very explicit name for the function that we can determine without the registry, as the activity may not even be present within the same registry as the workflow. Remove the concept of aliases, and mandate a decorator on all Activity functions.
For ease of use, continue to support the decorator via the registry to both register and decorate the activity at the same time.
In addition, rather than simply storing the activity function or setting attributes on it to track that it's registered, introduce a wrapping Callable. Within the context of a Workflow we can reinterpret invocations of the activity function as the execution of the activity itself.
What changed?
__qualname__instead of__name__for activity functions. This is the name of the function relative to the module root, so it includes the Class name for activities that are methods. It seems more intuitive, although if the annotated functions are deeply nested their names may not be coherent.Why?
How did you test it?
Potential risks
Release notes
Documentation Changes