asyncio: #8 consider contextvars #9
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
delfick
commented
Oct 16, 2021
| @wraps(original) | ||
| def run_fixture(*args, **kwargs): | ||
| try: | ||
| ctx.run(lambda: None) |
There was a problem hiding this comment.
there seems no other way of knowing if the context is already active then to use it and see if it makes RuntimError happen
delfick
commented
Oct 16, 2021
|
|
||
|
|
||
| def converted_async_test(test_tasks, func, timeout, *args, **kwargs): | ||
| """Used to replace async tests""" | ||
| __tracebackhide__ = True | ||
|
|
||
| info = {} | ||
| loop = asyncio.get_event_loop() | ||
| loop = asyncio.get_event_loop_policy().get_event_loop() |
There was a problem hiding this comment.
python3.10 change. Apparently asyncio.get_event_loop() becomes equivalent to asyncio.get_running_loop() and these sync functions where I get the loop don't necessarily have one setup already it seems
delfick
commented
Oct 16, 2021
alt_pytest_asyncio/plugin.py
Outdated
|
|
||
| def pytest_configure(self, config): | ||
| """Register our timeout marker which is used to signify async timeouts""" | ||
| config.addinivalue_line( | ||
| "markers", "async_timeout(length): mark async test to have a timeout" | ||
| ) | ||
|
|
||
| @pytest.hookimpl(tryfirst=True, hookwrapper=True) | ||
| def pytest_sessionstart(self, session): |
There was a problem hiding this comment.
it hurts so much that I can't do a run_until_complete with a particular context.
delfick
commented
Oct 16, 2021
setup.py
Outdated
| @@ -14,7 +14,7 @@ | |||
| , version = VERSION | |||
| , packages = find_packages(include="alt_pytest_asyncio.*", exclude=["tests*"]) | |||
|
|
|||
| , python_requires = ">= 3.5" | |||
| , python_requires = ">= 3.7" | |||
There was a problem hiding this comment.
python3.6 is nearly end of life, and the contextvars is new since 3.7
delfick
changed the title
|
originally I tried to make it so that each test got their own context, but it's too difficult to know what should stick around from fixtures of different scopes so I've made one context for everything. |
|
Thanks! Please, give me a couple of days. |
|
cool. No rush :) |
delfick
force-pushed
the
f-asyncio/consider-contextvars
branch
2 times, most recently
from
07432ae to
6bc310a
Compare
|
did you still need this @andredias ? |
|
no, I don't. Thank you very much!
Em qui, 6 de out de 2022 00:28, Stephen Moore ***@***.***>
escreveu:
… did you still need this @andredias <https://github.com/andredias> ?
—
Reply to this email directly, view it on GitHub
<#9 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAG4LDGU6BKVC523KCOG3CDWBZBOVANCNFSM5GDMXHHQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
mmkay, I'm gonna close this then. If you (or anyone else!) wants this, add a comment and I'll make it work again :) |
delfick
force-pushed
the
f-asyncio/consider-contextvars
branch
2 times, most recently
from
0b83dba to
433cd43
Compare
delfick
force-pushed
the
f-asyncio/consider-contextvars
branch
2 times, most recently
from
e0d8c92 to
246518c
Compare
Make it so everything gets executed in the same asyncio context
delfick
force-pushed
the
f-asyncio/consider-contextvars
branch
from
246518c to
c3c6112
Compare
delfick
force-pushed
the
f-asyncio/consider-contextvars
branch
from
c3c6112 to
c585125
Compare
|
It works for the simplest example I gave before. Unfortunately, when it gets a bit more complicated, it fails: from collections.abc import AsyncIterable
from pathlib import Path
from databases import Database
from pytest import fixture
from sqlalchemy import text
@fixture(scope="session")
async def db() -> AsyncIterable[Database]:
db: Database = Database("sqlite:///example.db")
await db.connect()
query = """
create table produto (
id integer primary key,
name text not null,
email text not null,
unique(email)
)
"""
await db.execute(query)
try:
yield db
finally:
await db.disconnect()
Path("exammple.db").unlink()
@fixture
async def trans(db: Database) -> AsyncIterable[Database]:
async with db.transaction(force_rollback=True):
yield db
async def test_db(trans: Database) -> None:
query = """insert into produto (id, name, email) values (1, 'Fulano', '[email protected]')"""
await trans.execute(text(query))The error is: Isn't creating a backport for Runner to run on older versions feasible? It feels like the best solution. |
|
@andredias ok, so the first problem is your test has a typo and says "exammple.db" instead of "example.db" The second problem is that when you let it work out which connection to use, it'll use one per asyncio.Task, which doesn't work because we create new asyncio.Tasks all the time in this plugin. So you want something more like from collections.abc import AsyncIterable
from pathlib import Path
from databases import Database
from databases.core import Connection
from pytest import fixture
from sqlalchemy import text
@fixture(scope="session")
async def db() -> AsyncIterable[Database]:
db: Database = Database("sqlite:///example.db")
await db.connect()
query = """
create table if not exists produto (
id integer primary key,
name text not null,
email text not null,
unique(email)
)
"""
await db.execute(query)
try:
yield db
finally:
await db.disconnect()
Path("example.db").unlink(missing_ok=True)
@fixture
async def nondurable_conn(db: Database) -> AsyncIterable[Connection]:
async with db.connection() as connection:
async with connection.transaction(force_rollback=True):
yield connection
async def test_db(nondurable_conn: Database) -> None:
query = """insert into produto (id, name, email) values (1, 'Fulano', '[email protected]')"""
await nondurable_conn.execute(text(query)) |
|
Hi, Stephen,
Thanks for your response!
You're right: I could pass a connection to other functions that need it as
we usually do with SQLAlchemy. However, one of the features I like most in
encode/databases is not doing that as it relies on contextvars to get the
right one (
https://www.encode.io/databases/connections_and_transactions/#transactions).
That keeps interfaces much simpler.
Regards,
André
Em seg., 27 de mai. de 2024 às 09:18, Stephen Moore <
***@***.***> escreveu:
… @andredias <https://github.com/andredias> ok, so the first problem is
your test has a typo and says "exammple.db" instead of "example.db"
The second problem is that when you let it work out which connection to
use, it'll use one per asyncio.Task, which doesn't work because we create
new asyncio.Tasks all the time in this plugin.
So you want something more like
from collections.abc import AsyncIterablefrom pathlib import Path
from databases import Databasefrom databases.core import Connectionfrom pytest import fixturefrom sqlalchemy import text
@fixture(scope="session")async def db() -> AsyncIterable[Database]:
db: Database = Database("sqlite:///example.db")
await db.connect()
query = """create table if not exists produto ( id integer primary key, name text not null, email text not null, unique(email)) """
await db.execute(query)
try:
yield db
finally:
await db.disconnect()
Path("example.db").unlink(missing_ok=True)
@fixtureasync def nondurable_conn(db: Database) -> AsyncIterable[Connection]:
async with db.connection() as connection:
async with connection.transaction(force_rollback=True):
yield connection
async def test_db(nondurable_conn: Database) -> None:
query = """insert into produto (id, name, email) values (1, 'Fulano', ***@***.***')"""
await nondurable_conn.execute(text(query))
—
Reply to this email directly, view it on GitHub
<#9 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAG4LDHXFAES3FN2GVZLD6DZEMQDFAVCNFSM5GDMXHH2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMJTGMZTMMZRGUYA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Well, the contextvars part works fine. The part that doesn't work is that it caches which connection is available based on the current asyncio.Task. And fundamentally the way this plugin works is by creating new asyncio.Task objects whenever it runs a test or fixture.
unsolicited advice, but certainly my default opinion would be that for anything more than a random script, you would find that convenience very limiting in the future in a way that would be very difficult to reverse. |
|
I have been doing some async programming but am no expert. After reading
PEP 555 Context-local variables (https://peps.python.org/pep-0555/) --
which has a good rationale but was withdrawn--, and PEP 567 – Context
Variables (https://peps.python.org/pep-0567/), I understand that
contextvars is a standard module with legitimate uses in asynchronous
programming to provide context-local variables.
The problem with the plugin is that the fixtures and tests don't share the
same context, right? For example, the database fixture in the example
caches a connection in its context but that context isn't shared with tests.
Asyncio.Runner would be a good solution according to this (
https://github.com/python/cpython/blob/d4680b9e17815140b512a399069400794dae1f97/Lib/asyncio/runners.py#L39
):
This can be useful for interactive console (e.g. IPython),
unittest runners, console tools, -- everywhere when async code
is called from existing sync framework and where the preferred single
asyncio.run() call doesn't work.
The issue is that requires Python 3.11 or greater. Couldn't it be
backported? At least for Python 3.8 as Python 3.6 is not supported anymore (
https://devguide.python.org/versions/)?
Regards,
André
Em qui., 30 de mai. de 2024 às 19:03, Stephen Moore <
***@***.***> escreveu:
… You're right: I could pass a connection to other functions that need it as
we usually do with SQLAlchemy. However, one of the features I like most in
encode/databases is not doing that as it relies on contextvars to get the
right one
Well, the contextvars part works fine. The part that doesn't work is that
it caches which connection is available based on the current asyncio.Task.
And fundamentally the way this plugin works is by creating new asyncio.Task
objects whenever it runs a test or fixture.
That keeps interfaces much simpler.
unsolicited advice, but certainly my default opinion would be that for
anything more than a random script, you would find that convenience very
limiting in the future in a way that would be very difficult to reverse.
—
Reply to this email directly, view it on GitHub
<#9 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAG4LDA37EFOS4AWTCLBXODZE6OZZAVCNFSM5GDMXHH2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMJUGA4TEOJRGYZA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
well, this PR makes the library 3.11+ so I get rid of the problem of needing at least python 3.11 As I said I definitely don't have time to use asyncio.Runners. Also I don't think that will solve your problem. This PR supports contextvars fine, and I'm gonna merge it now and release a new version. The problem is this code https://github.com/encode/databases/blob/0.9.0/databases/core.py#L89. It is able to find the connections it stores, but it's finding them based on the current task. And the way this plugin runs everything as async is by running each function as it's own asyncio.Task (necessary for the timeout stuff and error handling) |
|
I've released version 0.8.0. Please don't be afraid to keep seeking assistance :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Make it so everything gets executed in the same asyncio context
I've never worked with contextvars before and what I did here was very non obvious.
So before I write more tests and docs, can you give this a shot please @andredias ?
Thanks.