Update to construct_fields creates problems (this method has broader use than just library SQLAObjectType)

[maquino1985]

graphene-sqlalchemy/graphene_sqlalchemy/types.py

Line 40 in 8ad1f75

registry.register_orm_field(obj_type, name, column)

this line breaks code I use to create sqlalchemy Interfaces and InputObjectTypes. please parameterize a boolean operator to skip this or something?

[jnak]

Thanks for reporting. Can you elaborate on your use case?
construct_fields is considered a private function of SQLAObjectType.

Btw I'm about to merge a PR that is introduces a lot of changes this function (#214). I

[maquino1985]

if you look at my fork, you can see I use construct_fields to auto-generate fields for a variety of (new) graphene-sqlalchemy types such as mutations, input object types, and interfaces. Right now, that register_orm_field requires that whatever object_type is passed to is is a subclass of SQLAlchemyObjectType, which it doesn't need to be.

I also have the register_orm_field accept any of the new types I made, so that it doesn't throw an assertion error. It works (although I haven't thought about whether it's really necessary to call this new method on the registry that wasn't there in the last version.) but everything seems to work in this way.

e.g.

def register_orm_field(self, obj_type, field_name, orm_field, assert_type: bool = True):
        from .types import SQLAlchemyObjectType, SQLAlchemyInputObjectType
        from .interfaces import SQLAlchemyInterface
        if assert_type:
            if not isinstance(obj_type, type) or not issubclass(
                    obj_type, (SQLAlchemyObjectType, SQLAlchemyInterface, SQLAlchemyInputObjectType)
            ):
                raise TypeError(
                    "Expected one of {}, but got: {!r}".format([x.__name__ for x in (SQLAlchemyObjectType, SQLAlchemyInterface, SQLAlchemyInputObjectType)], obj_type)
                )
        if not field_name or not isinstance(field_name, str):
            raise TypeError("Expected a field name, but got: {!r}".format(field_name))
        self._registry_orm_fields[obj_type][field_name] = orm_field

[maquino1985]

I also had to do the same thing for UnsortedConnectionFIeld...

class UnsortedSQLAlchemyConnectionField(ConnectionField):
    @property
    def type(self, assert_type: bool = True):
        from .types import SQLAlchemyObjectType, SQLAlchemyInputObjectType
        from .interfaces import SQLAlchemyInterface
        _type = super(ConnectionField, self).type
        if issubclass(_type, Connection):
            return _type

        if assert_type:
            assert issubclass(_type, (SQLAlchemyObjectType, SQLAlchemyInterface, SQLAlchemyInputObjectType)), (
                "SQLALchemyConnectionField only accepts {} types, not {}"
            ).format([x.__name__ for x in (SQLAlchemyObjectType, SQLAlchemyInterface, SQLAlchemyInputObjectType)], _type.__name__)
        assert _type._meta.connection \
            , "The type {} doesn't have a connection".format(
            _type.__name__
        )
        return _type._meta.connection

[jnak]

I see. It seems that you are using construct_fields for a use case that is not currently supported. I don't think it makes sense to update this function for your specific use case unless we start officially supporting it through tests and documentation.

If you'd like to make a case for SQLAlchemyInputObjectType or SQLAlchemyInterface, please start a separate issue that outlines the feature and how do you think it should be implemented. Once we agree on a path forward, you are then welcome to submit a pull request.

I understand that this might be frustrating to hear but it is only way we can keep the codebase healthy.

[maquino1985]

Sure. I understand. Thanks.

On Thu, Jun 6, 2019 at 5:10 PM Julien Nakache ***@***.***> wrote: I see. It seems that you are using construct_fields for a use case that is not currently supported. I don't think it makes sense to update this function for your specific use case unless we start officially supporting it through tests and documentation. If you'd like to make a case for SQLAlchemyInputObjectType or SQLAlchemyInterface, please start a separate issue that outlines the feature and how do you think it should be implemented. Once we agree on a path forward, you are then welcome to submit a pull request. I understand that this might be frustrating to hear but it is only way we can keep the codebase healthy. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#223?email_source=notifications&email_token=ADE52SKH72SBXVAT6S4HS5TPZF4NFA5CNFSM4HVHFSJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXEFPVA#issuecomment-499668948>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADE52SPQG6ANGMIJGS2UULLPZF4NFANCNFSM4HVHFSJA> .

-- Sent from Gmail Mobile

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related topics referencing this issue.