-
-
Notifications
You must be signed in to change notification settings - Fork 138
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
Support adding types to GraphQLSchema #154
Comments
Hi @alexchamberlain. Thanks for the suggestion. Before extending the API in our own ways, we should ask at GraphQL.js how they would do it. They usually have some good ideas and if something is really missing, then I prefer if it can be added there, and then ported to Python. If you don't feel at home in the JavaScript world, I can also create an issue for you there. But you could help me by better explaining your use case with a runnable code example and maybe some motivation why you're doing it that way. |
I think this is a minimal(ish) example; I tested in a python3.9 virtual environment with only Imagine that from graphql import build_schema
from graphql.type import (
GraphQLField,
GraphQLList,
GraphQLNonNull,
GraphQLObjectType,
GraphQLString,
assert_valid_schema,
)
from graphql.type.schema import InterfaceImplementations
from graphql.utilities import print_schema
CHARACTER_CLASSES = ["Human", "Droid", "Animal", "Fungus", "Alien"]
schema = build_schema(
"""
enum Episode { NEWHOPE, EMPIRE, JEDI }
interface Character {
id: String!
name: String
friends: [Character]
appearsIn: [Episode]
}
type Query {
hero(episode: Episode): Character
}
"""
)
character_interface = schema.get_type("Character")
episode_class = schema.get_type("Episode")
query = schema.get_type("Query")
# TODO: Upstream an add type method on GraphQLSchema
if character_interface.name in schema._implementations_map:
implementations = schema._implementations_map[character_interface.name]
else:
implementations = schema._implementations_map[character_interface.name] = InterfaceImplementations(
objects=[], interfaces=[]
)
for character in CHARACTER_CLASSES:
concrete_class = GraphQLObjectType(
character,
{
"id": GraphQLField(GraphQLNonNull(GraphQLString)),
"name": GraphQLField(GraphQLString),
"friends": GraphQLField(GraphQLList(character_interface)),
"appearsIn": GraphQLField(GraphQLList(episode_class)),
"primaryFunction": GraphQLField(GraphQLString),
},
interfaces=[character_interface],
)
schema.type_map[character] = concrete_class
implementations.objects.append(concrete_class)
query.fields[character.lower()] = GraphQLField(concrete_class, args={"id": GraphQLNonNull(GraphQLString)})
schema._sub_type_map = {}
assert_valid_schema(schema)
print(print_schema(schema)) This is somewhat akin to extending a schema or similar. The output is rather long, so I posted to a gist.
I've not really engaged with the JS side of the GraphQL community before, but I can certainly give it a go if that is preferred. |
Ok, thanks for the example code. Just for better understanding your use case, why don't you do it this way? # only the interfaces
sdl = """
enum Episode { NEWHOPE, EMPIRE, JEDI }
interface Character {
id: String!
name: String
friends: [Character]
appearsIn: [Episode]
}
type Query {
hero(episode: Episode): Character
}
"""
# add concrete types
sdl += '\n'.join(
f"""
type {character} implements Character {{
id: String!
name: String
friends: [Character]
appearsIn: [Episode]
primaryFunction: String
}}
""" for character in CHARACTER_CLASSES)
schema = build_schema(sdl) Update: Forgot to add the fields to the query class, but that could be done similarly. |
In the real case, the interface is quite light - basically, just the |
Ok, but the question is why don't you compose the SDL dynamically (using the config system), as shown above? Or on the other hand, why don't you compose everything programmatically without SDL, like shown below? from enum import Enum
class EpisodeEnum(Enum):
NEWHOPE = 4
EMPIRE = 5
JEDI = 6
episode_enum = GraphQLEnumType('Episode', EpisodeEnum)
character_interface = GraphQLInterfaceType('Character', lambda: {
'id': GraphQLField(
GraphQLNonNull(GraphQLString)),
'name': GraphQLField(
GraphQLString),
'friends': GraphQLField(
GraphQLList(character_interface)),
'appearsIn': GraphQLField(
GraphQLList(episode_enum))})
for character_class in CHARACTER_CLASSES:
concrete_character = GraphQLObjectType(
character_class,
{
"id": GraphQLField(GraphQLNonNull(GraphQLString)),
"name": GraphQLField(GraphQLString),
"friends": GraphQLField(GraphQLList(character_interface)),
"appearsIn": GraphQLField(GraphQLList(episode_enum)),
"primaryFunction": GraphQLField(GraphQLString),
},
interfaces=[character_interface])
query_fields[character_class.lower()] = GraphQLField(
concrete_character, args={
"id": GraphQLArgument(GraphQLNonNull(GraphQLString))})
query_type = GraphQLObjectType('Query', query_fields)
schema = GraphQLSchema(query_type) |
I guess I didn't consider that. It feels odd to generate a string, just for that string to be parsed to objects we can construct directly.
That is certainly an option - we do this in other services. For this service, we are only dynamically generating part of the API, so it felt nicer to be able to write the bit we weren't generating in SDL, as it's easier to review. |
Ok, thanks for the feedback. Actually I think the way you do is not so bad, and doable already with the public API. You can simply replace the following code: if character_interface.name in schema._implementations_map:
implementations = schema._implementations_map[character_interface.name]
else:
implementations = schema._implementations_map[character_interface.name] = InterfaceImplementations(
objects=[], interfaces=[]
) with this: implementations = schema.get_implementations(character_interface) And regarding resetting the |
I may be wrong, as I haven't actually checked it, but if the interface is not in graphql-core/src/graphql/type/schema.py Line 345 in 2585715
|
Sorry, yes, you're right about that. I think I will open an issue upstream. |
Thanks Christoph. Please let me know if there's anything I can do to help - if there are code changes that come out of this discussion, I'd be more than happy to contribute. |
@alexchamberlain feel free to clarify or make suggestions upstream. |
@alexchamberlain: @yaacovCR suggested to consider the schema frozen after it has been created, and to recreate it from the config (keyword args in Python), see the example below. Would that work for you? It has the advantage that it's very clean, and you don't need to care about updating internal data like type or implementation maps. # Create schema from SDL
schema = build_schema(sdl)
# Add new types to query
query = schema.query_type
character_interface = schema.get_type("Character")
episode_enum = schema.get_type("Episode")
for character in CHARACTER_CLASSES:
character_type = GraphQLObjectType(
character,
{
"id": GraphQLField(GraphQLNonNull(GraphQLString)),
"name": GraphQLField(GraphQLString),
"friends": GraphQLField(GraphQLList(character_interface)),
"appearsIn": GraphQLField(GraphQLList(episode_enum)),
"primaryFunction": GraphQLField(GraphQLString),
},
interfaces=[character_interface],
)
query.fields[character.lower()] = GraphQLField(character_type, args={
"id": GraphQLNonNull(GraphQLString)})
# Recreate schema
kwargs = schema.to_kwargs()
kwargs.update(query=query)
schema = GraphQLSchema(**kwargs) |
Upstream it was recommended to consider a schema immutable once it has been created, for various good reasons. If you want to modify it, you must create a new schema from |
Thanks again @Cito for your help on this issue and escalating upstream. Happy New Year! |
I have a use case where part of the schema is specified using the SDL, and part of the schema is generated in code. In particular, interfaces are in SDL, while the concrete types are in code. To correctly modify the
GraphQLSchema
object that is generated by parsing the initial schema, you need to:type_map
_implementations_map
appropriately_sub_type_map
Would you be open to adding an
add_type
method or similar that takes care of all of the above?The text was updated successfully, but these errors were encountered: