Discussion: General Mechanism to Selectively Override Automatic Field Creations

まず、提案されている sql_field API を、SQLAlchemyObjectType、Graphene Field の定義、Meta のオーバーライド、および issue に記載されたフィールドの自動生成と併せて確認します。実装ファイルとテストを特定する前に、未解決の設計上の問題を解決し、オーバーライドの動作について合意します。完了とは、未確定の提案ではなく、決定され文書化された仕組みがあることを意味します。

Overview

In order to convert SQLAlchemy object models (SAM) into a full-blown GraphQL schema, SQLAlchemyObjectType (SAOT) does a lot of automatic mapping and conversions:

• it automatically names Graphene fields after their corresponding SAM column names
• it automatically converts SAM column types to common Graphene types
• it automatically converts SAM relationships to Relay-style paginated fields
• it automatically uses SAM descriptions to populate Graphene field's descriptions
• it automatically converts SQLAlchemy enums to Graphene enums (see https://github.com/graphql-python/graphene-sqlalchemy/issues/208)

While it is convenient that SAOT does all this out-of-the-box, developers should ultimately be in control of the Graphene types generated. Currently we can override SAOT default's behavior in 2 ways.

1. Use Meta

The Meta of SAOT allows the overriding of a few select automatic behaviors. For example, we can restrict what columns are exposed via the only_fields and exclude_fields parameters. In this example, Meta works fine because those parameters essentially act as toggles that completely disable or enable SOAM behavior for each field.

Similarly, we can currently override SAOT default connection field factory via connection_field_factory. But this is not flexible enough because we may want to have SAOT use different factories for different fields. For example, it may make sense to have only one paginated field be sortable (hence use SQLAlchemyConnectionField) and all the other fields be regular Relay-style collections (hence use UnsortedSQLAlchemyConnectionField).

One simple workaround would be to have a have a connection_field_factories parameter that takes a dictionary of field names to factories:

class MyType(SQLAlchemyObjectType):
    class Meta:
         model = MyModel
         default_connection_field_factory = MyConnectionField
         connection_field_factories = {
             'my_connection_field': SortedConnectionField,
         }

While that works fine, it leads to the creation of many parameters in the Meta class (eg https://github.com/graphql-python/graphene-sqlalchemy/pull/178 and https://github.com/graphql-python/graphene-sqlalchemy/issues/208#issuecomment-485819664) and we may end up with a Meta class that becomes hard to maintain.

My main concern though with this pattern is that scatters the logic for a field in separate Meta parameters. For example, we would need to do something like to rename a field, specify a connection factory and deprecate it:

class MyType(SQLAlchemyObjectType):
    class Meta:
         model = MyModel
         connection_field_factories = {
             'my_connection_field': SortedConnectionField,
             ...
         }
         descriptions = {
             'my_connection_field': "Some description",
             ...
         }
         rename = {
             'connection_field': 'my_connection_field',
            ...
         }
         deprecated = (
            'my_connection_field,
           ...
         )

We need to look at multiple places just to understand how we are overriding the SAOT default for a given field. I much prefer Graphene's pattern of grouping all the logic for a field into a Field definition on the ObjectType class.

2. Redefine the Graphene field

Since SAOT is a subclass of graphene ObjectType, we can always redefine the field auto-generated by SAOT. For example, we can change the type of a field like this:

class MyType(SQLAlchemyObjectType):
    class Meta:
         model = MyModel

   id = graphene.String()

Here it works pretty well because SAOT does not do much to the id column. The only downside is that we lose the description automatically generated by SAOT based on the SQLAlchemy column.

But the more logic SAOT does when converting a column to a field, the more we lose when we redefine a field from scratch. That's why I expect this pattern to become more of a problem as we improve SAOT. For example, one performance improvement that we probably want to do eventually is to only select the SAM columns for fields that have been queried. To do this, SAOT needs to build a mapping of columns to fields when it auto-generates fields. Regular Graphene fields (not generated by SAOT) will need to be manually annotated with the SAM fields they depend on. That's one more thing that we would lose" by redefining manually the Graphene field.

Proposed Solution

We could have a sql_field function that would look a lot like a graphene Field. The only difference is that instead of instantiating a field from scratch, it would use a combination of SOAT default and the user defined overrides:

class MyType(SQLAlchemyObjectType):
    class Meta:
        model = MyModel
  
  # Deprecate a field
  # The type and description are automatically set by SAOT
  id = sql_field(deprecated=True)  
  
  # Duplicate and rename a field
  id_v2 = sql_field(name='id') 

  # Override the default description
  column_1 = sql_field(description='This is the main column')

  # Override the default type / serializer 
  uuid = sql_field(type=graphene.String)

  # Override the default enum name
  kind = sql_field(enum_name='my_enum')

  # Override the default connection factory
  connection = sql_field(connection_factory=SortableConnectionField)

The main benefits of that approach are:

• It is very clear and consistent with Graphene API
• We only specify what we want to override
• All the logic for a given field is grouped in a sql_field call
• We can still use Meta to set default values such as connection_field_factory

One interesting side effect is that fields are automatically whitelisted:

class MyType(SQLAlchemyObjectType):
    class Meta:
        model = MyModel
        only_fields = ()   # Blacklist all fields by default
  
  # Both columns are whitelisted
  id = sql_field()  
  my_column = sql_field(description='Some description') 

From an API design perspective I can't think of any downside. Maybe a little more typing in some cases but I think we should optimize for clarity over the number of keystrokes.

What do you think of this pattern? Do you see any downsides? Do you have any ideas on how to make it better? I would love to hear your thoughts. I will send a PR if we agree this the right pattern to follow.