drf-spectacular automatically paginates error responses

[szymonhab]

Describe the bug
When there is DEFAULT_PAGINATION_CLASS specified in DRF settings or view implements pagination drf-spectacular is automatically adding pagination also to not successful list responses.

@extend_schema(
        parameters=[SOME_PARAMS],
        responses={
            200: TicketSerializer,
            400: {
                "type": "object",
                "properties": {"detail": {"type": "string"}},
                "example": {"detail": "Malformed request."},
            }
        }
    )
    def list(self, request, *args, **kwargs):
        pass

Expected behavior
I think that for responses other than 2** pagination shouldn't be included. Though it might be too much "magical" so maybe some additional variable in @extend_schema would do a job?
There is one more way to solve it: don't wrap response with pagination if response is raw dict.

Please correct me if there is ANY way to document such endpoint at this moment.

[tfranzel]

i see. that is the trade-off for modeling errors with "normal" responses. you can get around this with:

    class XViewset(viewsets.ViewSet):
        queryset = SimpleModel.objects.all()
        serializer_class = SimpleSerializer
        pagination_class = pagination.LimitOffsetPagination

        @extend_schema(
            responses={
                200: SimpleSerializer,
                400: extend_schema_serializer(
                    many=False,
                    examples=[OpenApiExample('Invalid Request', value={"detail": "Malformed request."}, status_codes=['400'])]
                )(
                    inline_serializer('Error500', {'detail': serializers.CharField()})
                )
            },

        )
        def list(self, request, *args, **kwargs):
            pass

this is just a compact example. important part is many=False, which forces the serializer into non-list mode. to use the extend_schema_serializer decorator, i had to create a serializer with inline_serializer. this can be done in many ways, just one example.

i can see that this is not pretty, but im still thinking about your suggestion regarding raw dict and +300 codes. it makes sense, but also breaks consistency and might be unexpected.

[yovelcohen]

@tfranzel what about situations where I have a pagination class set on a ModelViewSet but I want to disable it for the list action?

    @extend_schema(summary='Get All Groups', description="Get all of the user's groups")
    def list(self, request, *args, **kwargs):
        # overriding the method here to disable pagination
        queryset = self.filter_queryset(self.get_queryset())
        serializer = self.get_serializer(queryset, many=True)
        return Response(serializer.data)

in this case, I can pass to the decorator the responses serializer and pass many=False to the serializer but then the generated schema will show only one object and I do want to keep the list.
Workaround I did for now is:

class GroupSerializer(serializers.ModelSerializer):
    # My serializer stuff
@extend_schema_serializer(many=False)

class GroupsListReturn(serializers.Serializer):
    groups = GroupSerializer(many=True)

    class Meta:
        fields = ('groups',)

This code is able to disable the pagination from the response but changes the response from:

{ group object here}

to:

{
  groups: [ 
    { group object here},
  ]
}

[tfranzel]

@yovelcohen to what end? pagination_class is only used once in a ModelViewSet, i.e. list. why have pagination_class in the first place, if you want to turn off the one location it is actually used? wouldn't it be easier to just remove the pagination_class?

[yovelcohen]

@tfranzel
I used the list method as an example, but when I created custom actions and passed the serializer to the response param in extend_schema the redoc schema outputed it as a paginated result, which was not my intention, but also for the list method, if you set pagination class but you only want to use it in custom actions it will result in the same.
the built-in pagination classes also have the get_paginated_response_schema method, which I want to modify the extend_schema_view so it can override those. is this a possibility that is desirable? or that available already?

[tfranzel]

i see your problem now and it is tricky. i agree that we need to change something to accommodate that. pagination/list detection is an example where we have to use heuristics because it's not 100% certain what the expected outcome should be. in principle pagination is only specified for list and you cannot even rely on that. on the other side there is no mechanism to tell spectacular, this action body will use pagination. we could put it in extend_schema, but then it would apply to all (200, 400, 500) response codes (which we could restrict). it's tricky

the built-in pagination classes also have the get_paginated_response_schema method, which I want to modify the extend_schema_view so it can override those. is this a possibility that is desirable? or that available already?

yes we use that. if you customize the pagination class there, it will get used. customizing the the "pagination form" is possible there, but its not possible the the fix your root issue with that.

[tfranzel]

@yovelcohen in any case, this is the way i handled this before. i think this is quite clear and you don't use hacks in spectacular, just the regular DRF action configuration (pagination_class=None). this also works the other way round. of course this is orthogonal to the other points that were being made (e.g. non-2xx).

    class XViewset(viewsets.ReadOnlyModelViewSet):
        queryset = SimpleModel.objects.all()
        serializer_class = SimpleSerializer
        pagination_class = pagination.LimitOffsetPagination

        @extend_schema(responses={'200': SimpleSerializer(many=True)})
        @action(methods=['GET'], detail=False, pagination_class=None)
        def custom_action(self):
            pass  # pragma: no cover

[tfranzel]

this has been bugging me for a while now. @szymonhab i changed the behavior to what you would have expected in your initial post. after all it makes the most sense, even if it introduces status code dependent processing. at the end of the day, paginated 4XX responses should be a very fringe use-case.

i would argue it now follows the principle of least surprise.

[tfranzel]

closing this issue for now. feel free to comment if anything is missing or not working and we will follow-up.