Skip to content
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

Documentation on connection_field_factory #228

Closed
rdemetrescu opened this issue Jun 13, 2019 · 15 comments · Fixed by #229
Closed

Documentation on connection_field_factory #228

rdemetrescu opened this issue Jun 13, 2019 · 15 comments · Fixed by #229

Comments

@rdemetrescu
Copy link

I'm using graphene-sqlalchemy version 2.2.0 and I'm getting lots of createConnectionField is deprecated and will be removed in the next major version. Use SQLAlchemyObjectType.Meta.connection_field_factory instead. warnings.

Is there any example on how to configure this connection_field_factory? Are there any default factories? So far I couldn't find any example using this new feature on the project site.

Thank you!

@zmwangx
Copy link

zmwangx commented Jun 13, 2019

The problem here is that createConnectionField is used in default_connection_field_factory, so the library itself is relying on its supposedly deprecated API, and users get a WARNING through no fault of their own.

@vaskokj
Copy link

vaskokj commented Jun 14, 2019

Same exact issue.

Whats odd to me is that it happens when you add more than one class that uses SQLAlchemyObjectType.

With only one class subclassing the SQLAlchemyObjectType you don't get the warning. If you uncomment the commented out section you will get the warning.

import graphene
from graphene import relay
from graphene_sqlalchemy import SQLAlchemyObjectType, SQLAlchemyConnectionField
from myapp.models import Channel as ChannelModel, Model as ModelModel

# class Model(SQLAlchemyObjectType):
#     class Meta:
#         model = ModelModel
#         interfaces = (relay.Node, )

class Channel(SQLAlchemyObjectType):
    class Meta:
        model = ChannelModel
        interfaces = (relay.Node, )

class ChannelConnection(relay.Connection):
    class Meta:
        node = Channel

class Query(graphene.ObjectType):
    node = relay.Node.Field()
    all_channels = SQLAlchemyConnectionField(ChannelConnection)

schema = graphene.Schema(query=Query)

@zmwangx
Copy link

zmwangx commented Jun 14, 2019

With only one class subclassing the SQLAlchemyObjectType you don't get the warning.

Nah, that’s probably because your Channel does not auto-generate a connection field, while your Model does. Check your schema, including nested levels. There’s nothing magical about the warning: it’s printed every time a connection field is created with the default factory.

@vaskokj
Copy link

vaskokj commented Jun 14, 2019

@zmwangx I added the ConnectionField to the Model and it still does it. Uncomment the comments and it will throw the warning. this is the entire schema.

import graphene
from graphene import relay
from graphene_sqlalchemy import SQLAlchemyObjectType, SQLAlchemyConnectionField
from myapp.models import Channel as ChannelModel, Model as ModelModel


#class Model(SQLAlchemyObjectType):
#   class Meta:
#        model = ModelModel
#       interfaces = (relay.Node, )


class Channel(SQLAlchemyObjectType):
    class Meta:
        model = ChannelModel
        interfaces = (relay.Node, )

class Query(graphene.ObjectType):
    node = relay.Node.Field()
    all_channels = SQLAlchemyConnectionField(Channel)
    #all_models = SQLAlchemyConnectionField(Model)

schema = graphene.Schema(query=Query)

@zmwangx
Copy link

zmwangx commented Jun 15, 2019

I'm talking about auto-generated connection field from your SQLA model, not a SQLAlchemyConnectionField you explicitly add to the query (which does not go through the default connection field factory — you already explicitly instantiated the field).

A default connection field is created when you have, say, a one to many relationship on your SQLA model. You don't show your database models here, but say we have two very simple tables:

Base = declarative_base()


class UserModel(Base):
    __tablename__ = "user"
    id = Column(Integer, primary_key=True)
    transactions = relationship("TransactionModel", back_populates="user")


class TransactionModel(Base):
    __tablename__ = "transaction"
    id = Column(Integer, primary_key=True)
    user_id = Column(Integer, ForeignKey("user.id"), nullable=False)
    user = relationship("UserModel", back_populates="transactions")


class User(SQLAlchemyObjectType):
    class Meta:
        model = UserModel
        interfaces = (Node,)


class Transaction(SQLAlchemyObjectType):
    class Meta:
        model = TransactionModel
        interfaces = (Node,)

Then the generated User type will have a transactions field which is an auto-generated TransactionConnection, whereas the Transaction type won't have an auto-generated connection field.

Update: I made a mistake when I said "the Transaction type won't have an auto-generated connection field." Since it has a user field, it also ends up with an auto-generated connection field through User; one has to exclude user to see the effect.

@jnak
Copy link
Collaborator

jnak commented Jun 15, 2019

Hi everyone,

I introduced that warning in this PR when refactoring registerConnectionFieldFactory. It looks like the warning is incorrectly triggered even if registerConnectionFieldFactory is not used. My mistake. I'll be sending a PR over the weekend to fix this. You can safely ignore the warning in the meantime.

Apologies for the confusion.
J

@jnak
Copy link
Collaborator

jnak commented Jun 15, 2019

Here is the fix. Thanks for reporting the issue.

@vaskokj
Copy link

vaskokj commented Jun 17, 2019

I'm talking about auto-generated connection field from your SQLA model, not a SQLAlchemyConnectionField you explicitly add to the query (which does not go through the default connection field factory — you already explicitly instantiated the field).

A default connection field is created when you have, say, a one to many relationship on your SQLA model. You don't show your database models here, but say we have two very simple tables:

Base = declarative_base()


class UserModel(Base):
    __tablename__ = "user"
    id = Column(Integer, primary_key=True)
    transactions = relationship("TransactionModel", back_populates="user")


