Opened 8 years ago

Last modified 7 years ago

#28219 new Cleanup/optimization

Ease locating origin of queryset paginator warnings

Reported by: Denise Mauldin Owned by: nobody
Component: Core (Other) Version: 1.11
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I just updated from 1.10 to 1.11 and I get a ton of these UnorderedObjectListWarnings when running my tests. However, I have no idea where they're coming from, so I'm going through and searching for filter in my codebase and adding an order_by on the end of it. This is a quite annoying process and would be helped if the warning returned a stack trace of where it was being called from.

Change History (7)

comment:1 by Simon Charette, 8 years ago

Hi Denise, thank you for your report.

The stacklevel of the UnorderedObjectListWarning warning was adjusted to try to point at the right place in #28109 (shipped in 1.11.1) but it only accounts for direct Paginator manipulation. If you use ListView or ModelAdmin pagination features, which rely on Paginator internally, it's possible the warning is not pointing at the right user code location.

I'm afraid this will this be hard to solve without either adjusting all internal usages of Paginator to capture UnorderedObjectListWarning warnings and rewarn with a stacklevel matching the first possible user entry point but it sounds like a lost cause given the dynamic nature of queryset definition and generation.

For example

class FooListView(ListView):
    queryset = Foo.objects.all()
    paginate_by = 10

Maybe adjusting ListView.paginate_queryset() and ModelAdmin. get_paginator() to rewarn with a message mentioning self.__class__.qualname__ would be enough?

in reply to:  1 comment:2 by Denise Mauldin, 8 years ago

Hi Simon,

I'm not sure what to do. If I edit django/core/paginator.py to do a traceback.print_stack() call before the warning is called, then I get something really long that doesn't include any of my code other than the API call:

  File "manage.py", line 23, in <module>
    execute_from_command_line(sys.argv)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/core/management/__init__.py", line 363, in execute_from_command_line
    utility.execute()
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/core/management/__init__.py", line 355, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/core/management/commands/test.py", line 29, in run_from_argv
    super(Command, self).run_from_argv(argv)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/core/management/base.py", line 283, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/core/management/base.py", line 330, in execute
    output = self.handle(*args, **options)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/core/management/commands/test.py", line 62, in handle
    failures = test_runner.run_tests(test_labels)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/test/runner.py", line 603, in run_tests
    result = self.run_suite(suite)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/test/runner.py", line 567, in run_suite
    return runner.run(suite)
  File "/usr/local/python/3.5.1/lib/python3.5/unittest/runner.py", line 176, in run
    test(result)
  File "/usr/local/python/3.5.1/lib/python3.5/unittest/suite.py", line 84, in __call__
    return self.run(*args, **kwds)
  File "/usr/local/python/3.5.1/lib/python3.5/unittest/suite.py", line 122, in run
    test(result)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/test/testcases.py", line 213, in __call__
    super(SimpleTestCase, self).__call__(result)
  File "/usr/local/python/3.5.1/lib/python3.5/unittest/case.py", line 648, in __call__
    return self.run(*args, **kwds)
  File "/usr/local/python/3.5.1/lib/python3.5/unittest/case.py", line 600, in run
    testMethod()
  File "/var/www/ann/ann/orders/tests/test_api.py", line 58, in test_order_list_denied
    response = self.client.get('/api/orders/')
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/rest_framework/test.py", line 282, in get
    response = super(APIClient, self).get(path, data=data, **extra)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/rest_framework/test.py", line 208, in get
    return self.generic('GET', path, **r)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/test/client.py", line 416, in generic
    return self.request(**r)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/rest_framework/test.py", line 279, in request
    return super(APIClient, self).request(**kwargs)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/rest_framework/test.py", line 231, in request
    request = super(APIRequestFactory, self).request(**kwargs)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/test/client.py", line 483, in request
    response = self.handler(environ)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/test/client.py", line 144, in __call__
    response = self.get_response(request)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/rest_framework/test.py", line 251, in get_response
    return super(ForceAuthClientHandler, self).get_response(request)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/core/handlers/base.py", line 124, in get_response
    response = self._middleware_chain(request)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/core/handlers/exception.py", line 41, in inner
    response = get_response(request)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/utils/deprecation.py", line 140, in __call__
    response = self.get_response(request)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/core/handlers/exception.py", line 41, in inner
    response = get_response(request)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/utils/deprecation.py", line 140, in __call__
    response = self.get_response(request)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/core/handlers/exception.py", line 41, in inner
    response = get_response(request)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/utils/deprecation.py", line 140, in __call__
    response = self.get_response(request)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/core/handlers/exception.py", line 41, in inner
    response = get_response(request)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/utils/deprecation.py", line 140, in __call__
    response = self.get_response(request)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/core/handlers/exception.py", line 41, in inner
    response = get_response(request)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/utils/deprecation.py", line 140, in __call__
    response = self.get_response(request)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/core/handlers/exception.py", line 41, in inner
    response = get_response(request)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/utils/deprecation.py", line 140, in __call__
    response = self.get_response(request)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/core/handlers/exception.py", line 41, in inner
    response = get_response(request)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/utils/deprecation.py", line 140, in __call__
    response = self.get_response(request)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/core/handlers/exception.py", line 41, in inner
    response = get_response(request)
  File "/var/www/ann/ann/common/middleware.py", line 33, in __call__
    return self.get_response(request)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/core/handlers/exception.py", line 41, in inner
    response = get_response(request)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/utils/deprecation.py", line 140, in __call__
    response = self.get_response(request)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/core/handlers/exception.py", line 41, in inner
    response = get_response(request)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/core/handlers/base.py", line 185, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/local/python/3.5.1/lib/python3.5/contextlib.py", line 30, in inner
    return func(*args, **kwds)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/views/decorators/csrf.py", line 58, in wrapped_view
    return view_func(*args, **kwargs)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/rest_framework/viewsets.py", line 86, in view
    return self.dispatch(request, *args, **kwargs)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/rest_framework/views.py", line 486, in dispatch
    response = handler(request, *args, **kwargs)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/rest_framework/mixins.py", line 42, in list
    page = self.paginate_queryset(queryset)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/rest_framework/generics.py", line 173, in paginate_queryset
    return self.paginator.paginate_queryset(queryset, self.request, view=self)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/rest_framework/pagination.py", line 208, in paginate_queryset
    paginator = self.django_paginator_class(queryset, page_size)
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/core/paginator.py", line 32, in __init__
    self._check_object_list_is_ordered()
  File "/home/vagrant/.virtualenvs/ann/lib/python3.5/site-packages/django/core/paginator.py", line 110, in _check_object_list_is_ordered
    traceback.print_stack()

