Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Test suite updated to Django 1.4 + much needed performance and API fixes #99

Closed
wants to merge 18 commits into from

8 participants

@hcarvalhoalves

Since I'm not sure if @alex is still actively working on django-taggit anymore I'm going to maintain a fork with some badly needed fixes. Here's the first pull request:

vdboor and others added some commits
@vdboor vdboor Optimize the query in `bulk_lookup_kwargs` for QuerySet args.
When passing a QuerySet to bulk_lookup_kwargs(), the queryset was
undesirably executed to get a list of IDs. This causes an unnecessary
performance loss.

There is no need to loop over a QuerySet to execute a IN (1, 3, 4) query.
Instead, just allow the ORM and database to execute a IN (SELECT id FROM ..) query.
In a real life app with a large set of objects, the difference in performance is noticeable.
a165f9b
@hcarvalhoalves hcarvalhoalves Renaming README with proper Markdown extension
e9f377a
@hcarvalhoalves hcarvalhoalves Updated AUTHORS.txt
34c6a52
@hcarvalhoalves hcarvalhoalves Fixed setup.py
4d83d4b
@hcarvalhoalves hcarvalhoalves Fixed test runner for Django 1.4
8e1702a
@hcarvalhoalves hcarvalhoalves Fixed failing test for non-deterministic list order
c084daf
@hcarvalhoalves hcarvalhoalves Updated AUTHORS.txt file
7a99909
@marcofucci marcofucci added related_name param to TaggableManager
edee405
@hcarvalhoalves hcarvalhoalves Updated AUTHORS.txt file
7dde9d8
@bkonkle bkonkle Add m2m_reverse_field_name to TaggableManager
be93aec
@hcarvalhoalves hcarvalhoalves Merge pull request #1 from bkonkle/master
Add m2m_reverse_field_name to TaggableManager. Thank you @bkonkle !
5349763
@gw0

Have you tried contacting him? Maybe over email or over other means?

@vdboor

@hcarvalhoalves great initiative to merge some good features back in. I hope @alex can merge it, or - if he's busy (which I can imagine) - allow others to become maintainer of the django-taggit package on pypi.

epicserve and others added some commits
@epicserve epicserve Updated admin.py
- added the slug to the list_display
- added search_fields
87e64ea
@epicserve epicserve Fixed a bug
Fixed an exception error that happens when someone adds a tag in a
different case, when a tag already exists in another case. In other
words when someone submits a new tag that creates the same slug as an
existing tag.
30dd950
@epicserve epicserve Made it so the exception catching is more database agnostic
89570fd
@hcarvalhoalves hcarvalhoalves Merge pull request #3 from epicserve/master
Updated admin.py + bugfix for psycopg2 backend
1cd2148
@vibragiel vibragiel Fixing database exceptions
All Django database backends capture the adapter exceptions an re-raise
them as django.db.utils exceptions, so it's safe and agnostic to just
capture these.
7e08a66
@vibragiel vibragiel Don't catch DatabaseError
63dceea
@hcarvalhoalves hcarvalhoalves Merge pull request #4 from vibragiel/master
Fixing database exceptions
e0f9642
@apollo13
Collaborator

@hcarvalhoalves, @vdboor: Could you see what's still relevant and open new pull requests for individual issues? Thanks!

@apollo13 apollo13 closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 28, 2012
  1. @vdboor @hcarvalhoalves

    Optimize the query in `bulk_lookup_kwargs` for QuerySet args.

    vdboor authored hcarvalhoalves committed
    When passing a QuerySet to bulk_lookup_kwargs(), the queryset was
    undesirably executed to get a list of IDs. This causes an unnecessary
    performance loss.
    
    There is no need to loop over a QuerySet to execute a IN (1, 3, 4) query.
    Instead, just allow the ORM and database to execute a IN (SELECT id FROM ..) query.
    In a real life app with a large set of objects, the difference in performance is noticeable.
  2. @hcarvalhoalves
  3. @hcarvalhoalves

    Updated AUTHORS.txt

    hcarvalhoalves authored
  4. @hcarvalhoalves

    Fixed setup.py

    hcarvalhoalves authored
Commits on Mar 29, 2012
  1. @hcarvalhoalves
  2. @hcarvalhoalves
  3. @hcarvalhoalves
  4. @marcofucci @hcarvalhoalves

    added related_name param to TaggableManager

    marcofucci authored hcarvalhoalves committed
  5. @hcarvalhoalves
Commits on Mar 31, 2012
  1. @bkonkle
Commits on Apr 9, 2012
  1. @hcarvalhoalves

    Merge pull request #1 from bkonkle/master

    hcarvalhoalves authored
    Add m2m_reverse_field_name to TaggableManager. Thank you @bkonkle !
Commits on Jul 19, 2012
  1. @epicserve

    Updated admin.py

    epicserve authored
    - added the slug to the list_display
    - added search_fields
Commits on Jul 23, 2012
  1. @epicserve

    Fixed a bug

    epicserve authored
    Fixed an exception error that happens when someone adds a tag in a
    different case, when a tag already exists in another case. In other
    words when someone submits a new tag that creates the same slug as an
    existing tag.
  2. @epicserve
Commits on Aug 2, 2012
  1. @hcarvalhoalves

    Merge pull request #3 from epicserve/master

    hcarvalhoalves authored
    Updated admin.py + bugfix for psycopg2 backend
Commits on Sep 20, 2012
  1. @vibragiel

    Fixing database exceptions

    vibragiel authored
    All Django database backends capture the adapter exceptions an re-raise
    them as django.db.utils exceptions, so it's safe and agnostic to just
    capture these.
Commits on Sep 21, 2012
  1. @vibragiel

    Don't catch DatabaseError

    vibragiel authored
  2. @hcarvalhoalves

    Merge pull request #4 from vibragiel/master

    hcarvalhoalves authored
    Fixing database exceptions