class TransactionModel(Base):
    __tablename__ = "transaction"
    id = Column(Integer, primary_key=True)
    user = relationship("UserModel", back_populates="transactions")


class User(SQLAlchemyObjectType):
    class Meta:
        model = UserModel
        interfaces = (Node,)


class Transaction(SQLAlchemyObjectType):
    class Meta:
        model = TransactionModel
        interfaces = (Node,)

Then the generated User type will have a transactions field which is an auto-generated TransactionConnection, whereas the Transaction type won't have an auto-generated connection field.

I thought I had figured it out but obviously not. I'm still confused...this stuff seems very hand wavy (magical stuff going on in the background) and its really confusing me. For example, was getting this (#153) error when I had "Connection" at the end of a class name. This type of stuff makes things extremely difficult to understand whats going on when its valid Python and its simply due to all the stuff going on in the background. While not an error, its something thats not very obvious of whats going on and makes things extremely difficult to understand and debug.

But I digress, I'm still confused about this...

You stated:

I'm talking about auto-generated connection field from your SQLA model, not a SQLAlchemyConnectionField you explicitly add to the query (which does not go through the default connection field factory — you already explicitly instantiated the field).

Based off this statement it should not throw the warning in my example when you uncomment those lines in my example above correct?

Here is what I'm seeing with your example...

class User(Base):
    __tablename__ = 'user'

    id = Column(Integer, primary_key=True, server_default=text("nextval('\"User_id_seq\"'::regclass)"))

    transactions = relationship("Transaction", back_populates="user")

class Transaction(Base):
    __tablename__ = 'transaction'

    id = Column(Integer, primary_key=True, server_default=text("nextval('transaction_id_seq'::regclass)"))
    user_id = Column(ForeignKey('user.id'), nullable=False)

    user = relationship("User", back_populates="transactions")
class User(SQLAlchemyObjectType):
    class Meta:
        model = UserModel
        interfaces = (relay.Node,)


class Transaction(SQLAlchemyObjectType):
    class Meta:
        model = TransactionModel
        interfaces = (relay.Node,)
class Query(graphene.ObjectType):
    node = relay.Node.Field()

    #all_users = SQLAlchemyConnectionField(User)
    #all_transaction = SQLAlchemyConnectionField(Transaction)

schema = graphene.Schema(query=Query)

Commented out all_users and all_transaction you do not get the warning. Removing the comment (#), and the warnings appear. This is basically the opposite of what I would expect if the "default" is to automatically generate the code. I kind of see what you are saying about user = relationship("User", back_populates="transactions") driving this and that basically if graphene-sqlalchemy is generating a field automatically it will appear, if its explicitly called out it won't display the warning. But regardless I can't tell when graphene-sqlalchemy is actually doing this.

@maquino1985
Copy link

I tried a million ways to figure out how to instantiate a connection field factory via SQLAlchemyObjectType.Meta.connection_field_factory to get this warning to stop spamming me to death...

@vaskokj
Copy link

vaskokj commented Jun 17, 2019

I tried a million ways to figure out how to instantiate a connection field factory via SQLAlchemyObjectType.Meta.connection_field_factory to get this warning to stop spamming me to death...

Yeah, me too. I'm still trying to figure out why its throwing the messages. Its really not clear to me on how/why its happening. The fluffery in the library seems to be confusing me. Luckily it will be fixed in the next release.

@jnak
Copy link
Collaborator

jnak commented Jun 17, 2019

@maquino1985 @vaskokj

#228 (comment)
#228 (comment)

This will be merged tomorrow

@vaskokj
Copy link

vaskokj commented Jun 17, 2019

@jnak appreciate it! I saw your message that it will be fixed. I was just talking to @zmwangx on why the messages were appearing in different circumstances. I know the whole point would be moot after it gets fixed but just to know whats going on behind the scenes is why I was asking. The whole graphene-sqlalchemy library seems to be doing some magic in the background depending on if it detects different things so I just was wanting to be more aware of what was going on.

@zmwangx
Copy link

zmwangx commented Jun 18, 2019

Probably not a good place to discuss how graphene-sqlalchemy works, but guess I'll respond one more time (everyone else: sorry about the noise).

@vaskokj Use introspection[1] to look at generated schema (GraphiQL Docs panel is another way to consume the introspection data, but sometimes it's nice to see everything at once). It should help you a lot.

Commented out all_users and all_transaction you do not get the warning.

That's because you also commented out all references to User and Transaction. Use introspection and you'll realize they don't appear in the schema at all. No inferred types, of course no warning.

Forget about SQLAlchemyConnectionField, you think the connection field-related warning came from SQLAlchemyConnectionField when it's got nothing to do with it. Instead, consider this Query type:

class Query(ObjectType):
    user = Field(User)  # Field is graphene.Field
    transaction = Field(Transaction)

And play with field exclusion:

class Transaction(SQLAlchemyObjectType):
    class Meta:
        model = TransactionModel
        interfaces = (Node,)
        exclude_fields = ("user",)  # Try to turn this on and off

Again, use introspection. You should understand what I meant.

Finally, I found the source code fairly readable, so you can always just read the code to understand what's going on.

[1] https://graphql.org/learn/introspection/

(I made a mistake in my original comment and posted an update.)

@jnak jnak closed this as completed in #229 Jun 18, 2019
@jnak
Copy link
Collaborator

jnak commented Jun 18, 2019

The fix is now available on PyPi: https://pypi.org/project/graphene-sqlalchemy/2.2.1/

I will document connection_field_factory as part of an upcoming larger effort to document this library.

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants