Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add support for decorating async functions #33

Merged
merged 17 commits into from
May 10, 2023
Merged

Add support for decorating async functions #33

merged 17 commits into from
May 10, 2023

Conversation

brettimus
Copy link
Collaborator

@brettimus brettimus commented May 9, 2023

This PR adds support for async functions, making the following changes:

  • Conditionally return a sync or async wrapped function, depending on whether the decorated function returns true from iscoroutinefunction
  • Write an additional overload for the autometrics decorator, using Callable[P, Awaitable[T]] as part of the type signature
  • Factor out shared logic from decorator and async_decorator
  • Add a test of a simple async function in test_decorator.py (which required adding the pytest-asyncio module to dev dependencies)
  • Add async code to the fastapi-example.py example

Fixes #29

@brettimus brettimus requested a review from actualwitch May 9, 2023 19:33
)
# Reraise exception
raise exception

"""Decorator for tracking function calls and duration."""

def decorator(func: Callable[P, T]) -> Callable[P, T]:
module_name = get_module_name(func)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this logic might not need to live here too — i could try factoring out of deocrator and async_decorator

Copy link
Member

@flenter flenter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Got a couple of minor points of feedback though

Copy link
Member

@flenter flenter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀


if func is None:
return decorator
return sync_decorator
Copy link
Collaborator Author

@brettimus brettimus May 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm actually not quite sure what this accomplishes! now that we have two different decorators (one for sync, one for async) i am a slightly worried about returning the sync decorator here when the func arg is None.

i did try a version of this PR that used one decorator, but to get the value of an async function synchronously, you have to do some tricks with the event loop which seemed like they were against best practices. e.g.,:

import asyncio

async def async_function():
    # Your async code here
    return result

def sync_function():
    loop = asyncio.get_event_loop()
    result = loop.run_until_complete(async_function())
    return result

anyways, we don't seem to document this case (where the decorator is called without being attached to a function), so this might not be a problem

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of returning different decorators, would it be possible to use only one and check whether the result is a coroutine?
https://docs.python.org/3/library/inspect.html#inspect.iscoroutine

@brettimus brettimus merged commit b2c878d into main May 10, 2023
@brettimus brettimus deleted the support-async branch May 10, 2023 08:50
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for async functions
2 participants