This page is out of date. Refresh to see the latest.
View
4 AUTHORS.txt
@@ -12,3 +12,7 @@ Frank Wiles
Jonathan Buchanan
idle sign <idlesign@yandex.ru>
Charles Leifer
+Diederik van der Boor <vdboor@edoburu.nl>
+Henrique Carvalho Alves <hcarvalhoalves@gmail.com>
+Marco Fucci <info@marcofucci.com>
+
View
0  README.txt → README.md
File renamed without changes
View
5 setup.py
@@ -4,7 +4,7 @@
from taggit import VERSION
-f = open(os.path.join(os.path.dirname(__file__), 'README.txt'))
+f = open(os.path.join(os.path.dirname(__file__), 'README.md'))
readme = f.read()
f.close()
@@ -32,6 +32,5 @@
'Programming Language :: Python',
'Framework :: Django',
],
- test_suite='taggit.tests.runtests.runtests'
+ test_suite='taggit.tests.runtests.runtests',
)
-
View
4 taggit/admin.py
@@ -6,8 +6,10 @@
class TaggedItemInline(admin.StackedInline):
model = TaggedItem
+
class TagAdmin(admin.ModelAdmin):
- list_display = ["name"]
+ list_display = ("name", 'slug')
+ search_fields = ('name', 'slug')
inlines = [
TaggedItemInline
]
View
11 taggit/managers.py
@@ -27,8 +27,8 @@ def all(iterable):
class TaggableRel(ManyToManyRel):
- def __init__(self):
- self.related_name = None
+ def __init__(self, related_name=None):
+ self.related_name = related_name
self.limit_choices_to = {}
self.symmetrical = True
self.multiple = True
@@ -37,9 +37,9 @@ def __init__(self):
class TaggableManager(RelatedField):
def __init__(self, verbose_name=_("Tags"),
- help_text=_("A comma-separated list of tags."), through=None, blank=False):
+ help_text=_("A comma-separated list of tags."), through=None, blank=False, related_name=None):
self.through = through or TaggedItem
- self.rel = TaggableRel()
+ self.rel = TaggableRel(related_name)
self.verbose_name = verbose_name
self.help_text = help_text
self.blank = blank
@@ -109,6 +109,9 @@ def related_query_name(self):
def m2m_reverse_name(self):
return self.through._meta.get_field_by_name("tag")[0].column
+
+ def m2m_reverse_field_name(self):
+ return self.through._meta.get_field_by_name("tag")[0].name
def m2m_target_field_name(self):
return self.model._meta.pk.name
View
23 taggit/models.py
@@ -1,9 +1,11 @@
import django
from django.contrib.contenttypes.models import ContentType
from django.contrib.contenttypes.generic import GenericForeignKey
-from django.db import models, IntegrityError, transaction
+from django.db import models, transaction
+from django.db.models.query import QuerySet
from django.template.defaultfilters import slugify as default_slugify
from django.utils.translation import ugettext_lazy as _, ugettext
+from django.db import utils
class TagBase(models.Model):
@@ -38,7 +40,7 @@ def save(self, *args, **kwargs):
res = super(TagBase, self).save(*args, **kwargs)
transaction.savepoint_commit(sid, **trans_kwargs)
return res
- except IntegrityError:
+ except utils.IntegrityError:
transaction.savepoint_rollback(sid, **trans_kwargs)
self.slug = self.slugify(self.name, i)
else:
@@ -137,11 +139,18 @@ def lookup_kwargs(cls, instance):
@classmethod
def bulk_lookup_kwargs(cls, instances):
- # TODO: instances[0], can we assume there are instances.
- return {
- "object_id__in": [instance.pk for instance in instances],
- "content_type": ContentType.objects.get_for_model(instances[0]),
- }
+ if isinstance(instances, QuerySet):
+ # Can do a real object_id IN (SELECT ..) query.
+ return {
+ "object_id__in": instances,
+ "content_type": ContentType.objects.get_for_model(instances.model),
+ }
+ else:
+ # TODO: instances[0], can we assume there are instances.
+ return {
+ "object_id__in": [instance.pk for instance in instances],
+ "content_type": ContentType.objects.get_for_model(instances[0]),
+ }
@classmethod
def tags_for(cls, model, instance=None):
View
24 taggit/tests/runtests.py
@@ -6,7 +6,11 @@
if not settings.configured:
settings.configure(
- DATABASE_ENGINE='sqlite3',
+ DATABASES={
+ 'default': {
+ 'ENGINE': 'django.db.backends.sqlite3',
+ }
+ },
INSTALLED_APPS=[
'django.contrib.contenttypes',
'taggit',
@@ -14,21 +18,13 @@
]
)
-from django.test.simple import run_tests
+from django.test.simple import DjangoTestSuiteRunner
-def runtests(*test_args):
- if not test_args:
- test_args = ['tests']
- parent = os.path.join(
- os.path.dirname(os.path.abspath(__file__)),
- "..",
- "..",
- )
- sys.path.insert(0, parent)
- failures = run_tests(test_args, verbosity=1, interactive=True)
+def runtests():
+ runner = DjangoTestSuiteRunner()
+ failures = runner.run_tests(['tests'], verbosity=1, interactive=True)
sys.exit(failures)
-
if __name__ == '__main__':
- runtests(*sys.argv[1:])
+ runtests()
View
25 taggit/tests/tests.py
@@ -211,10 +211,31 @@ def test_lookup_by_tag(self):
cat.tags.add("fuzzy")
self.assertEqual(
- map(lambda o: o.pk, self.pet_model.objects.filter(tags__name__in=["fuzzy"])),
- [kitty.pk, cat.pk]
+ set(map(lambda o: o.pk, self.pet_model.objects.filter(tags__name__in=["fuzzy"]))),
+ set([kitty.pk, cat.pk])
)
+ def test_lookup_bulk(self):
+ apple = self.food_model.objects.create(name="apple")
+ pear = self.food_model.objects.create(name="pear")
+ apple.tags.add('fruit', 'green')
+ pear.tags.add('fruit', 'yummie')
+
+ def lookup_qs():
+ # New fix: directly allow WHERE object_id IN (SELECT id FROM ..)
+ objects = self.food_model.objects.all()
+ lookup = self.taggeditem_model.bulk_lookup_kwargs(objects)
+ list(self.taggeditem_model.objects.filter(**lookup))
+
+ def lookup_list():
+ # Simulate old situation: iterate over a list.
+ objects = list(self.food_model.objects.all())
+ lookup = self.taggeditem_model.bulk_lookup_kwargs(objects)
+ list(self.taggeditem_model.objects.filter(**lookup))
+
+ self.assert_num_queries(1, lookup_qs)
+ self.assert_num_queries(2, lookup_list)
+
def test_exclude(self):
apple = self.food_model.objects.create(name="apple")
apple.tags.add("red", "green", "delicious")
Something went wrong with that request. Please try again.