strawberry-graphql / strawberry

A GraphQL library for Python that leverages type annotations 🍓
https://strawberry.rocks
MIT License
3.87k stars 511 forks source link

Allow passing dataclass kwargs to the underlying dataclass #2688

Open Corentin-Br opened 1 year ago

Corentin-Br commented 1 year ago

Feature Request Type

Description

Currently _wrap_dataclass doesn't allow setting kwargs to the dataclass it creates. This leads to the creation (among other things) of a __repr__ method, which may not be desired.

Case in point, I have classes that are SQLAlchemy models and wrapped with type and who inherits a generic __repr__. However, the underlying dataclass creates a new __repr__ method which makes me load all my attributes. (Which is obviously undesired).

Upvote & Fund

Fund with Polar

ThirVondukr commented 1 year ago

Are you using sqlalchemy dataclasses integration and wrapping it in a strawberry.type? I'd still have two separate models here 🤔

Corentin-Br commented 1 year ago

We're still on SQLAlchemy 1.4 for now.

Having two models means having to maintain both and needing a way to go from one to the other. It's also easy to modify a SQLAlchemy model and forget to update the schema model in turn.

We worked with two models for a while and for now we believe having a single unified model will be easier. We'll see down the line.

ThirVondukr commented 1 year ago

Your database model and graphql schema are different entities and shouldn't directly depend on each other 🤔 It's easy for an opposite situation to occur: You can add a private field to your class and it would be automatically exposed in your schema. You also lose a lot of flexibility in how you can map your db models to strawberry types.

I usually do something like this:

class DbModel(Base):
    ...

@strawerry.type
class StrawberryType:
    id: strawberry.ID
    name: str
    price: int

    @classmethod
    def from_orm(cls, model: DbModel) -> Self:
        return cls(
            id=strawberry.ID(str(model.id)),
            name=model.name,
            price=model.price,
        )

This way you can map your models however you want, and your graphql models are explicitly defined. To summarize: I think having one entity handle multiple concerns like database schema and your api's schema is a bad idea.

ThirVondukr commented 1 year ago

Here's an example, but it's a bit outdated: https://gitlab.com/manga-microservices/manga/-/blob/main/src/gql/modules/manga/types.py

Corentin-Br commented 1 year ago

I appreciate your concern, and I do understand those issues, but from our point of view your way is just too much boilerplate to maintain.

Regardless of our specific usecase, I believe being able to pass kwargs is a good thing :)