[Feature Request] Support non-function callables as activities

[llchan]

Is your feature request related to a problem? Please describe.

Currently the activity definition implementation here requires __code__ on a callable. From what I can tell it seems like it's only using that to verify that there are no kwonly arguments.

Describe the solution you'd like

We can instead use the inspect module to analyze the callable's signature, allowing a broader set of non-function callables to be used as activities.

[cretz]

Definitely can fix this. Can you give an example of which callable you are using and how so I can include it in the integration tests?

[nathanielobrown]

So this isn't 100% related but I'm just dumping my thoughts here in case they're useful when changing the @activity.defn decorator.

I think it is nice to have a thin wrapper around activities that allows the developer to more easily customize their activity functions. Dramatiq uses a wrapper class which is applied using a decorator. I like this setup because:

1. It makes it easy to mock out the underlying function without messing up the task related attributes and without creating an otherwise useless wrapper function. For Dramatiq you just mock task.fn
   • As it stands in this SDK, if you directly mock the function things fail because it's missing __temporal_activity_definition. This isn't the end of the world because you can just pass a stubbed actor to the worker when you test, but it still might be nice to have easy mocking
2. It makes the path to customization fairly clear and straightforward. Just subclass the Actor class
3. It's easier not to interfere with the internals of the framework when you customize how the callable is called, because the callable is an attribute of the wrapper, rather than the wrapper attributes stuff on the callable.

I'm still customizing how the current decorator is applied, it's just a little more complicated. Here's what I'm using in my project. It logs exceptions, injects a database access object and does some extra parsing of arguments that are Pydantic classes:

import functools
import inspect
from time import time
from typing import Any, Callable, no_type_check, Optional, TypeVar

from loguru import logger
from temporalio import activity

from bots_interface.actors import _parse_type
from dd.repositories import repo as repo_module

ActivityFunc = TypeVar("ActivityFunc", bound=Callable[..., Any])


@no_type_check
def define_activity(
    fn: ActivityFunc = None, name: Optional[str] = None
) -> ActivityFunc:
    if name is not None and fn is None:
        return functools.partial(define_activity, name=name)
    assert fn is not None
    defn = activity.defn(name=name or fn.__name__)(fn)
    fn_annotations = fn.__annotations__

    @functools.wraps(fn)
    def wrapper(*args: Any, **kwargs: Any) -> Any:
        if "repo" in fn_annotations:
            assert fn_annotations["repo"] is repo_module.Repository
            assert "repo" not in kwargs
            repo = repo_module.repo_factory()
            kwargs["repo"] = repo
        parsed_kwargs = _parse_args(fn, args, kwargs)
        if "repo" in fn_annotations:
            with repo.commit_and_close():
                return _run_func_log_exception(fn, parsed_kwargs)
        else:
            return _run_func_log_exception(fn, parsed_kwargs)

    wrapper.__temporal_activity_definition = defn.__temporal_activity_definition
    return wrapper


@no_type_check
def _run_func_log_exception(fn: ActivityFunc, kwargs: dict) -> Any:
    t_start = time()
    try:
        result = fn(**kwargs)
    except Exception:
        logger.exception(f"Activity function {fn.__name__!r} failed")
        raise
    duration = time() - t_start
    logger.debug(f"{fn.__name__} ran in {duration:.3f} seconds")
    return result


def _parse_args(fn: Callable, args: Any, kwargs: Any) -> dict:
    """Returns kwargs with args converted to kwargs and pydantic types parsed.
    Pendulum Datetimes are parsed as well"""
    call_args = inspect.getcallargs(fn, *args, **kwargs)
    for arg_name, value in call_args.items():
        annotation = fn.__annotations__.get(arg_name)
        call_args[arg_name] = _parse_type(annotation, value)
    return call_args

[cretz]

There is no customization beyond name. I just need a way to mark a callable as an activity and, at the time, an attribute seemed more reasonable than wrapping the function. This is what the @no_type_check decorator you're showing in your example does, see: https://github.com/python/cpython/blob/40d81fd63b46cf998880ce3bf3e5cb42bc3199c1/Lib/typing.py#L2476-L2509. This allows other decorators that actually do need to proxy the invocation to do so without affecting our simple need to just set a small piece of metadata.

I may introduce an alternative activity concept that can be used instead of the decorator, but again the only customization is name. And the only thing I need is simple metadata, I do not want to proxy the function. Is there a more Pythonic way to carry around metadata without proxying the invocation? (forgive my naivete on Python here)

[nathanielobrown]

I may introduce an alternative activity concept that can be used instead of the decorator, but again the only customization is name. And the only thing I need is simple metadata, I do not want to proxy the function. Is there a more Pythonic way to carry around metadata without proxying the invocation? (forgive my naivete on Python here)

Nothing more pythonic comes to mind. In fact, stuffing attributes on whatever object feels very Python...

I like what you're saying about keeping it simple, and I think if I want to have proxied/mockable/customizeable tasks I can still do that under this model with my own decorator. I realize now that my current decorator could be cleaned up so it doesn't care about what activity.defn does if I apply it at the end (activity.defn(name=name or fn.__name__)(wrapper)).