Pulp

Also applies to Task and Worker detail routes, so I assume it applies to all of them.

Worth pointing out that when filtering on a detail route, a 404 is returned if the resource doesn't match the filter, instead of the 200 with an empty list returned when filtering on a list route.

So, according to one of my ex-coworkers who is the top contributor to django-filters by a solid margin:

Yeah, so this is more of a DRF issue and not a django-filters issue
basically, django-filters implements the DRF filter backend spec, but it has no control over when DRF invokes it
Your problem spot is here: https://github.com/encode/django-rest-framework/blob/3.8.2/rest_framework/generics.py#L85
`get_object` calls `filter_queryset`
Your best bet is to override filter_queryset
and to noop on `retrieve` actions
so...

def filter_queryset(self, queryset):
    if self.action != 'list':
        return queryset
    return super().filter_queryset(queryset)
 

All filter_queryset does is invoke the filter backends
Just make sure that your permissions are not implemented as filters
or something like that
which would be weird
and probably bad

So, this is fixable, but it does involve overriding basic DRF behavior. Is it still something we want to do? I would say, you probably just shouldn't attempt to filter on a detail endpoint to begin with.

Is opening an issue or PR against DRF an option?

Already been done - the issue was closed

https://github.com/encode/django-rest-framework/issues/5412

This will be more clear if you consider the filter_queryset removes rows a user shouldn't be able to see. If we remove the call to filter_queryset then anyone will be able to see any object.

NB: I agree in your use case that's counter intuitive although in other it's totally valid.
Maybe we can improve that by adding a word on the documentation about the instances calling the filtering.

I see.

I'm less worried about users trying to filter on a detail or retrieve endpoint and more worried about how our schema doc looks. I think any user who looks at one of these endpoints is going to wonder why these fields exist.

I don't like the idea of overriding filter_queryset either though. Maybe we can just close this out for now as WONTFIX and see if other users complain?

I agree that it would be better to not override filter_queryset. Perhaps we could add a note in the docs somewhere at least if we opt for WONTFIX?

FYI I looked at the generated schema and the filter options do not show up on detail endpoints, just list endpoints as you would expect.

They "work" on detail endpoints but (whatever thing is responsible for the schema) is smart enough to know that you shouldn't do it.

dalley, I am seeing something different. Here is the repo versions read endpoint:

                    "read": {
                        "_type": "link",
                        "action": "get",
                        "fields": [
                            {
                                "location": "path",
                                "name": "number",
                                "required": true,
                                "schema": {
                                    "_type": "string",
                                    "description": "",
                                    "title": "number"
                                }
                            },
                            {
                                "location": "path",
                                "name": "repository_pk",
                                "required": true,
                                "schema": {
                                    "_type": "string",
                                    "description": "",
                                    "title": ""
                                }
                            },
                            {
                                "location": "query",
                                "name": "version_min",
                                "schema": {
                                    "_type": "number",
                                    "description": "",
                                    "title": ""
                                }
                            },
                            {
                                "location": "query",
                                "name": "version_max",
                                "schema": {
                                    "_type": "number",
                                    "description": "",
                                    "title": ""
                                }
                            },
                            {
                                "location": "query",
                                "name": "created_after",
                                "schema": {
                                    "_type": "string",
                                    "description": "",
                                    "title": ""
                                }
                            },
                            {
                                "location": "query",
                                "name": "created_before",
                                "schema": {
                                    "_type": "string",
                                    "description": "",
                                    "title": ""
                                }
                            },
                            {
                                "location": "query",
                                "name": "content",
                                "schema": {
                                    "_type": "string",
                                    "description": "",
                                    "title": ""
                                }
                            }
                        ],
                        "url": "/api/v3/repositories/{repository_pk}/versions/{number}/"
                    },

@daviddavis

Granted, I'm doing this from the branch that switches to using drf_yasg for docs, instead of drf_openapi. I've noticed that the two have some divergent behavior and that the output from drf_openapi actually doesn't validate properly against the openapi schema, while drf_yasg does.

dalley ah maybe drf_yasg knows somehow to hide the filters. If that's the case, then +1 to WONTFIX on this from me.

Yeah, this is what yasg outputs (yaml format)

/repositories/{repository_pk}/versions/{number}/:
    get:
      operationId: repositories_versions_read
      description: ''
      parameters: []
      responses:
        '200':
          description: ''
          schema:
            $ref: '#/definitions/RepositoryVersion'
      tags:
        - repositories

I'm +1 on WONTFIX as well but I'll leave the issue open for the rest of the day in case the list brings in more discussion.

I am not very concerned about users actually using the filters. For me, the only important reason to change this is to remove extraneous docs.

Would override of filter_queryset remove those filters from the docs?

• My instinct is no, which would mean that we still document those filters, but that users couldn't use them.
• If no, -1 to override
• if yes (and drf-yasg doesnt work out) I am not too concerned with overriding this in a base class. If some plugin needs filters for a detail route (I cant imagine why they would) they can easily override.

If these filters are "legal" in the code, but not mentioned in the docs, I am fine with that situation and we can close.

If these filters are mentioned in the docs, +0 leave open as a documentation bug. (Remove Pulp 3 MVP tag)

At least on my drf_yasg branch (and again I haven't looked at the current openapi branch), the filters definitely do not show up in the docs for the repoversion detail endpoint regardless. I just checked via the live docs webpage.

And since I do think we're planning to use drf_yasg as soon as the Travis issues get worked out so that the tests can pass, personally that says "wontfix" to me.