search

schireson / pytest-mock-resources

Pytest Fixtures that let you actually test against external resource (Postgres, Mongo, Redshift...) dependent code.
https://pytest-mock-resources.readthedocs.io/en/latest/quickstart.html
MIT License
179 stars 19 forks source link

Async fixtures support #105

Closed michaelbukachi closed 3 years ago

michaelbukachi commented 3 years ago

Is your feature request related to a problem? Please describe. Sqlalchemy 1.4 & 2.0 were released with async support. The current fixtures produce synchronous engines so they can't be used with async code.

Describe the solution you'd like Add support for async sqlalchemy. Maybe by adding a parameter such as async_ to the fixture functions

Describe alternatives you've considered I haven't found any

Additional context Here's a code snippet:

# returns an async engine
engine = create_postgres_fixture(Base, scope="session", async_=True)
# or
engine = create_async_postgres_fixture(Base, scope="session")

We might have to use pytest-asyncio

I'm already working on something. I'm willing to do a PR for this.

DanCardin commented 3 years ago

Because the function returns the fixture, i suppose i'd prefer the async_ parameter to a secondary function. @oakhan3?

I suspect it's going to be vastly easier to produce an async engine after things like model creation/data insertion, lest you have to duplicate all that logic unnecessarily. Given that, I maybe don't expect that we will need an event loop, handy, so i think we shouldn't need pytest-asyncio.

We should probably have a new extra (postgres-async, say) to include the driver.

A "problem" that occurs to me is that there are more (relative to psycopg2 for sync) realistically usable drivers for async (asyncpg, aiopg). The extra can pick one (i'd guess asyncpg), but today we have no mechanism to pick which one you want your engine created with. That's probably something we can worry about if anyone ever cares though :D.

Speaking of which (@oakhan3). Today we technically support python2, adding this code (unless we @coroutine decorate instead of async def) will break <3.5. Personally I'm happy for us to drop support and document 1.0 as the version to use if you use python2.

oakhan3 commented 3 years ago

An async_ parameter works if all original inputs are supported which would be ideal and preferred.

SQLAlchemy does offer asyncio support although it seems to be beta-level atm and only supports python 3.6+. This would likely be the most straight forward way to implement support - albeit more opinionated that letting the user pick via package extras. I'm open to all suggestions!

With that said, I agree, we should drop python 2 support so that we aren't forced to use older syntax and miss out on new performance and options.

michaelbukachi commented 3 years ago

@oakhan3 @DanCardin so two different releases? One for dropping python 2 first then followed by another for adding async support?

DanCardin commented 3 years ago

~I dont necessarily think we even really need a release for dropping python 2 explicitly~ Jk, i think i actually will version bump. I will update the docs, version constraint, and CI in a PR of my own.

You should just be able to deal with the async support in your PR for now, and ignore <3.5 compat freely

EDIT: Yea, so just released 2.0 as a release which just removes "support" for <3.6, so you should be good to go

michaelbukachi commented 3 years ago

~I dont necessarily think we even really need a release for dropping python 2 explicitly~ Jk, i think i actually will version bump. I will update the docs, version constraint, and CI in a PR of my own.

You should just be able to deal with the async support in your PR for now, and ignore <3.5 compat freely

EDIT: Yea, so just released 2.0 as a release which just removes "support" for <3.6, so you should be good to go

Awesome! Thanks, let me get to it.

DanCardin commented 3 years ago

I'll submit a follow up PR tomorrow morning to get this released

DanCardin commented 3 years ago

Actually, i think the PR might have inadvertantly broken sqlalchemy 1.3, so i'm going to try to test that today and fix if so before i release

michaelbukachi commented 3 years ago

I see what you meaning. Some of the tests I added were specific to > 1.3. Maybe we can a CI job to test 1.3 specifically?

DanCardin commented 3 years ago

I'm actually not concerned about tests being 1.3 specific, necessarily, because we can choose our own version in the test suite. although we may want to add a 1.3-specific CI job to have detected this.

I'm more concerned about 1.3 runtime compatibility, which i think https://github.com/schireson/pytest-mock-resources/pull/110 should address (hopefully). Tests passed locally with 1.3

DanCardin commented 3 years ago

Should be released as version 2.1.0

michaelbukachi commented 3 years ago

Hey @DanCardin, has the release been made yet?

DanCardin commented 3 years ago

Yup, https://pypi.org/project/pytest-mock-resources/2.1.0/