Support for 3.0?

[davidroeca]

There's a new graphene alpha release targeting graphql-core-next. Just curious: are plans yet to support this, or will it be delayed until the graphene interface reaches a beta/rc/stable version?

[jnak]

There is currently no plan to support it before a version of graphene 3.0 is released.

[DoctorJohn]

This just became a chicken-egg problem as the following was stated in graphql-python/graphene#1127 (comment)

However, we are pending to updated the GraphQL-Python Graphene dependencies (such as graphene-mongo, graphene-sqlalchemy and so) to support Graphene 3.0 and GraphQL-core 3.0. So, until we don't update those dependencies to support Graphene v3, we can't publish the new version.

[jnak]

Thanks for raising this. I don't see any reason for wait for dependent projects to migrate. I commented on the issue you linked to.

[DoctorJohn]

If we were to update this package to support graphene v3, how would we handle the fact that graphene dropped support for dataloaders from the promise library? As far as I understand letting resolvers return promises wont work anymore, thus the following lines would be problematic:

graphene-sqlalchemy/graphene_sqlalchemy/batching.py

Lines 69 to 70 in 631513f

	def resolve(root, info, **args):
	return loader.load(root)

(Disclaimer: I never used the promise dataloaders, this is just what I learned from updating graphene-mongo earlier today).

[DoctorJohn]

Just to let you folks know, I started working on an PR. Got it down to 8 failing tests, 5 of which are related to promises/dataloaders.

[nikolaik]

Just to let you folks know, I started working on an PR. Got it down to 8 failing tests, 5 of which are related to promises/dataloaders.

@DoctorJohn Did you get any further on your PR?

[skewty]

Upstream issue 1127 (linked above) to release version 3.0 will be 1 year old later this month. This issue (248) is now the only remaining project to make it to 3.0 and is blocking upstream AFAIK.

@DoctorJohn which branch / Pull / Merge Request are you working out of? If it hasn't been formalized with a PR/MR please do so others can assist. If the remaining issues can be documented in the MR I am confident we the community can come together and figure them out.

[zsiciarz]

We at Emplocity are willing to collaborate on compatibility with graphene 3. We haven't contributed to the graphene ecosystem before, but we're heavy users of the 2.x versions of libraries and are quite familiar with them. I took a look at @DoctorJohn's PR and got the tests to run (not pass, that would be too much to ask ;) ). There appears to be a few points that need a decision:

• Python 2 support. I think we can safely drop it, following upstream graphene.
• Batching. That's the part I'm not very familiar with (we're using eventlet, which unfortunately causes a ton of issues with promises used by graphene). The tests related to batching don't error out on missing APIs, they just return wrong results. @DoctorJohn pointed out graphene dropped (?) promises in favor of asyncio and yet the DataLoader documented here uses promises. However, there's a new asyncio-based approach, see: Update Dataloader docs [Waiting for Graphene v3 to be live] graphene#1190. I guess we should follow that approach.
• Dependency versions. I needed to pin SQLAlchemy to < 1.4 to get the tests to run. I'll have a look at what changed there and how graphene-sqlalchemy is using the removed/changed APIs.

Our WIP branch based on DoctorJohn's work is here, for anyone interested: https://github.com/Emplocity/graphene-sqlalchemy/commits/graphene-v3

[zsiciarz]

The benchmarks fail due to BatchSQLAlchemyConnectionField. If I remove or comment out

connection_field_factory = BatchSQLAlchemyConnectionField.from_relationship

all the benchmarks pass. This again relies on batching, which uses promises in 2.x and should be updated for aiodataloader. I'm not sure if that's within the scope of graphene-sqlalchemy. Though avoiding the N+1 problem would be desirable.
Using aiodataloader would possibly require SQLAlchemy 1.4 which introduced asyncio support - considered as beta quality right now. Perhaps we should feature-gate batching in graphene-sqlalchemy to be available only with SQLAlchemy >= 1.4?
Aside from these benchmarks, I haven't spotted any serious usage of BatchSQLAlchemyConnectionField in the wild. What if we drop that class (documented as experimental and prone to change) and the entire batching.py module?

[zsiciarz]

We've managed to resolve all test failures except those related to batching and dataloader. The draft PR is here: #306.