Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Building a higher-level query API: the right way to use Django's ORM

dabapps.com

Post reply on HN
Print view View original

11–20 of 30 posts

Re: Building a higher-level query API: the right way to use Django's ORM

#11

Post by mgw »

mitechie wrote: post #6

I can't agree more. Always wrap your ORM with your own logic that makes sense to you and your application. You'll find code reuse will go way up, readability will go way up, testing is easier. You can actually test the model without bootstrapping the whole app. I've been preaching this in my SqlAlchemy talks and tutorials for years.

For our project I've been thinking about this a lot lately. Could you maybe elaborate on how you did this with SqlAlchemy and how you write your tests against the new model? Do you now of any more in-depth articles on the subject?

This really seems to hit a sweet spot of moving business logic into the model and I would love to use this in some form.

Re: Building a higher-level query API: the right way to use Django's ORM

#12

Post by legutierr »

j4mie wrote: post #9
zacharyvoase wrote: post #8

> Personally, I'm not completely convinced by the decorator-based idea. It obscures the details slightly, and feels a little "hacky". The purpose of the decorator is, indeed, to obscure the implementation details in favour of more semantic code. But then again we're using an ORM which makes heavy use of metaprogramming to obscure the details of the database layer from us; I don't see how this is a bad thing.

Yep, and my criticism of your suggested approach wasn't intended to be particularly strong by any means. I could definitely be sold on the idea. It just felt like a workaround to a problem that could probably be solved in a nicer way. I think my main objection is that these query methods should conceptually be on the QuerySet, and so defining them on the Manager (the "wrong place") and magically copying them to the Q…

Why not just pass a QuerySet implementation into models.Manager as an optional argument? This would be much cleaner, I think:

   class QuerySetSubclass(QuerySet):

      def new_method(self):
         pass

   class Model(models.Model):

      objects = models.Manager(QuerySetSubclass)

Re: Building a higher-level query API: the right way to use Django's ORM

#13

Post by jonknee »

tocomment wrote: post #4

I see what he's trying to do but this just seems like a ton of extra "boilerplate" code when you're trying to make an app. I'd rather spend extra time tracking down where I've used the is_done field if I later change it to a status, than spend all this time writing a manager for every query I do. Unit tests help with catching it if you've missed somewhere. On the other hand, if you have an extremely complicated query…

It's not much overhead and considering the savings in views the LOC will probably be equal or less. It doesn't make sense for every model or attribute, but if you find yourself doing the same types of queries multiple times, this can be a big savings.

Re: Building a higher-level query API: the right way to use Django's ORM

#15

Post by ryankask »

Aramgutang wrote: post #5

The main point raised by the article is spot-on, and I'm ashamed to say that I had never recognised it as an issue before reading it. It applies even more strongly for more complex lookups (possibly involving Q objects), which I've always felt would find a better home in models.py than in views.py. And I too cringe every time I come across the django.db.models.manager source code. Some thoughts: The approach goes sli…

I've been using django-model-utils for a while and really love it.

I contributed the patch for PassThroughManager that adds the for_queryset_class.

I agree that it's not pretty, but there's a bit of history that made it that way.

There are lot of custom QuerySet snippets floating around. One was `manager_from` by George Sakkis which Carl included in django-model-utils in July 2010. It was great except that the QuerySets it returned couldn't be pickled. It is currently pending deprecation.

It was replaced by Paul McLanahan's PassThroughManager.

You used it like

    objects = PassThroughManager(MyQuerySet)
That looks great except when related managers are instantiated, they aren't passed MyQuerySet (I haven't looked at the code in while and you have to dig around, but check out https://github.com/django/django/blob/master/django/db/model...).

You can still use it the old way but if you had an `alive` method on your QuerySet, you couldn't do:

    home.occupant_set.alive()

Re: Building a higher-level query API: the right way to use Django's ORM

#16

Post by richardlblair »

randlet wrote: post #7
Aramgutang wrote: post #5

The main point raised by the article is spot-on, and I'm ashamed to say that I had never recognised it as an issue before reading it. It applies even more strongly for more complex lookups (possibly involving Q objects), which I've always felt would find a better home in models.py than in views.py. And I too cringe every time I come across the django.db.models.manager source code. Some thoughts: The approach goes sli…

Agreed. I found the contrived chained filters at the beginning of the article off putting but the rest of the article was quite interesting.

I completely agree, but I think this was author's way of demonstrating how ugly a call to the ORM can get. With his very simple to-do list app I think this was the easiest way for him to do that.

In real life you would obviously have all your filter in one call (most of the time), then perhaps .values call with an .annotation call in there too.

Re: Building a higher-level query API: the right way to use Django's ORM

#17

Post by richardlblair »

This article is great and provides some awesome insight from someone who clearly has been down this road before. It couldn't have come at a better time for me. I was just about to implement my own manager today, but I'll take this much cleaner approach.

Thanks!

Re: Building a higher-level query API: the right way to use Django's ORM

#18

Post by jpwagner »

Wait, what's wrong with:

  class Todo(models.Model):
    content = models.CharField(max_length=100)
    # other fields go here..

    @classmethod
    def incomplete(cls):
        return cls.objects.filter(is_done=False)

    @classmethod
    def high_priority(cls):
        return cls.objects.filter(priority=1)

Re: Building a higher-level query API: the right way to use Django's ORM

#19

Post by j4mie »

zacharyvoase wrote: post #10
j4mie wrote: post #9

Earlier quoted context omitted.

Yep, and my criticism of your suggested approach wasn't intended to be particularly strong by any means. I could definitely be sold on the idea. It just felt like a workaround to a problem that could probably be solved in a nicer way. I think my main objection is that these query methods should conceptually be on the QuerySet, and so defining them on the Manager (the "wrong place") and magically copying them to the Q…

But my problem is that most people wouldn't even think of subclassing QuerySet. When we write methods that operate on collections of things, we typically use @classmethod. Without @classmethod, we'd have to write a custom metaclass (and instruct our class to use that) if we wanted even a single class method on a class. Multiple inheritance would break (or at least be difficult to reason about) when classes defined cl…

> But my problem is that most people wouldn't even think of subclassing QuerySet.

I'd argue that's a documentation issue. QuerySet is Django's abstraction of a set of filterable database results. Keeping this conceptually separate from other parts of the ORM, such as the model class, is valuable IMO, and I'm not sure we gain much by trying to hide the details. One of the things I love about Django's philosophy is that it generally provides shortcuts that encapsulate common patterns without obscuring them.

There's a fine line between discussing the minutiae of API design and bikeshedding, and I think that anything Django can do to help here is valuable, so I won't push the issue any further. Let's pick it up again on the mailing list or Trac at an appropriate time.

Re: Building a higher-level query API: the right way to use Django's ORM

#20

Post by lojack »

jpwagner wrote: post #18

Wait, what's wrong with: class Todo(models.Model): content = models.CharField(max_length=100) # other fields go here.. @classmethod def incomplete(cls): return cls.objects.filter(is_done=False) @classmethod def high_priority(cls): return cls.objects.filter(priority=1)

There's a couple reasons that's not ideal.

The big one is that you'd lose filter chaining. With your example you couldn't do

    Todo.objects.high_priority().incomplete()
The other issue is a semantic one. In your example you could do:

    Todo.objects.all()[0].incomplete()
which will return a QuerySet of all incomplete Todo items. This, at least to me, doesn't make sense.

The last reason is that by using a Manager you are encapsulating this filter data. If you later decide that you want to create a new model with similar types of filters, then you'd have to rewrite these methods. With a Manager, both models can simply use the same manager.

Post reply on HN