Allow adding GraphQL types to Graphene schema #1224
Conversation
|
This PR has to be a bit better analyzed (both technically and non-technically). On the tech side, it might be not as trivial to do. On the non-tech side is not as clear on what are the use cases (we talked about Strawberry integration, but I feel very strongly about adding type support inside Graphene). |
Can you expand on that and give examples? I don't understand what you mean.
The use case in my head is for incremental migration to either another library or to Graphene from another library. For example say you've written your server using just graphql-core but you want to start using Graphene because you prefer the API, you could replace the root Query and Schema with Graphene but still use the graphql-core types you already have without having to rewrite them all. Regarding Strawberry: even if we added Python type support inside Graphene, Strawberry might have other features that people prefer and again this PR would allow users to incrementally adopt that library rather than having to completely rewrite it. |
What happens when you provide a raw interface type to a graphene ObjectType. Should the fields be reflected on the graphene ObjectType? If not, then how should we do it? That's what I mean saying that the tech side might be not as trivial to do.
That's precisely what I'm concerned with. If people are migrating from Graphene to other libraries it will mean that Graphene itself has not evolved as their users expected. This PR provides a patch that eventually allow such scenarios to happen. |
Thanks that's very helpful, I was missing those use cases. The Graphene Interface type already checks that the interfaces are Graphene types which I think is a good thing and we should keep because of the technical challenge that you've mentioned. For Unions: it only needed a small change to support unions of a mixture of Graphene and GraphQL types. I've added tests for both cases to assert their behaviour.
I strongly disagree with this reasoning. Yes in an ideal world Graphene would be all things to all people but that is not likely or desirable in my opinion. This change allows for interop with other libraries (both current and future) built on top of graphql-core that might provide radically different APIs or functionality than Graphene shouldn't attempt to incorporate. This change allows people to experiment without having to wait on Graphene to improve which makes the whole ecosystem better. IMO Graphene is an opinionated way of build GraphQL servers and we should allow developers to incorporate alternative opinions without forcing them to rewrite everything. |
jkimbo
force-pushed
the
allow-graphql-types
branch
from
acfc3f4 to
2fbe0a6
Compare
Why not likely? I agree that some things like SDL-first schemas are not a priority for Graphene. However annotation-based schemas is an objective of Graphene. I can't see any use case for a mixture of different styles mixed together (GraphQL-core plain types + Graphene + SDL based schemas, for example) other than transitioning from one to other, which I don't believe we should strive for. |
|
I've personally mixed SDL-first and code-first types when building GraphQL servers in other languages. I'm not big on SDL-first but do appreciate its ergonomics, but you often hit roadblocks when it comes to metaprogramming and you have to do awkward string-building functions. But there's a larger philosophical point here, and I'm strongly in agreement with @jkimbo here. Every project exists because it set out to solve problems in a way that aligns with the author's (or authors') principles of how things should be done. Two projects could be interchangeable on a raw capability level, but still be vastly different in how they choose to tackle the problems. The idea that someone choosing to migrate away is a damning criticism of your project, is an expression of a flawed winner-takes-all worldview. Your project might do everything someone needs, but they might just enjoy working with something else more. Rails and Django are largely interchangeable, but some people like Python more and some people like Ruby more. As a suggestion to @syrusakbary, make it clear what your values and objectives with Graphene are (though i'd prefer if you avoided grand ones like being the only library in town), and then it'll be clearer for contributors at every level where they stand on each issue. |
I 100% agree with this. However, I don't see people not choosing Graphene as a criticism for the project. I see people not choosing Graphene because it's missing features that I think Graphene should have, as a failure of the project. One of this clear scenarios is typing/annotations. I believe typing annotations are going to be critical for Python in the future while making of an easier API to read and use. As such I think Graphene should have this bundled into the library (I even wrote Graphene 2 with typing in mind :) ). As commented in this PR, If we want to add GraphQL interexchange support as a way for people to use other library that have typing support I see as a failure of Graphene as a library itself (since it's one of its library goals).
But I agree with your point, expectations should be clear. Perhaps they have not been clear enough, so let me be a bit clearer.
Constructing a schema from a SDL string is not pythonic. It deviates typing to other system and makes harder to glue things together. Supporting python annotations is however quite pythonic, and I believe it solves two issues:
This PR was created with the initial intention to interexchange Graphene and Strawberry types. As having annotation support is one of the objectives of Graphene (since a few years ago, sadly it haven't been implemented), this PR collides with the first principle as: people will need to use two libraries, a non-cohesive syntax and more artifacts. If someone has other real usage for interexchange GraphQL types that is not covered by what I commented, please let me know. |
|
@syrusakbary I've just pushed another commit to show how you could interchange GraphQL-core SDL parsing with Graphene types. It's another example of something that would allow people to incrementally migrate to Graphene from another paradigm (e.g. using ariadne) without having to re-write everything and would not be possible without this change.
That was the original use case that I had in mind but as I have demonstrated there are other cases that would benefit from this change. We can all agree that adding type support to Graphene is an objective of Graphene but blocking this change because you believe it will encourage people to migrate to a different library is just petty. I have no intention of encouraging people to interchange libraries, this change is just the bare minimum to make it possible so that people can experiment if they want. |
Oh no, is not that way. My main concern is that it might make us lazy as Graphene developers. If people have already a way to use typing within Graphene they will not look to other packages (for example), with this PR they will simply stop asking for that feature within Graphene which will led to us relaxing on adding the feature to the library (which I believe very strongly Graphene should have). My main concern about the PR is that it discourages usage within the library adding a scape goat. I don't believe that's a good way of approaching future problems. The side concern I have is that I this is resulting more in a philosophical discussion than practical one. I haven't seen an issue in this repo where people are trying to intercommunicate different libraries. I'm sorry but I feel very strong about this. If you want to discuss further I'm happy to jump on a Zoom/hangouts to clarify further :) |
|
Closing the PR for now. For future readers: if you have a use case of mixing Graphene types with other types for an existing project, please write here your use case so we can recalibrate the strategy. |
This PR allows adding plain GraphQL types to a Graphene schema. This opens up the option for other libraries to interop with Graphene.