simonw / datasette-graphql

Datasette plugin providing an automatic GraphQL API for your SQLite databases
https://datasette-graphql-demo.datasette.io/
Apache License 2.0
98 stars 6 forks source link

500: Field names must not be keywords: 'if' #84

Closed simonw closed 2 years ago

simonw commented 2 years ago

Got this on https://datasette-graphql-demo.datasette.io/github/issue_comments - likely caused by the Graphene 3.0 upgrade in #80.

simonw commented 2 years ago

Here's a traceback - looks like the error comes from the Python stdlib dataclasses module!

  File "/Users/simon/Dropbox/Development/datasette-graphql/datasette_graphql/utils.py", line 147, in schema_for_database
    table_node_class = await make_table_node_class(
  File "/Users/simon/Dropbox/Development/datasette-graphql/datasette_graphql/utils.py", line 482, in make_table_node_class
    return type(meta.graphql_name, (graphene.ObjectType,), table_dict)
  File "/Users/simon/.local/share/virtualenvs/datasette-graphql-n1OSJCS8/lib/python3.8/site-packages/graphene/types/objecttype.py", line 45, in __new__
    dataclass = make_dataclass(name_, fields, bases=())
  File "/Users/simon/.pyenv/versions/3.8.2/lib/python3.8/dataclasses.py", line 1219, in make_dataclass
    raise TypeError(f'Field names must not be keywords: {name!r}')
TypeError: Field names must not be keywords: 'if'
simonw commented 2 years ago
-> dataclass = make_dataclass(name_, fields, bases=())

(Pdb) pprint(fields)
[('name',
  'typing.Any',
  Field(name=None,type=None,default=None,default_factory=<dataclasses._MISSING_TYPE object at 0x10cded970>,init=True,repr=True,hash=None,compare=True,metadata=mappingproxy({}),_field_type=None)),
 ('runs_on',
  'typing.Any',
  Field(name=None,type=None,default=None,default_factory=<dataclasses._MISSING_TYPE object at 0x10cded970>,init=True,repr=True,hash=None,compare=True,metadata=mappingproxy({}),_field_type=None)),
 ('strategy',
  'typing.Any',
  Field(name=None,type=None,default=None,default_factory=<dataclasses._MISSING_TYPE object at 0x10cded970>,init=True,repr=True,hash=None,compare=True,metadata=mappingproxy({}),_field_type=None)),
 ('needs',
  'typing.Any',
  Field(name=None,type=None,default=None,default_factory=<dataclasses._MISSING_TYPE object at 0x10cded970>,init=True,repr=True,hash=None,compare=True,metadata=mappingproxy({}),_field_type=None)),
 ('if',
  'typing.Any',
  Field(name=None,type=None,default=None,default_factory=<dataclasses._MISSING_TYPE object at 0x10cded970>,init=True,repr=True,hash=None,compare=True,metadata=mappingproxy({}),_field_type=None)),
 ('id',
  'typing.Any',
  Field(name=None,type=None,default=None,default_factory=<dataclasses._MISSING_TYPE object at 0x10cded970>,init=True,repr=True,hash=None,compare=True,metadata=mappingproxy({}),_field_type=None)),
 ('workflow',
  'typing.Any',
  Field(name=None,type=None,default=None,default_factory=<dataclasses._MISSING_TYPE object at 0x10cded970>,init=True,repr=True,hash=None,compare=True,metadata=mappingproxy({}),_field_type=None)),
 ('repo',
  'typing.Any',
  Field(name=None,type=None,default=None,default_factory=<dataclasses._MISSING_TYPE object at 0x10cded970>,init=True,repr=True,hash=None,compare=True,metadata=mappingproxy({}),_field_type=None)),
 ('steps_list',
  'typing.Any',
  Field(name=None,type=None,default=None,default_factory=<dataclasses._MISSING_TYPE object at 0x10cded970>,init=True,repr=True,hash=None,compare=True,metadata=mappingproxy({}),_field_type=None))]
(Pdb) 

So yup, there's an if column. See it here: https://github-to-sqlite.dogsheep.net/github/steps

simonw commented 2 years ago

My mistake, it's not the standard library dataclasses, it's in Graphene: https://github.com/graphql-python/graphene/blob/a61f0a214d4087acac097ab05f3969d77d0754b5/graphene/pyutils/dataclasses.py#L1159-L1164

        if not isinstance(name, str) or not name.isidentifier():
            raise TypeError(f"Field names must be valid identifers: {name!r}")
        if keyword.iskeyword(name):
            raise TypeError(f"Field names must not be keywords: {name!r}")
        if name in seen:
            raise TypeError(f"Field name duplicated: {name!r}")
        seen.add(name)
simonw commented 2 years ago

There's an unanswered issue about this here: https://github.com/graphql-python/graphene/issues/1348

simonw commented 2 years ago

I already have code that tries to come up with unique, valid names for columns here: https://github.com/simonw/datasette-graphql/blob/a71e56cbfb8e7293860f2a5b040b482a87431eea/datasette_graphql/utils.py#L701-L724

I can teach it to also disallow Python keywords, by pre-populating the names set with them.