From 030e97290d61b13953c5329270d81d18128affbe Mon Sep 17 00:00:00 2001 From: Tom Wojcik Date: Sat, 26 Nov 2022 23:28:21 +0100 Subject: [PATCH 1/5] add-concurrency-test --- .pre-commit-config.yaml | 12 +-- Makefile | 3 + requirements-dev.in | 3 +- requirements-dev.txt | 159 +++++++++++++++++++------------------- tests/test_concurrency.py | 88 +++++++++++++++++++++ 5 files changed, 179 insertions(+), 86 deletions(-) create mode 100644 tests/test_concurrency.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b461556..eebddea 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -3,7 +3,7 @@ fail_fast: false repos: - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.2.0 + rev: v4.4.0 hooks: - id: debug-statements - id: check-added-large-files @@ -13,7 +13,7 @@ repos: - id: trailing-whitespace - repo: https://github.com/ambv/black - rev: 22.3.0 + rev: 22.10.0 hooks: - id: black language_version: python3 @@ -22,7 +22,7 @@ repos: types: [ python ] - repo: https://github.com/myint/docformatter - rev: v1.4 + rev: v1.5.0 hooks: - id: docformatter args: ['--in-place', '--wrap-summaries=78', '--wrap-descriptions=78', '--pre-summary-newline'] @@ -35,17 +35,17 @@ repos: exclude: '__init__.py' - repo: https://github.com/pre-commit/mirrors-mypy - rev: v0.960 + rev: v0.991 hooks: - id: mypy files: ^starlette_context/ - repo: https://github.com/pycqa/flake8 - rev: 4.0.1 + rev: 6.0.0 hooks: - id: flake8 - repo: https://github.com/asottile/pyupgrade - rev: v2.32.1 + rev: v3.2.2 hooks: - id: pyupgrade diff --git a/Makefile b/Makefile index f66078f..9b7d1ed 100644 --- a/Makefile +++ b/Makefile @@ -32,3 +32,6 @@ minor: upgrade-deps: pre-commit autoupdate pip-compile --upgrade requirements-dev.in + +deps: + pip-compile requirements-dev.in diff --git a/requirements-dev.in b/requirements-dev.in index 2c2606e..806c3b7 100644 --- a/requirements-dev.in +++ b/requirements-dev.in @@ -1,5 +1,6 @@ starlette -requests +asgi-lifespan +httpx pytest pytest-cov pytest-sugar diff --git a/requirements-dev.txt b/requirements-dev.txt index 6d7d471..a974a3c 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,136 +1,144 @@ # -# This file is autogenerated by pip-compile +# This file is autogenerated by pip-compile with python 3.9 # To update, run: # # pip-compile requirements-dev.in # alabaster==0.7.12 # via sphinx -anyio==3.6.1 - # via starlette -attrs==21.4.0 +anyio==3.6.2 + # via + # httpcore + # starlette +asgi-lifespan==2.0.0 + # via -r requirements-dev.in +attrs==22.1.0 # via pytest -babel==2.10.1 +babel==2.11.0 # via sphinx +build==0.9.0 + # via pip-tools bump2version==1.0.1 # via -r requirements-dev.in -certifi==2022.5.18.1 - # via requests +certifi==2022.9.24 + # via + # httpcore + # httpx + # requests cfgv==3.3.1 # via pre-commit -charset-normalizer==2.0.12 +charset-normalizer==2.1.1 # via requests click==8.1.3 # via pip-tools codecov==2.1.12 # via -r requirements-dev.in -coverage[toml]==6.4.1 +coverage[toml]==6.5.0 # via # codecov # pytest-cov -distlib==0.3.4 +distlib==0.3.6 # via virtualenv docutils==0.17.1 # via # sphinx # sphinx-rtd-theme +exceptiongroup==1.0.4 + # via pytest execnet==1.9.0 # via pytest-xdist -filelock==3.7.1 +filelock==3.8.0 # via virtualenv -identify==2.5.1 +h11==0.14.0 + # via httpcore +httpcore==0.16.2 + # via httpx +httpx==0.23.1 + # via -r requirements-dev.in +identify==2.5.9 # via pre-commit -idna==3.3 +idna==3.4 # via # anyio # requests -imagesize==1.3.0 + # rfc3986 +imagesize==1.4.1 + # via sphinx +importlib-metadata==5.1.0 # via sphinx -importlib-metadata==4.11.4 - # via - # click - # pep517 - # pluggy - # pre-commit - # pytest - # sphinx - # virtualenv iniconfig==1.1.1 # via pytest jinja2==3.1.2 # via sphinx markupsafe==2.1.1 # via jinja2 -nodeenv==1.6.0 +nodeenv==1.7.0 # via pre-commit packaging==21.3 # via + # build # pytest # pytest-sugar # sphinx -pep517==0.12.0 - # via pip-tools -pip-tools==6.6.2 +pep517==0.13.0 + # via build +pip-tools==6.10.0 # via -r requirements-dev.in -platformdirs==2.5.2 +platformdirs==2.5.4 # via virtualenv pluggy==1.0.0 # via pytest -pre-commit-hooks==4.2.0 +pre-commit==2.20.0 # via -r requirements-dev.in -pre-commit==2.19.0 +pre-commit-hooks==4.4.0 # via -r requirements-dev.in -py==1.11.0 - # via - # pytest - # pytest-forked -pygments==2.12.0 +pygments==2.13.0 # via sphinx pyparsing==3.0.9 # via packaging -pytest-asyncio==0.18.3 - # via -r requirements-dev.in -pytest-cov==3.0.0 - # via -r requirements-dev.in -pytest-forked==1.4.0 - # via pytest-xdist -pytest-sugar==0.9.4 - # via -r requirements-dev.in -pytest-xdist==2.5.0 - # via -r requirements-dev.in -pytest==7.1.2 +pytest==7.2.0 # via # -r requirements-dev.in # pytest-asyncio # pytest-cov - # pytest-forked # pytest-sugar # pytest-xdist -pytz==2022.1 +pytest-asyncio==0.20.2 + # via -r requirements-dev.in +pytest-cov==4.0.0 + # via -r requirements-dev.in +pytest-sugar==0.9.6 + # via -r requirements-dev.in +pytest-xdist==3.0.2 + # via -r requirements-dev.in +pytz==2022.6 # via babel pyyaml==6.0 # via pre-commit -requests==2.27.1 +requests==2.28.1 # via - # -r requirements-dev.in # codecov # sphinx -ruamel.yaml.clib==0.2.6 - # via ruamel.yaml -ruamel.yaml==0.17.21 +rfc3986[idna2008]==1.5.0 + # via httpx +ruamel-yaml==0.17.21 # via pre-commit-hooks -six==1.16.0 - # via virtualenv -sniffio==1.2.0 - # via anyio +ruamel-yaml-clib==0.2.7 + # via ruamel-yaml +sniffio==1.3.0 + # via + # anyio + # asgi-lifespan + # httpcore + # httpx snowballstemmer==2.2.0 # via sphinx -sphinx-rtd-theme==1.0.0 - # via -r requirements-dev.in -sphinx==5.0.1 +sphinx==5.3.0 # via # -r requirements-dev.in # sphinx-rtd-theme +sphinx-rtd-theme==1.1.1 + # via -r requirements-dev.in sphinxcontrib-applehelp==1.0.2 # via sphinx sphinxcontrib-devhelp==1.0.2 @@ -143,35 +151,28 @@ sphinxcontrib-qthelp==1.0.3 # via sphinx sphinxcontrib-serializinghtml==1.1.5 # via sphinx -starlette==0.20.1 +starlette==0.22.0 # via -r requirements-dev.in -termcolor==1.1.0 +termcolor==2.1.1 # via pytest-sugar toml==0.10.2 - # via - # pre-commit - # pre-commit-hooks + # via pre-commit tomli==2.0.1 # via + # build # coverage - # pep517 + # pre-commit-hooks # pytest -typing-extensions==4.2.0 - # via - # anyio - # importlib-metadata - # pytest-asyncio - # starlette -urllib3==1.26.9 +typing-extensions==4.4.0 + # via starlette +urllib3==1.26.13 # via requests -virtualenv==20.14.1 +virtualenv==20.16.7 # via pre-commit -wheel==0.37.1 +wheel==0.38.4 # via pip-tools -zipp==3.8.0 - # via - # importlib-metadata - # pep517 +zipp==3.11.0 + # via importlib-metadata # The following packages are considered to be unsafe in a requirements file: # pip diff --git a/tests/test_concurrency.py b/tests/test_concurrency.py new file mode 100644 index 0000000..bf76ff9 --- /dev/null +++ b/tests/test_concurrency.py @@ -0,0 +1,88 @@ +import uuid + +import httpx + +from starlette.applications import Starlette +from starlette.middleware import Middleware +from starlette.requests import Request +from starlette.responses import JSONResponse + +from starlette_context import context, plugins +from starlette_context.header_keys import HeaderKeys +from starlette_context.middleware import RawContextMiddleware +import pytest +import pytest_asyncio +from starlette.routing import Route +from asgi_lifespan import LifespanManager +from starlette.exceptions import HTTPException + + +def should_raise(number: int) -> bool: + return number % 2 == 0 + + +@pytest_asyncio.fixture +async def app(): + class CloudProviderException(HTTPException): + pass + + async def cloud_provider_exception_handler( + request: Request, exc: HTTPException + ): + return JSONResponse( + {"detail": exc.detail}, + status_code=exc.status_code, + headers={HeaderKeys.request_id: context[HeaderKeys.request_id]}, + ) + + async def index(request: Request) -> JSONResponse: + number = request.path_params["number"] + if should_raise(number): + raise CloudProviderException + return JSONResponse( + content={ + "trace_id": context[HeaderKeys.request_id], + "from": "view", + } + ) + + middleware = [ + Middleware( + RawContextMiddleware, + plugins=(plugins.RequestIdPlugin(),), + ) + ] + exception_handlers = { + CloudProviderException: cloud_provider_exception_handler + } + app = Starlette( + middleware=middleware, + routes=[Route("/{number:int}", index)], + exception_handlers=exception_handlers, + ) + + async with LifespanManager(app): + yield app + + +@pytest.mark.asyncio +async def test_concurrency_correct_headers(app): + transport = httpx.ASGITransport(app=app, raise_app_exceptions=False) + async with httpx.AsyncClient( + app=app, transport=transport, base_url="http://test" + ) as client: + for number in range(1, 101): + rid = uuid.uuid4().hex + resp = await client.get( + f"/{number}", headers={HeaderKeys.request_id: rid} + ) + + if should_raise(number): + assert resp.status_code == 500 + assert resp.headers[HeaderKeys.request_id] == rid + else: + assert resp.status_code == 200 + d = resp.json() + assert d["from"] == "view" + assert d["trace_id"] == rid + assert resp.headers[HeaderKeys.request_id] == rid From 88086c6633081de040d4240fb2b9fb4b7667603a Mon Sep 17 00:00:00 2001 From: Tom Wojcik Date: Sat, 26 Nov 2022 23:41:01 +0100 Subject: [PATCH 2/5] reorder exc handler with exc --- tests/test_concurrency.py | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/tests/test_concurrency.py b/tests/test_concurrency.py index bf76ff9..a1d459d 100644 --- a/tests/test_concurrency.py +++ b/tests/test_concurrency.py @@ -1,3 +1,4 @@ +import random import uuid import httpx @@ -23,20 +24,22 @@ def should_raise(number: int) -> bool: @pytest_asyncio.fixture async def app(): - class CloudProviderException(HTTPException): - pass - async def cloud_provider_exception_handler( request: Request, exc: HTTPException ): return JSONResponse( - {"detail": exc.detail}, - status_code=exc.status_code, - headers={HeaderKeys.request_id: context[HeaderKeys.request_id]}, + {"detail": "asd"}, + status_code=400, ) + class CloudProviderException(Exception): + pass + async def index(request: Request) -> JSONResponse: number = request.path_params["number"] + r1 = random.randint(1, 10000) + r2 = random.randint(1, 10000) + _ = r1**r2 if should_raise(number): raise CloudProviderException return JSONResponse( @@ -71,18 +74,14 @@ async def test_concurrency_correct_headers(app): async with httpx.AsyncClient( app=app, transport=transport, base_url="http://test" ) as client: - for number in range(1, 101): + for number in range(1, 501): rid = uuid.uuid4().hex resp = await client.get( f"/{number}", headers={HeaderKeys.request_id: rid} ) - if should_raise(number): - assert resp.status_code == 500 + assert resp.status_code == 400 assert resp.headers[HeaderKeys.request_id] == rid else: assert resp.status_code == 200 - d = resp.json() - assert d["from"] == "view" - assert d["trace_id"] == rid assert resp.headers[HeaderKeys.request_id] == rid From 9226c880180a44aae2d2f007d814fb3f9c381fb3 Mon Sep 17 00:00:00 2001 From: Tom Wojcik Date: Sat, 26 Nov 2022 23:47:14 +0100 Subject: [PATCH 3/5] fix dummy sleep --- tests/test_concurrency.py | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/tests/test_concurrency.py b/tests/test_concurrency.py index a1d459d..e476fce 100644 --- a/tests/test_concurrency.py +++ b/tests/test_concurrency.py @@ -17,11 +17,20 @@ from asgi_lifespan import LifespanManager from starlette.exceptions import HTTPException +import asyncio + def should_raise(number: int) -> bool: return number % 2 == 0 +async def dummy_sleep(): + for i in range(random.randint(0, 100000)): + if i % 2 and i % 3: + pass + return 0 + + @pytest_asyncio.fixture async def app(): async def cloud_provider_exception_handler( @@ -37,9 +46,13 @@ class CloudProviderException(Exception): async def index(request: Request) -> JSONResponse: number = request.path_params["number"] - r1 = random.randint(1, 10000) - r2 = random.randint(1, 10000) - _ = r1**r2 + + # for some reason it's not possible to + # await asyncio.sleep(1) + loop = asyncio.get_running_loop() + f1 = loop.create_task(dummy_sleep()) + await asyncio.wait([f1]) + if should_raise(number): raise CloudProviderException return JSONResponse( @@ -79,6 +92,7 @@ async def test_concurrency_correct_headers(app): resp = await client.get( f"/{number}", headers={HeaderKeys.request_id: rid} ) + # print(number) if should_raise(number): assert resp.status_code == 400 assert resp.headers[HeaderKeys.request_id] == rid From f30f152f84d7b9735246319e67dd75ab6d2c586c Mon Sep 17 00:00:00 2001 From: Tom Wojcik Date: Sat, 26 Nov 2022 23:49:40 +0100 Subject: [PATCH 4/5] add a working assertion error so the test actually fails --- tests/test_concurrency.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/test_concurrency.py b/tests/test_concurrency.py index e476fce..38e7a35 100644 --- a/tests/test_concurrency.py +++ b/tests/test_concurrency.py @@ -26,7 +26,7 @@ def should_raise(number: int) -> bool: async def dummy_sleep(): for i in range(random.randint(0, 100000)): - if i % 2 and i % 3: + if i % 2 == 0 and i % 3 == 0: pass return 0 @@ -84,6 +84,7 @@ async def index(request: Request) -> JSONResponse: @pytest.mark.asyncio async def test_concurrency_correct_headers(app): transport = httpx.ASGITransport(app=app, raise_app_exceptions=False) + unordered_numbers = [] async with httpx.AsyncClient( app=app, transport=transport, base_url="http://test" ) as client: @@ -92,10 +93,12 @@ async def test_concurrency_correct_headers(app): resp = await client.get( f"/{number}", headers={HeaderKeys.request_id: rid} ) - # print(number) + unordered_numbers.append(number) if should_raise(number): assert resp.status_code == 400 assert resp.headers[HeaderKeys.request_id] == rid else: assert resp.status_code == 200 assert resp.headers[HeaderKeys.request_id] == rid + + assert unordered_numbers != sorted(unordered_numbers) From 0d7eaad91cb12cfa746c395e009b8c12a0dfdba9 Mon Sep 17 00:00:00 2001 From: Tom Wojcik Date: Sun, 27 Nov 2022 00:33:18 +0100 Subject: [PATCH 5/5] fix dummy_sleep (still broken) --- tests/test_concurrency.py | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/tests/test_concurrency.py b/tests/test_concurrency.py index 38e7a35..e36cf36 100644 --- a/tests/test_concurrency.py +++ b/tests/test_concurrency.py @@ -24,11 +24,16 @@ def should_raise(number: int) -> bool: return number % 2 == 0 -async def dummy_sleep(): - for i in range(random.randint(0, 100000)): - if i % 2 == 0 and i % 3 == 0: - pass - return 0 +async def sleep_alternative(): + """ + Doesn't seem to work with asyncio.sleep. + + see https://github.com/spulec/freezegun/issues/437 + + Even with this hack, the order of numbers is still ordered. + """ + loop = asyncio.get_running_loop() + await asyncio.sleep(random.randint(0, 3), loop=loop) @pytest_asyncio.fixture @@ -46,13 +51,7 @@ class CloudProviderException(Exception): async def index(request: Request) -> JSONResponse: number = request.path_params["number"] - - # for some reason it's not possible to - # await asyncio.sleep(1) - loop = asyncio.get_running_loop() - f1 = loop.create_task(dummy_sleep()) - await asyncio.wait([f1]) - + await sleep_alternative() if should_raise(number): raise CloudProviderException return JSONResponse( @@ -88,7 +87,7 @@ async def test_concurrency_correct_headers(app): async with httpx.AsyncClient( app=app, transport=transport, base_url="http://test" ) as client: - for number in range(1, 501): + for number in range(1, 10): rid = uuid.uuid4().hex resp = await client.get( f"/{number}", headers={HeaderKeys.request_id: rid}