search

graphql-python / aiohttp-graphql

Adds GraphQL support to your aiohttp app.
Other
119 stars 24 forks source link

graphql-server-core breaking change #5

Closed nikordaris closed 6 years ago

nikordaris commented 6 years ago

The deps in setup.py pull in graphl-server-core>=1.0.dev which pulls in the 1.1rc0 but keeps graphql-core at 2.0. graphql-server-core 1.1rc0 and graphql-core 2.0 are incompatible. This breaks aiohttp-graphql.

It is probably not a good idea to define such a wide range for dependencies on core graphql libs so this won't happen again.

Workaround for now is to downgrade graphql-server-core in my project to graphql-server-core==1.0.dev20170322001

dfee commented 6 years ago

Yeah, building a compatibility matrix is the tricky thing with "component" packages – it's exponentially complex.

It also turns out that the tests aren't currently runnable due to API changes in aiohttp. Assuming I find time to update those tests, @syrusakbary what are your thoughts on versioning across the graphql-python landscape?

japrogramer commented 6 years ago

any updates, to this package? currently trying a pet project with aiohttp and want to have access to graphiql and this seems to be the only package that offers it.

dfee commented 6 years ago

It seems to work fine? What problem are you having? New issue?

japrogramer commented 6 years ago

@dfee I had not tried it yet, since there had been little activity in the project and this issue was still open I thought it was still an issue so I thought i ask if it was before trying it myself.

nikordaris commented 6 years ago

Ultimately, this bug is not fixed since the setup.py still points to 1.0.dev for graphql-server-core. If it works now that is just because the latest 1.0 release candidate is now compatible with graphql-core 2.0. The fix for this is to fix setup.py to have graphql-server-core version be >= to a release that is compatible with graphql-core 2.0

dfee commented 6 years ago

@nikordaris the problem seems to not be with this library (which I wrote), but the python-graphql ecosystem. And unfortunately, it's complicated (as hopefully I can explain here).

When I uploaded this package initially, there were no problems with graphql-server-core compatibility. And in fact, there are more recent configurations with graphl-server-core that work. There are also configurations that don't work. I'm not privy to the releases of graphql-server-core or really anything else in this group outside of aiohttp-graphql.

Anyway, we have something like "dependency sets". That is it's known to work with certain versions, and known to not work with other versions. But at the front-end and back-end of these dependency sets (for an appropriate version of graphql-server-core and graphql-core that are high or low, everything seems to work well (it's just in the middle that things get hairy).

So while I think we have the problem of chasing compatibility with other packages (in the same "team"), I don't know that it's anymore right for me to exclude graphql-server-core==1.0.dev20170322001 when the problem is graphql-server-core==1.1rc0. I guess I could update setup.py to have the following:

 'graphql-server-core>=1.0.dev,!=1.1rc0',

I guess I'm not inspired by that either, as those libraries can continue to maintain incompatibilities as they evolve. Thoughts?

japrogramer commented 6 years ago

well, I don't know much about graphql-server-code and know even less about its development cycle so I can't really comment abut the setup.py change yet

but by looking at the code at a glance it appears to be a compatibility layer to standardize the integration between all the web frameworks

if so I spotted a bit of code that appears to be a bit of duplication

https://github.com/graphql-python/graphene-django/blob/master/graphene_django/views.py#L199 is a duplicate of, https://github.com/graphql-python/graphql-server-core/blob/master/graphql_server/__init__.py#L98

not a big issue tho

nikordaris commented 6 years ago

@dfee I think I understand the issue now. Ultimately, this is the problem with including .dev and rc versions. They are usually unstable. Given graphql-server-core is on 1.1.1 now it seems like this lib should depend on a stable version.

Also, in my opinion if a library depends on a rc or .dev release, then it should also mark their version as such to convey to the users that it is unstable.

To be clear I think a we should have a new release where it points to stable versions, leaving the current release as an option for those still using the rc candidate deps. This would ensure those who upgrade get a stable version.