Comments (16)
One cool thing would be to inspect field asts and generate the correct prefetch_related, select_related and only calls onto the queryset. I think syrus is working on something like this.
Sent from my iPhone
On Nov 23, 2015, at 11:12 AM, Adam Charnock [email protected] wrote:
I've been thinking a bit about Django integration for Graphene. This is in part because I think it could be really useful, and in part because this is something I'd be interested in contributing a pull request for. However, development seems to be moving quickly so I thought it best to discuss these ideas first in case there are already plans.
The main motivation here is reducing boilerplate code. My intention would be to make all automatic functionality overridable & customisable.
Nodes:
Automatically traverse Django relations. A resolve_FIELD() may be specified to customise this traversal, but if not then traversal will be done using the relation's manager's all() method.
Automatically support query args for all/specified model fields (reduces resolve_FIELD() boilerplate code).
Query:Automatically resolve ConnectionFields which connect to DjangoNodes without the need for a resolve_FIELD() method.
Support for pulling in schema definitions from individual Django apps, rather than being defined centrally. (I currently do this by using a top-level Query class which inherits from multiple app-specific Query classes)
Mutations:Provide a DjangoMutation which provides standard functionality though which to update Django models.
Consider a class-based views approach, with CreateMutation, UpdateMutation, and DeleteMutation base classes.
All/any thoughts very welcome indeed!—
Reply to this email directly or view it on GitHub.
from graphene.
I agree @jhgg, I think that is #28.
As far as my ability to contribute is concerned, I would probably look to implement the basic ideas/functionality which could then be optimised.
from graphene.
Right now graphene is already transversing Django relations (only if the target model is in some ObjectType
in the schema), and resolving to ConnectionFields whenever the target model is defined in some DjangoNode
in the schema, without the need for a resolve_FIELD()
method.
I was thinking about adding support to django-filter for filtering the querysets (so you can define which attributes you can filter on).
I agree that could be very useful having the possibility to autodiscover the schema by searching into the django INSTALLED_APPS
(something similar to django.admin is doing).
About optimizing the hits to the dbs only fetching the data needed... I'm working on that! :)
The mutations will probably require a little bit more of work!
from graphene.
Probably adding custom permissions for querying/updating using django-guardian is also a good enhancement (and could be required when using automutation models?). Thoughts about that?
from graphene.
@syrusakbary I'll checkout the existing Django traversing, I think I mustn't have realised that.
Supporting third party packages such as django-filter and django-guardian sounds like a good idea, and I think you're right that some permission checking would be needed for most projects. I suggest we avoid requiring their use though, as people may already have other systems in place (especially for permissions). Which makes me wonder if a pluggable system of some form could make sense.
I haven't looked into how I'd actually implement the mutations, but I'd be open to suggestions.
from graphene.
I've just knocked out a proof of concept for a DjangoQuery
class. I just took my existing Query class and wrote a metaclass to autogenerate the necessary attributes. I'd be willing to accept that this is somewhat crude, but I felt it was worth a shot. It also has the advantage of allowing one to extend/override the attributes/methods in the child class.
https://gist.github.com/adamcharnock/78dbc5d8e448b4e5b1d9
from graphene.
Great work @adamcharnock!
About creating a BaseQuery class for DjangoNodes I think is something very opinionated for the framework. Why? I think imposing the ConnectionField fields names as all_X
should be done explicitly by the developer.
Once the django-filter
integration is done, probably will be not strictly required to create resolve_FIELDNAME
methods, as will be resolved by default with the DjangoFilter resolver.
Thoughts?
from graphene.
Thanks @syrusakbary, and I totally agree. That was something I meant to comment but clearly forgot. I couldn't see an obvious way of allowing the developer control of those names, so I just hard coded it for the sake of a simple proof-of-concept.
Good point on the resolve_FIELDNAME
methods. I'll take a look at creating a resolver based on django-filter
.
Given these comments, are you happy with the general direction of the above gist? If so I'll continue to develop it. If not, I'll hold off until we figure out a better idea.
EDIT: Added comments to gist
from graphene.
I think creating directly this fields by the developer will be better and less opinionated for the framework. (Maybe having this as some external package?)
I imagine doing something like this:
class Query(graphene.ObjectType):
all_droids = DjangoFilterConnectionField(Droid)
(Note that might not be necessary to create the resolve_all_droids
method)
from graphene.
Just checking in to say I haven't forgotten about this. My initial attempt – while a learning experience for me – is clearly not the way to go. I've spent some time getting on with my own project and in doing so I have definitely come around to the method you suggest above.
I'll knock up another gist in a bit.
EDIT: @syrusakbary Thank you very much for the suggestions and taking the time
from graphene.
Ok, here is attempt number two:
https://gist.github.com/adamcharnock/ad051b419d4c613d40fe
I have it loosely working at the moment, but no doubt they'll be issues. The main points are:
- The addition of a
DjangoFilterConnectionField
field - ...which uses a
FilterConnectionResolver
to resolve the field to a queryset as provided bydjango-filter
- The contents of
FilterConnectionResolver
were largely taken from thedjango-filter
FilterView
class
Points of concern:
- The dubious conversion of
django-filter
filters to Graphene types inDjangoFilterConnectionField.get_filtering_args()
- The use of the
on
parameter to specify the model field upon which the filtering should be performed. Just a general feeling that this could be done better.
from graphene.
Ok, I've just rewritten the conversion to Graphene types and updated the gist. The conversion is now down based upon the the form fields graphene uses to parse & validate the incoming values. See converter.py in the gist. Maybe there is a smarter way of doing this, but doing lookups seems to be the way both graphene
and django-filter
already do this so I did the same.
Issues to be addressed (suggestions welcome):
- The ordering of fields in the root
Query
is important. If a filter can filter upon a relation, then Graphene must already be aware of that model's existence, and that model's entry in the rootQuery
must therefore come first. - I think I need to implement an extra filter (
GlobalIDFilter
?) in order to filter against global IDs. This would need to reference aGlobalIDField
form field, which'll also need implementing.
from graphene.
@adamcharnock, after taking a deeper look at the gist here are some thoughts:
- Why
convert_form_field_to_djangomodel
returns an ID? (If I'm querying for a model inside a model, akauser.group.name
, I want to get the whole instance for the group, not just the id) - What you think about having
SimpleQuerySetConnectionResolver
,FilterConnectionResolver
as function decorators? So you can also do:
class Query(ObjectType):
# ...
@filter_connection
def resolve_all_accounts(self, args, info):
return Account.objects.filter(user=info.context.request_context.user) # Only for my user
- We could also create a Filter class dynamically (aka
django_filters.FilterSet
) when usingDjangoFilterConnectionField
, similar as Django Rest Framework is doing, something like:
class Query(ObjectType):
account = relay.NodeField(accounts_nodes.Account)
all_accounts = DjangoFilterConnectionField(accounts_nodes.Account, fields=['name', 'slug', 'created'])
Overall I really like your solution!
from graphene.
@syrusakbary Great, those sounds like some very good points. It's 3:30am here so I should really get some sleep, but I'll probably take a look tomorrow.
from graphene.
I've created PR #60 as this seems like a sensible time to stop working with gists.
@syrusakbary My reasoning for convert_form_field_to_djangomodel()
returning an ID
type is that, if one is filtering for a model, one would do so by the model's ID. However, this decision was not arrived at after an abundance of reflection. Do you have thoughts on this?
Your other two points sound like good suggestions. I'll investigate them further when get back to coding.
from graphene.
Closing this issue as the main PR #60 is merged into master.
from graphene.
Related Issues (20)
- Enum equality comparison is inconsistent
- http://www.graphene-python.org Has EXPIRED HOT 3
- Upload file HOT 2
- Unexpected argument in Pycharm 2023.2.1 HOT 1
- Default_value option in Arguments inside Mutation is not working as expected HOT 2
- Exception: NonNull could not have a mounted String() as inner type. Try with NonNull(String).
- Returning non fatal errors alongside data HOT 1
- Conversion to snake_case fails on nested inputs
- Depth limit validator key error if fragment undefined
- Tests in `graphene/relay/tests/test_custom_global_id.py` fail due incompatibility with pytest 8.x HOT 2
- Enum.from_enum() fails with Django IntegerChoices
- Do you support GQL over websocket? HOT 1
- More documentation for DisableIntrospection?
- Subclass built-in scalars without creating new GraphQL type
- In 3.1.0, `resolve_type` method on Interfaces and Unions no longer supports returning the GraphQL Type as a string HOT 1
- release 3.4.0 HOT 2
- grphene.List not work correctly HOT 1
- When passing the empty string to Date Type, it occurs `IndexError: string index out of range` HOT 2
- Schema execution timing out after upgrade from 2.1.9 to 3.3
- pregenerate schema and cache it for serverless cold start? HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from graphene.