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

async support doesn't require patching #1392

Merged
merged 1 commit into from
Apr 12, 2021
Merged

async support doesn't require patching #1392

merged 1 commit into from
Apr 12, 2021

Conversation

davidism
Copy link
Member

@davidism davidism commented Apr 10, 2021

The patching is not required, async def is valid in all supported Python versions. Moves the async/patched sync functions from asyncsupport into the regular code. Moves the async filters from asyncfilters to filters. This does mess with the signature arguments in the docs for the filters, I think that can be fixed in Pallets-Sphinx-Theme's Jinja support.

import asyncio is done inside render and generate to avoid importing it at the top, which would slow down import time.

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

@davidism davidism added the async label Apr 10, 2021
@davidism davidism added this to the 3.0.0 milestone Apr 10, 2021
@davidism
Copy link
Member Author

@pgjones related to #1191:

asyncio is imported in two methods on Environment, to call asyncio.get_event_loop() and loop.run_until_complete(coroutine). We can probably make those two separate, overridable methods on Environment in order to allow using Trio, etc.

@pgjones
Copy link
Member

pgjones commented Apr 11, 2021

As I understand it the imports are in render and generate the sync versions of render_async and generate_async. In which case I don't think this is an issue (as trio.run(environment.render_async, ...) is valid).

I wonder though if these sync methods should instead raise runtime/not implemented errors in an async environment. I think this would happen naturally anyway as calling run_until_complete from within an event loop already raises a runtime error.

@davidism davidism merged commit 9d4689b into master Apr 12, 2021
@davidism davidism deleted the inline-async branch April 12, 2021 01:55
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remove async patching system
2 participants