Django-rest-framework: Unrelated assert error message when there is an error in get_queryset

[Neraste]

In a view, I have define a get_queryset method in a APIView derived class. For some (not important) reasons, I have made a mistake in my code, but the error message I get is unrelated to the actual mistake: it is an AssertError, complaining there is no .queryset attribute or get_queryset method in the view. And the traceback does only imply files of the framework, not files I haver written and, obviously, neither the file I have made the mistake in.

Steps to reproduce

Create a class in views.py, deriving from APIView. Define a get_queryset method with a mistake in it, say an IndexError. the method do not return anything. Run it.

Txpected behavior

Get an error message involving an IndexError.
Get an error message about the empty return of get_queryset.

Actual behavior

Get an error message related to an AssertError.

[xordoquy]

Hi there, this feels like a chat I had at pycon.fr. Was it with you ?

That put appart, you need to provide a queryset for the view to work with routers, whether or not you override get_queryset. Will send more informations once I can use my computer.

Meanwhile I'm keeping this opened as we may have a documentation issue here

[Neraste]

Yep, that is me.

I am following the doc about this.

I will give you a MWE to locate the problem more precisely.

[xordoquy]

Thanks for raising it :)
I managed to reproduce on the train back home.

We have an inconsistence here.

There's a word in the documentation about using permissions with views that don't have a queryset attribute but looking at the code, permission are using get_queryset several times

I don't see a reason why routers don't use get_queryset instead of queryset. So, we'll need a PR to fix the permission documentation and one to let routers use get_queryset like permissions do.

[xordoquy]

I'm having hard time here and disgressing, sorry about that.
This issue is about getting rid of the assert message in favor of the initial exception within get_queryset right ?

[kevin-brown]

I don't see a reason why routers don't use get_queryset instead of queryset.

There's a good chance that if someone is overriding get_queryset, they are relying on something being set (the view object, request, etc.) that cannot normally be set without going through the view lifecycle. By using the queryset argument, it allows us to quickly get a queryset (even if it's an empty queryset, like Model.objects.none()) without having to prepare the view every time.

[xordoquy]

@kevin-brown good catch, thanks a lot for your feedback. That point will be solved by adding some words in the documentation.

[Neraste]

It seems that the problem I tried to describe is far less systematic than I thought... Raising an exception is not enough for poping an AssertError instead. And I didn't took note of what I was doing precisely when facing this problem.

However, I think I have isolated one form of it: if (for some dumb reasons), the get_queryset method _did not return_ anything, I get:

AssertionError at /some/place

Cannot apply DjangoModelPermissions on a view that does not have `.queryset` property or overrides the `.get_queryset()` method.

It does not complain get_queryset did not return anything (which is the real problem), the error message is pretty confusing and the traceback does not involve files of my project.

[tomchristie]

It does not complain get_queryset did not return anything (which is the real problem), the error message is pretty confusing.

Okay, that's a nicely isolated bit of behavior that'd be worth addressing. Let's constrain this issue to just that particular aspect, at least for now.