So I'm not sure why this warning is being triggered. I set Meta: ordering defaults on all of my models and apiviews. What else am I missing?

Maybe I should report this on rest_framework's bug tracker?

comment:3 by Simon Charette, 8 years ago

I'm pretty sure this is caused by your Order model's queryset not being ordered but it's hard to tell without your model and DRF view definitions. In all cases there's not much Django can do to solve your specific DRF issue. It should make sure the queryset passed to paginator is ordered and decide what to do from there.

For the record, the catch_warnings() method I suggested in my previous comment about rewarning won't work as it's not threadsafe.

Last edited 8 years ago by Simon Charette (previous) (diff)

in reply to:  3 comment:4 by Denise Mauldin, 8 years ago

Replying to Simon Charette:

I'm pretty sure this is caused by your Order model's queryset not being ordered but it's hard to tell without your model and DRF view definitions. In all cases there's not much Django can do to solve your specific DRF issue. It should make sure the queryset passed to paginator is ordered and decide what to do from there.

For the record, the catch_warnings() method I suggested in my previous comment about rewarning won't work as it's not threadsafe.

Yeah, but when there's 50 Order model tests and 2 of them throw this error, then it's hard to tell what's causing them when the stacktrace just points to paginator. Someone on SO did suggest that I run the tests on verbose mode (-v2), so that did help pinpoint which test was throwing the error. However, it'd be nice if the UnorderedObject warning would have an option to throw the full traceback so I could tell that it was line 58 in my test_api.py. Maybe that's not possible?

comment:5 by Tim Graham, 8 years ago

Component: UncategorizedCore (Other)
Summary: Paginator warning about unordered object should include stack traceEase locating origin of queryset paginator warnings
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

Tentatively accepting for further investigation, even though it's unclear if a solution is feasible.

comment:6 by Simon Charette, 8 years ago

However, it'd be nice if the UnorderedObject warning would have an option to throw the full traceback so I could tell that it was line 58 in my test_api.py. Maybe that's not possible?

Did you try running your test suite with python -W error? You could use warnings.simplefilter to only error in the UnorderedObjectListWarning case

import warnings
from django.core.paginator import UnorderedObjectListWarning
warnings.simplefilter('error', UnorderedObjectListWarning)

comment:7 by Simon Charette, 7 years ago

FWIW the re-warning approach mentioned in comment:1 could eventually be used if PEP 505 gets adopted as it would make warnings.catch_warnings() safe to use in multi-threaded environments.

Last edited 7 years ago by Simon Charette (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top