graphene-sqlalchemy-filter
graphene-sqlalchemy-filter copied to clipboard
art1415926535
Issue 42 - Generic solution for filtering over relationships
Great library!
This PR proposes a generic solution for filtering over relationships. For our project, we have to expose (almost) the entire database and creating custom filters for each relationship, especially when going over multiple levels, is just not feasible for us.
This is how it would work (Not functional, irrelevant bits are omitted):
The models:
class User(Base):
assignments = relationship('Assignment', back_populates='user')
class Task(Base):
name = Column(String(32))
assignments = relationship('Assignment', back_populates='task')
class Assignment(Base):
task_id = Column(Integer, ForeignKey('task.id'), primary_key=True)
task = relationship('Task', back_populates='assignments')
user_id = Column(Integer, ForeignKey('user.user_id'), primary_key=True)
user = relationship('User', back_populates='assignments')
active = Column(Boolean)
The filter:
class UserFilter(BaseFilter):
class Meta:
model = User
fields = {
'id': ['eq'],
'assignments': {
'task': {
'name': ['eq'],
},
'active': ['eq']
}
}
GraphQL (All users with active assignments on a certain task):
{
user(filters: {
assignments: {
and: [
{
task: {
name: "Write code"
}
},
{
active: true
}
]
}
}){
edges{
node{
username
}
}
}
}
Hi! Thanks for PR.
So far, I have only managed to partially look at the code... (_translate_many_filter is no longer easy 😅)
There are a few general notes:
- lints failed
- we need to update README.md (to let users know about the new functionality)
Follow these steps to run linters:
pip install nox
nox -s lint
@PaulSchweizer hey Paul, awesome work!
May I ask if this has been tested in the production? I'm thinking of just rip off your code and do the override directly since I don't see the author will add this change any time soon...
@JBrVJxsc yes, we are running this in production. Unfortunately I can't demo or show you anything because its a company-internal app. But please rip off the code and give it a go on your end. If you encounter any issues, please let me know. I want this to work properly because we heavily rely on it
@PaulSchweizer thanks Paul, let me try it out today. Will let you know if i see any issues
@PaulSchweizer, I'm trying to automatically generate filters:
def dfs(_model: DefaultMeta, _filters: Dict[str, Any], visited: set):
if _model.__name__ in visited:
return {}
visited.add(_model.__name__)
# create filters for columns
for column in _model.__table__.columns:
_filters[column.name] = [...]
# create filters for relationship recursively
for attr, field in inspect(_model).relationships.items():
relationship_filters = dfs(field.mapper.class_, {}, visited)
if relationship_filters:
_filters[attr] = relationship_filters
return _filters
But I'm getting an error from graphene:
AssertionError: Found different types with the same name in the schema: foobar, foobar.
I think somewhere in your recursive code, you might want to dedup? I think it might be here:
AutoType = type("_".join(parents), (graphene.InputObjectType,), filters)
So that Problem is most likely coming from your models. There will be a duplicated name in there somewhere. For example, we had used two different enums with the same name in two different models. Solution was to rename them/merge them into one. Hard to tell though without knowing your codebase. The line you are poointing out is the correct one to change if you can't find the culprit or my intuition is false. What you can do in that case is:
AutoType = type("_".join(parents + [uuid.uuid4().replace("-", "")]), (graphene.InputObjectType,), filters)
That makes it unique but the name of the filter in graphiql will look a bit ugly (The filter name itself will be the same though, so don't worry).
If you can't find a solution within your models, please let me know, then we have to think of a more permanent solution to this.
@PaulSchweizer the solution is I used a dictionary to reuse the InputObject that has already generated:
auto_type_name = f"{titleize(parents)}RelationFilter"
if auto_type_name not in all_auto_types:
all_auto_types[auto_type_name] = type(auto_type_name, (graphene.InputObjectType,), filters)
Please let me know if this is the correct way to resolve the issue.
(my gut says this is not an ideal solution, e.g., Foo table might have a relationship called BarFoo, and FooBar table might have a relationship called Foo, then after auto-gen, we cannot simply reuse FooBarFoo as the input objects might represent different things).
@JBrVJxsc that is unfortunate that we run into these issues regarding the naming. Reusing them is not really an option as you say. We could keep the behaviour as is, and only in those cases where we encounter this issue, we attach a uuid (my lazy fix) or a number (by keeping track of the names as you do now) to the end of the name to keep the type names unique. I'll set up a test for it and incorporate it into the PR
@PaulSchweizer Adding an appendix number sounds a better solution than the UUID, looking forward to it!
@PaulSchweizer if it's not too difficult would you try to implement the appendix number change in this PR ? @art1415926535 Would you be able to review the PR when you get some time? I think, this is going to be a really great feature for the library as well.
I'm very much looking forward to it.
The appendix number is now in place, please give it a try @JBrVJxsc and @sreerajkksd and let me know if it resolves your issues
The appendix number is now in place, please give it a try @JBrVJxsc and @sreerajkksd and let me know if it resolves your issues
I have tested the patches and it works beautifully!
@art1415926535 would you be able to approve this PR if this looks sane to you ?
The code you wrote doesn’t work with hybrid methods and throws an exception.
Also, am I reading this wrong? It looks like you are still manually defining the filters for all your nested relationships. If not can you provide an example of how you use this
@maquino1985 do hybrid methods work on the main branch of this repo? If so I'll have to look into it, but if not I suggest we tackle them in a separate ticket/PR as this PR is already quite big. Yes, you have to define the filter configuration for each model by hand, keeping it consistent with the original design of this library. There is however the option to create the configuration dict automatically by using sqlalchemy introspection (we actually do this in our project). This method has some pitfals however, so I recommend to only specify the filters you really need/want to expose. Maybe a solution in between can be helpful, like having a flat list of filterable attributes per model and then build the filter configuration automatically from them. Let me know if you need more info or describe where your problems are and I'll try to provide some more specific insights.
@PaulSchweizer no I don't think so -- i added a type check for instances of hybrid method to bypass them without raising an exception.
I've tried using this library with introspection but can't get it to work at all with my implementation. The connection field factory doesn't create any nested filters for some reason.
I added some example code into an issue on this repo but it's a bit complex because I also forked graphene-sqlalchemy for my project to create Mutations Interfaces and InputObjectTypes using sqlalchemy introspection. I need interfaces for my project because our types are polymorphic and I need to be able to do queries on the base type and then get subclass level attributes, e.g.
{
allProjects{
edges{
node{
contents{
edges{
node{
... on Request{ #implements TrackedEntityNode
id
#some request specific column
}
... on TrackedEntityNode{ #interface
id
# interface attribs
}
}
}
}
}
}
}
}
so like you my problem is that i have a huge database and can't (won't) manually write all the Graphene types, so I use some metaprogramming magic to create all of the types
class SQLAlchemyAutoSchemaFactory(ObjectType):
@staticmethod
def set_fields_and_attrs(
klazz: Type[ObjectType],
node_model: Type[SQLAlchemyInterface],
field_dict: Mapping[str, Field],
):
_name = to_snake_case(node_model.__name__)
field_dict[
f"all_{(pluralize(_name))}"
] = SQLAlchemyFilteredConnectionField(node_model.connection, sort=None)
field_dict[_name] = node_model.Field()
setattr(klazz, _name, node_model.Field())
setattr(
klazz,
"all_{}".format(pluralize(_name)),
SQLAlchemyFilteredConnectionField(node_model.connection, sort=None),
)
@classmethod
def __init_subclass_with_meta__(
cls,
interfaces: Tuple[Type[SQLAlchemyInterface]] = (),
models: Tuple[Type[DeclarativeMeta]] = (),
excluded_models: Tuple[Type[DeclarativeMeta]] = (),
exclude_model_fields: Tuple[str] = (),
node_interface: Type[Node] = Node,
default_resolver: ResolveInfo = None,
_meta=None,
**options,
):
if not _meta:
_meta = ObjectTypeOptions(cls)
fields = OrderedDict()
for interface in interfaces:
if issubclass(interface, SQLAlchemyInterface):
# sets fields and attrs based on model columns not all sqla properties
SQLAlchemyAutoSchemaFactory.set_fields_and_attrs(cls, interface, fields)
for model in excluded_models:
if model in models:
models = (
models[: models.index(model)] + models[models.index(model) + 1:]
)
possible_types = ()
for model in models[0:len(models)]:
model_name = model.__name__
_model_name = to_snake_case(model.__name__)
if hasattr(cls, _model_name):
continue
if hasattr(cls, "all_{}".format(pluralize(_model_name))):
continue
_model_interfaces = []
for iface in interfaces:
if issubclass(model, iface._meta.model):
_model_interfaces.append(iface)
if not _model_interfaces:
_model_interfaces = [node_interface]
_node_class = type(
model_name,
(SQLAlchemyObjectType,),
{
"Meta": {
"model": model,
"interfaces": (tuple(_model_interfaces)),
"only_fields": [],
"exclude_fields": exclude_model_fields,
}
},
)
fields[
"all_{}".format(pluralize(_model_name))
] = SQLAlchemyFilteredConnectionField(_node_class.connection, sort=_node_class.sort_argument(has_default=False))
setattr(
cls,
"all_{}".format(pluralize(_model_name)),
SQLAlchemyFilteredConnectionField(_node_class.connection, sort=_node_class.sort_argument(has_default=False)),
)
iface = _model_interfaces[0]
fields[_model_name] = iface.Field(_node_class)
setattr(cls, _model_name, iface.Field(_node_class))
possible_types += (_node_class,)
if _meta.fields:
_meta.fields.update(fields)
else:
_meta.fields = fields
_meta.schema_types = possible_types
super(SQLAlchemyAutoSchemaFactory, cls).__init_subclass_with_meta__(
_meta=_meta, default_resolver=default_resolver, **options
)
From the docs it would appear all I should need to do is create a FilterSet for each SQLAlchemy Model, then create a FilterableConnectionField object that has a dict with all types in it and use that field.factory as the connection_field_factory on all of my types, however this not only doesn't actually create any nested filters, it breaks all of the type resolution in the sqlalchemy filters.
I tried something like this (simplified a bit for clarity, hopefully not added confusion): create a FilterSet for each SQLAlchemy Model
_node_class_filter = type(
f"{model_name}Filter",
(FilterSet,),
{
"Meta": {
"model": model,
"fields": {
column.name: [...] for column in inspect(model).columns.values()
}
}
},
)
create the FilterableConnectionField to create the connection_field_factory:
class CustomField(FilterableConnectionField):
filters = {
model : _node_class_filter for model_filters.items()
}
create the graphene-sqlalchemy object types (interfaces assigned the same connection_field_factory):
_node_class = type(
model_name,
(SQLAlchemyObjectType,),
{
"Meta": {
"model": model,
"interfaces": (tuple(_model_interfaces)),
"only_fields": [],
"exclude_fields": exclude_model_fields,
"connection_field_factory": CustomField.factory
}
},
)
#add the fields to the Query
fields[
"all_{}".format(pluralize(model_name))
] = SQLAlchemyFilteredConnectionField(_node_class.connection)
@art1415926535 I think the final word should come from you.. Would you check this whenever you get some time ?
@maquino1985 I don't quite understand why explicit declaration does not work for you? We have a big database with tons of models as well, but we still explicitely declare a Filter and a Type for each sqlalchemy model, it's a bit of work but once the bootstrap code is done it all works as expected. The only time we use introspection is for creating the filter dict. This is how we use it (abbreviated and from memory):
class UserType(SQLAlchemyObjectType):
class Meta:
name = "User"
model = User
class UserFilter(BaseFilter):
class Meta:
model = User
fields = create_filter_fields_by_introspecting_model(User)
class Queries(graphene.ObjectType):
node = relay.Node.Field()
users = FilterableConnectionField(
UserType.connection, filters=UserFilter()
)
def create_filter_fields_by_introspecting_model(model):
inspected = inspection.inspect(model)
relationships = inspected.relationships
# further inspection code here ...
i found the issue. it's actually related to the library itself he doesn't seem to support polymorphic data models properly. i completely forgot that I found this and provided a fix back in March and a PR has been open since then....
Hello everyone, there has not been much activity on this repo recently. I've tried to contact art1415926535 many times without success to assist with updates. Would you be interested in a sponsored engagement to integrate this repo into graphene-sqlalchemy as a standard feature set?
yes!
Mark Aquino
From: Peter Harrison @.> Sent: Wednesday, March 30, 2022 1:09:39 PM To: art1415926535/graphene-sqlalchemy-filter @.> Cc: maquino1985 @.>; Mention @.> Subject: Re: [art1415926535/graphene-sqlalchemy-filter] Issue 42 - Generic solution for filtering over relationships (#49)
Hello everyone, there has not been much activity on this repo recently. I've tried to contact art1415926535 many times without success to assist with updates. Would you be interested in a sponsored engagement to integrate this repo into graphene-sqlalchemy as a standard feature set?
— Reply to this email directly, view it on GitHubhttps://github.com/art1415926535/graphene-sqlalchemy-filter/pull/49#issuecomment-1083402119, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADE52SJNUESAGZQZFTDEOGLVCSDFHANCNFSM4YI3YOAA. You are receiving this because you were mentioned.Message ID: @.***>
yes! Mark Aquino … ________________________________ From: Peter Harrison @.> Sent: Wednesday, March 30, 2022 1:09:39 PM To: art1415926535/graphene-sqlalchemy-filter @.> Cc: maquino1985 @.>; Mention @.> Subject: Re: [art1415926535/graphene-sqlalchemy-filter] Issue 42 - Generic solution for filtering over relationships (#49) Hello everyone, there has not been much activity on this repo recently. I've tried to contact art1415926535 many times without success to assist with updates. Would you be interested in a sponsored engagement to integrate this repo into graphene-sqlalchemy as a standard feature set? — Reply to this email directly, view it on GitHub<#49 (comment)>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADE52SJNUESAGZQZFTDEOGLVCSDFHANCNFSM4YI3YOAA. You are receiving this because you were mentioned.Message ID: @.***>
Thanks. If you are interested in contributing, please contact me on the Graphene slack channel (Peter Harrison) The link can be found here: https://github.com/graphql-python/graphene
Hi @palisadoes I'd also be happy to contribute, I'll contact you on slack
Thanks Paul. Your assistance is gratefully accepted. We are starting to have a small team for this. There is another contributor @sabard who has already started to review both code bases and working on a formal proposal. Please contact him on slack too, I'm sure he'll appreciate the help. When you contact me I'll try to create a dedicated slack channel for this feature.
