Skip to content

Commit

Permalink
Do not wrap async function with threaded_callback
Browse files Browse the repository at this point in the history
Async callbacks are being wrapped by ThreadedCoroutine when they shouldn't be. For some context, the purpose of ThreadedCoroutine is to allow sync callbacks with aiohttp client. When the callback is an async function, this condition should skip this behavior.

See #256
  • Loading branch information
prkumar committed Feb 22, 2022
1 parent d9aa5b2 commit 5a17ccd
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 6 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.rst
Expand Up @@ -6,6 +6,12 @@ All notable changes to this project will be documented in this file.
The format is based on `Keep a Changelog`_, and this project adheres to the
`Semantic Versioning`_ scheme.

Unreleased
==========
Fixed
-----
- Fix behavior of async ``@response_handler`` with ``AiohttpClient``. (`#256`_)

0.9.6_ - 2022-01-24
===================
Added
Expand Down
1 change: 1 addition & 0 deletions tests/conftest.py
Expand Up @@ -5,6 +5,7 @@
collect_ignore.extend(
[
"unit/test_aiohttp_client.py",
"integration/test_handlers_aiohttp.py",
"integration/test_retry_aiohttp.py",
]
)
48 changes: 48 additions & 0 deletions tests/integration/test_handlers_aiohttp.py
@@ -0,0 +1,48 @@
# Local imports.
import uplink

# Third-party imports
import aiohttp
import pytest
from uplink.clients.aiohttp_ import AiohttpClient

# Constants
BASE_URL = "https://example.com/"
SIMPLE_RESPONSE = "simple response"


@pytest.fixture
def mock_aiohttp_session(mocker):
return mocker.Mock(spec=aiohttp.ClientSession)


@uplink.response_handler
async def simple_async_handler(response):
return SIMPLE_RESPONSE


class Calendar(uplink.Consumer):
@simple_async_handler
@uplink.get("todos/{todo_id}")
def get_todo(self, todo_id):
pass


@pytest.mark.asyncio
async def test_simple_async_handler(mock_aiohttp_session, mock_response):
mock_response.status = 200

async def request(*args, **kwargs):
return mock_response

mock_aiohttp_session.request = request

calendar = Calendar(
base_url=BASE_URL, client=AiohttpClient(mock_aiohttp_session)
)

# Run
response = await calendar.get_todo(todo_id=1)

# Verify
assert response == SIMPLE_RESPONSE
8 changes: 4 additions & 4 deletions tests/unit/test_hooks.py
Expand Up @@ -16,15 +16,15 @@ def test_handle_response(self, mocker):

class TestRequestAuditor(object):
def test_audit_request(self, mocker):
auditor = mocker.stub()
auditor = mocker.Mock()
ra = hooks.RequestAuditor(auditor)
ra.audit_request("consumer", "request")
auditor.assert_called_with("request")


class TestExceptionHandler(object):
def test_handle_exception_masked(self, mocker):
handler = mocker.stub()
handler = mocker.Mock()
eh = hooks.ExceptionHandler(handler)
eh.handle_exception("consumer", "exc_type", "exc_val", "exc_tb")
handler.assert_called_with("exc_type", "exc_val", "exc_tb")
Expand All @@ -45,8 +45,8 @@ def test_delegate_handle_response(self, transaction_hook_mock):

def test_delegate_handle_response_multiple(self, mocker):
# Include one hook that can't handle responses
mock_response_handler = mocker.stub()
mock_request_auditor = mocker.stub()
mock_response_handler = mocker.Mock()
mock_request_auditor = mocker.Mock()

chain = hooks.TransactionHookChain(
hooks.RequestAuditor(mock_request_auditor),
Expand Down
7 changes: 6 additions & 1 deletion uplink/builder.py
Expand Up @@ -7,6 +7,7 @@
arguments,
auth as auth_,
clients,
compat,
converters as converters_,
exceptions,
helpers,
Expand Down Expand Up @@ -41,7 +42,11 @@ def _get_request_hooks(contract):
return chain

def _wrap_hook(self, func):
return functools.partial(func, self._consumer)
@compat.wraps(func)
def wrapper(*args, **kwargs):
return func(self._consumer, *args, **kwargs)

return wrapper

def apply_hooks(self, execution_builder, chain):
# TODO:
Expand Down
4 changes: 3 additions & 1 deletion uplink/clients/aiohttp_.py
Expand Up @@ -5,6 +5,7 @@
# Standard library imports
import asyncio
import collections
import inspect
import threading
from concurrent import futures

Expand Down Expand Up @@ -86,7 +87,8 @@ async def session(self):
return self._session

def wrap_callback(self, callback):
if not asyncio.iscoroutinefunction(callback):
func = inspect.unwrap(callback)
if not asyncio.iscoroutinefunction(func):
callback = self._sync_callback_adapter(callback)
return callback

Expand Down
1 change: 1 addition & 0 deletions uplink/compat.py
Expand Up @@ -5,3 +5,4 @@

abc = six.moves.collections_abc
reraise = six.reraise
wraps = six.wraps
1 change: 1 addition & 0 deletions uplink/hooks.py
Expand Up @@ -15,6 +15,7 @@ def _wrap_if_necessary(hook, requires_consumer):


def _wrap_to_ignore_consumer(hook):
@compat.wraps(hook)
def wrapper(_, *args, **kwargs):
# Expects that consumer is the first argument
return hook(*args, **kwargs)
Expand Down

0 comments on commit 5a17ccd

Please sign in to comment.