Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Dicussion: Support mutations in graphene-sqlalchemy #285

Closed
sreerajkksd opened this issue Sep 8, 2020 · 7 comments
Closed

Dicussion: Support mutations in graphene-sqlalchemy #285

sreerajkksd opened this issue Sep 8, 2020 · 7 comments

Comments

@sreerajkksd
Copy link

This is going to be a discussion thread to debate whether it is good to implement mutations in graphene-sqlalchemy. There is definitely a scope for this feature and I think it's useful, so the real question is more towards how to implement it.

We can probably start with creating objects and then later expand to updating the various attributes.

There are discussions around this topic in #213 as well as #29. I'll copy the relevant items here so that we won't have to repeat them again.

Points to note:

  • Have a class named SQLAlchemyInputObjectType and have model and exclude_fields as meta properties.
class CreateUser(SQLAlchemyInputObjectType):
    class Meta:
        exclude_fields = ('id', )
        model = UserModel
  • No need to worry about hybrid or composite columns as they are basically created from other columns. So, we just need a mechanism to just accept the fields. So, a function named construct_fields_for_input.

  • How to handle relationships ?
    Since we are creating row entries for the table, we can probably accept id of the related table entry and convert it to the database id.

@jnak thoughts ?

@maquino1985
Copy link

maquino1985 commented Oct 2, 2020

It's beyond obvious that this is useful, it's basically required for anyone to seriously use graphene-sqlalchemy in a complex project. I've been using this in my fork for about 2 years now. It's not pretty because I mostly copy pasted stuff from the library code for creating these objects with a few changes, but it gets the job done.

class SQLAlchemyMutation(Mutation):
    @classmethod
    def __init_subclass_with_meta__(
            cls,
            model=None,
            create=False,
            delete=False,
            registry=None,
            arguments=None,
            only_fields=(),
            structure: Type[Structure] = None,
            exclude_fields=(),
            **options,
    ):
        meta = SQLAlchemyMutationOptions(cls)
        meta.create = create
        meta.model = model
        meta.delete = delete

        if arguments is None and not hasattr(cls, "Arguments"):
            arguments = {}
            # don't include id argument on create
            if not meta.create:
                arguments["id"] = ID(required=True)

            # don't include input argument on delete
            if not meta.delete:
                inputMeta = type(
                    "Meta",
                    (object,),
                    {
                        "model": model,
                        "exclude_fields": exclude_fields,
                        "only_fields": only_fields,
                    },
                )
                inputType = type(
                    cls.__name__ + "Input",
                    (SQLAlchemyInputObjectType,),
                    {"Meta": inputMeta},
                )
                arguments = {"input": inputType(required=True)}
        if not registry:
            registry = get_global_registry()
        output_type: ObjectType = registry.get_type_for_model(model)
        if structure:
            output_type = structure(output_type)
        super(SQLAlchemyMutation, cls).__init_subclass_with_meta__(
            _meta=meta, output=output_type, arguments=arguments, **options
        )

    @classmethod
    def mutate(cls, root, info, **kwargs):
        session = get_session(info.context)
        with session.no_autoflush:
            meta = cls._meta

            if meta.create:
                model = meta.model(**kwargs["input"])
                session.add(model)
            else:
                model = (
                    session.query(meta.model)
                        .filter(meta.model.id == kwargs["id"])
                        .first()
                )
            if meta.delete:
                session.delete(model)
            else:

                def setModelAttributes(model, attrs):
                    relationships = model.__mapper__.relationships
                    for key, value in attrs.items():
                        if key in relationships:
                            if getattr(model, key) is None:
                                # instantiate class of the same type as
                                # the relationship target
                                setattr(model, key, relationships[key].mapper.entity())
                            setModelAttributes(getattr(model, key), value)
                        else:
                            setattr(model, key, value)

                setModelAttributes(model, kwargs["input"])
            session.flush()  # session.commit() now throws session state exception: 'already committed'

            return model

    @classmethod
    def Field(cls, *args, **kwargs):
        return Field(
            cls._meta.output, args=cls._meta.arguments, resolver=cls._meta.resolver
        )

You can use it like this, very easy:
(in the Meta class set create=True for creation, delete=True for deletion. Setting neither will update)

class CreateWorkflow(SQLAlchemyMutation):
    """CreateWorkflow"""

    class Arguments:
        """Arguments"""

        input = graphene.Argument(WorkflowInput, required=True)

    class Meta:
        """Meta"""

        create = True
        model = WorkflowModel

    @classmethod
    def mutate(cls, root: Any, info: ResolveInfo, input) -> WorkflowModel:
        """mutate

        Args:
            root (Any):
            info (ResolveInfo):
            **kwargs (Any):

        Returns:
            model (WorkflowModel)

        """

        workflow = WorkflowFactory.generate(workflow_input=input)
        return workflow

@seanxlliu
Copy link

@maquino1985 This a good example and good start. Maybe we can work together on a PR.

@jbvsmo
Copy link

jbvsmo commented Jun 14, 2021

Any news on this?

@kpraznik
Copy link

Any news on this?

maybe tagging @jnak will help to get some feedback.

@ArielLahiany
Copy link

Hello. Just switched from Django to FastAPI for lightweighting a project I'm working on.
There is a big Django project here in the community, called "Saleor". It seems that they have perfected the model mutation logic, based on Graphene. Please Consider to have a look on how they implemented the code 😃

@alexanderhupfer
Copy link

Another blueprint could be the way Dgraph autogenerates mutations:

https://dgraph.io/docs/graphql/mutations/mutations-overview/

@erikwrede
Copy link
Member

erikwrede commented May 5, 2022

There's definitely a use case for this!
Just adding a few remarks:

No need to worry about hybrid or composite columns as they are basically created from other columns. So, we just need a mechanism to just accept the fields. So, a function named construct_fields_for_input.

I think there's a more differentiated view to this. In some cases, only the composite/hybrid field is relevant to the API. When users want to create/modify an object, they might only know about the hybrid property but not the underlying columns/data structures. Sqlalchemy provides setters for hybrid, tackling this problem. Optimally, our solution should provide similar functionality. However, we can leave that to the developer implementing the mutation for now.
This would be an issue that needs to be resolved first for auto-generation.

Another blueprint could be the way Dgraph autogenerates mutations

This is also a great idea, but I'd see that as a separate feature since it drastically extends the scope of the input objects. A first step could be to design the input objects to be compatible with future extensions like auto-generation.

InputObjects would already be helpful in common mutations aimed at modifying an object.

Some other points that come to my mind are the following:

  • Handling of required & excluded fields on a mutation basis.
    There should be an easy way to configure which fields of an input object are required for a particular mutation. An example of
    this is "create" and "update" mutations. Of course, the dev could manually check for specific fields to be not null in the input
    type, but providing a general solution might improve this feature's usability. Staying with the create and update example, an update mutation may require the ID to be present while a create mutation would not allow IDs in the input. The original proposal by would now require two separate inputobjects (one excluding the ID, one not excluding any fields). That leads to a lot of clutter when applied to the entire Schema.
  • Automatic generation of InputType Objects
    In most cases, an InputType would have the same fields as its corresponding SQLAlchemyObjectType (OutputType). It might be worth investigating overridable auto-generation of the input types based on the output types. This could be configured in the meta options of the OutputType. @maquino1985's approach could also be an alternative.

@graphql-python graphql-python locked and limited conversation to collaborators May 13, 2022
@erikwrede erikwrede converted this issue into discussion #346 May 13, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Projects
None yet
Development

No branches or pull requests

8 participants