Compatibility with graphene-django-filter #148

ethagnawl · 2024-09-12T20:03:29Z

ethagnawl
Sep 12, 2024

I'm working on a project that's happily been using this library (GDQO) to optimize away N+1 queries.

We are exploring options for adding more sophisticated filtering and are wondering how feasible it is to use this library in conjunction with graphene-django-filter.

Thanks to this library's DEFAULT_FILTERSET_CLASS setting, I was able to specify that I wanted to use GRAPHQL_QUERY_OPTIMIZER = { "DEFAULT_FILTERSET_CLASS": "graphene_django_filter.AdvancedFilterSet" } and things seem to continue to work.

I do have a few other unanswered questions, though:

Do we need to implement a qs method on the AdvancedFilterSet class, as is done in this library's FilterSet subclass?
Our old GDQO-compatible order_by feature does not work as-is. I've been digging into this a bit and it's not clear to me how that was ever working but it seems like GDQO was automatically applying the ordering argument when present. Our previous approach was as follows: (It's since been pointed out to me that this behavior is explicitly introduced elsewhere in a set of app-specific GDQO extensions -- I will need to review this implementation and follow up.)
I'd tried using an explicit OrderingFilter, as cribbed from the example app, and the result was:

# filterset
class BuildingFilterSet(FilterSet):
    order_by = OrderingFilter(fields=["name"])

    class Meta:
        model = Building
        fields = ["id", "name", "street_address"]

#error:
  File "/home/appuser/.local/lib/python3.12/site-packages/graphql/type/schema.py", line 432, in collect_referenced_types                                                         
    for field in named_type.fields.values():                                                                                                                                     
                 ^^^^^^^^^^^^^^^^^                                                                                                                                               
  File "/usr/local/lib/python3.12/functools.py", line 993, in __get__                                                                                                            
    val = self.func(instance)                                                                                                                                                    
          ^^^^^^^^^^^^^^^^^^^                                                                                                                                                    
  File "/home/appuser/.local/lib/python3.12/site-packages/graphql/type/definition.py", line 811, in fields                                                                       
    raise cls(f"{self.name} fields cannot be resolved. {error}") from error                                                                                                      
TypeError: SomeOtherNode fields cannot be resolved. More than one Argument have same name "order_by"

... which is very confusing because the filter set for SomeOtherNode was not modified. (This may also relate to the previous point.)

So, to summarize, I'm initially wondering if there's any fundamental reason that this won't work and if anyone else has attempted it. Also, if anyone has suggestions about how I might get our order_by filter working again.

Thanks!

MrThearMan · 2024-09-13T05:40:43Z

MrThearMan
Sep 13, 2024
Maintainer

Hi, thanks for using the library!

Do we need to implement a qs method on the AdvancedFilterSet class, as is done in this library's FilterSet subclass?

The reasoning stated in the custom FilterSet is as follows:

# Override the default `qs` property to include this.
# It's needed to ensure that the `queryset._result_cache`
# is not cleared when using `queryset.all()`
qs = maybe_queryset(self.queryset)

What this means is that this line in the original FilterSet is self.queryset.all(), which creates a clone of the queryset. Cloning a queryset does remove queryset._result_cache, which if it needs to be evaluated at this point in code would result in additional queries since the queryset would then need to be evaluated again later.

However, removing this doesn't seem to do anything based on my testing at present. This custom queryset was added pretty early in development, so perhaps something has changed in the details of the implementation so that this is no longer needed. I'm not 100% sure though. I might remove the custom filterset in the future based on this, but to answer you question: No, I don't think you need to reimplement it in your subclass.

We are exploring options for adding more sophisticated filtering and are wondering how feasible it is to use this library in conjunction with graphene-django-filter.

I haven't tested filtering with this library (I have my own implementation for a similar thing in graphene-django-extensions), but likely graphene-django-filter does something custom stuff to enable its behavior. I'm going to need a minimal example for reproducing this if I'm going to be able to help you. Can you set up a repo that I could clone to figure this out?

3 replies

ethagnawl Sep 13, 2024
Author

Thanks for the prompt response, @MrThearMan!

I'd actually given GDX a quick look yesterday but the logical filtering feature did not jump out at me from the README*. I just looked again, digging a little deeper, and saw some examples in the test suite which make this option a very compelling one for us.

So, while I greatly appreciate your offer to help dig into my issues with getting graphene-django-filter and GDQO working together, I'm going to pause on that line of work and instead see about getting GDX working.

*Assuming we make progress here, I'd be happy to take a pass at adding some color to the docs and/or adding an example project which highlights some of this functionality.

MrThearMan Sep 16, 2024
Maintainer

I do have an example project that I use for testing. I've moved this to the root level for easier discovery.

The readme is a bit short, but I don't want to make it too long either. I have separate docs where most examples e.g. for the logical filtering feature are described. That specifically is not in the readme, since I didn't think that would be a deal breaker for anyone. Feel free to suggest any improvements if you which though.

ethagnawl Sep 18, 2024
Author

Thanks, @MrThearMan. The docs are very instructive and example application is very useful, as is the tilavarauspalvelu-core repository (found while searching code on GitHub) which may be worth referencing.

Regarding the README, N=2 but a colleague and I both scanned past the mention of the ModelFilterSet without realizing that it offered logical filtering which, for us, is exactly what we were looking for.

I think we can close this discussion for now, though. I'm trying to work through a set of use-cases while integrating GDX and it's possible we may have so feedback about how the ModelFilterSet might be optionally extended to support complex/dynamic use cases (i.e. where composing Q()s may not work) but we'll open an issue/discussion in the appropriate place.

Thanks again for your prompt response and for maintaining such useful libraries!

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Compatibility with graphene-django-filter #148

{{title}}

{{editor}}'s edit

{{editor}}'s edit

Replies: 1 comment 3 replies

{{title}}

{{title}}

{{title}}

{{title}}

Select a reply

Compatibility with graphene-django-filter #148

ethagnawl Sep 12, 2024

Replies: 1 comment · 3 replies

MrThearMan Sep 13, 2024 Maintainer

ethagnawl Sep 13, 2024 Author

MrThearMan Sep 16, 2024 Maintainer

ethagnawl Sep 18, 2024 Author

ethagnawl
Sep 12, 2024

Replies: 1 comment 3 replies

MrThearMan
Sep 13, 2024
Maintainer

ethagnawl Sep 13, 2024
Author

MrThearMan Sep 16, 2024
Maintainer

ethagnawl Sep 18, 2024
Author