Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Smallest possible change to bump to Pydantic 2.x #15

Open
wants to merge 5 commits into
base: simple
Choose a base branch
from

Conversation

charles-dyfis-net
Copy link

Addressing #14.

This leaves in place usage of several Pydantic 1.x-isms, which will need to be excised before Pydantic 3.x can be supported.

This leaves in place usage of several Pydantic 1.x-isms, which will need to be excised before Pydantic 3.x can be supported.
# "pytest-asyncio~=0.20.3",
"pytest-asyncio @ https://github.com/joeblackwaslike/pytest-asyncio/releases/download/v0.20.4.dev42/pytest_asyncio-0.20.4.dev42-py3-none-any.whl",
# "pytest-asyncio @ https://github.com/joeblackwaslike/pytest-asyncio/releases/download/v0.20.4.dev42/pytest_asyncio-0.20.4.dev42-py3-none-any.whl",
Copy link
Owner

Choose a reason for hiding this comment

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

this forked dependency is essential. pytest-asyncio has a longstanding bug they refuse to fix with context propagation which breaks async fixtures in pytest. The problem was that async fixtures that yield were having their finalizers run in a separate asyncio task that did not preserve contextvars.

I was able to fork this pytest plugin and fix that, and for the first time ever I saw async generator fixtures work under pytest. It felt magical. The upstream repo is not interested in solving the problem in this way, they would rather not solve it.

The last time I checked, my fork was the only version of pytest-asyncio that addresses this bug, and without it the tests fail due to an async fixture not propagating its context. Feel free to review my changes in the following repo: https://github.com/joeblackwaslike/pytest-asyncio

@@ -73,7 +74,6 @@ filterwarnings = [
"ignore:.*Field .* has conflict with protected namespace \"model_\".*:UserWarning",
]
asyncio_mode = "auto"
py311_task = true
Copy link
Owner

Choose a reason for hiding this comment

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

this is a toggle i created to activate the patch that fixes everything, so that behavior has to be explicitly opted into. you need to include it.

@@ -61,7 +62,7 @@ excludes = [
]

[tool.pytest.ini_options]
addopts = "-rsx --tb=short --loop-scope session"
addopts = "-rsx --tb=short"
Copy link
Owner

Choose a reason for hiding this comment

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

loop-scope is a new option i've added which parameterizes the scope of the event-loop fixture. Sharing the same event loop for the entire pytest session allows async fixtures to work at higher scopes.

Comment on lines -9 to -15
pytest -v -rsx --tb=short --asyncio-mode=auto --py311-task true --loop-scope session {posargs} tests
pytest -v -rsx --tb=short --asyncio-mode=auto {posargs} tests

[testenv:q183]
commands =
pdm install --dev
pip install aiofiles==23.2.1 blinker==1.5 click==8.1.7 flask==2.2.1 quart==0.18.3 werkzeug==2.2.0 jinja2==3.1.2
pytest -v -rsx --tb=short --asyncio-mode=auto --py311-task true --loop-scope session {posargs} tests
Copy link
Owner

Choose a reason for hiding this comment

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

these options need to remain for things to keep working when run from tox

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants