https://pulp.plan.io/https://pulp.plan.io/favicon.ico2018-03-28T19:59:32ZPulpPulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=270972018-03-28T19:59:32Zdaviddavis
<ul><li><strong>Subject</strong> changed from <i>Filters should not exist for /api/v3/repositories/uuid/versions/<version>/</i> to <i>Filters should not exist for /api/v3/repositories/<uuid>/versions/<version>/</i></li><li><strong>Description</strong> updated (<a title="View differences" href="/journals/27097/diff?detail_id=27663">diff</a>)</li></ul> Pulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=271532018-04-03T14:47:17Zdaviddavis
<ul><li><strong>Tags</strong> <i>Pulp 3, Pulp 3 MVP</i> added</li></ul><p>We should also confirm that this problem doesn't exist elsewhere in Pulp.</p> Pulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=272092018-04-04T14:28:40Zdaviddavis
<ul><li><strong>Sprint</strong> set to <i>Sprint 35</i></li></ul> Pulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=272152018-04-04T18:21:11Zdalleydalley@redhat.com
<ul><li><strong>Status</strong> changed from <i>NEW</i> to <i>ASSIGNED</i></li><li><strong>Assignee</strong> set to <i>dalley</i></li><li><strong>Triaged</strong> changed from <i>No</i> to <i>Yes</i></li></ul> Pulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=272712018-04-05T19:28:44Zdalleydalley@redhat.com
<ul></ul><p>Also applies to Task and Worker detail routes, so I assume it applies to all of them.</p>
<p>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.</p> Pulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=273772018-04-11T00:05:50Zdalleydalley@redhat.com
<ul></ul><p>So, according to one of my ex-coworkers who is the top contributor to django-filters by a solid margin:</p>
<blockquote>
<p>Yeah, so this is more of a DRF issue and not a django-filters issue<br>
basically, django-filters implements the DRF filter backend spec, but it has no control over when DRF invokes it<br>
Your problem spot is here: <a href="https://github.com/encode/django-rest-framework/blob/3.8.2/rest_framework/generics.py#L85" class="external">https://github.com/encode/django-rest-framework/blob/3.8.2/rest_framework/generics.py#L85</a><br>
`get_object` calls `filter_queryset`<br>
Your best bet is to override filter_queryset<br>
and to noop on `retrieve` actions<br>
so...</p>
</blockquote>
<pre><code class="python syntaxhl" data-language="python"><span class="k">def</span> <span class="nf">filter_queryset</span><span class="p">(</span><span class="bp">self</span><span class="p">,</span> <span class="n">queryset</span><span class="p">):</span>
<span class="k">if</span> <span class="bp">self</span><span class="p">.</span><span class="n">action</span> <span class="o">!=</span> <span class="s">'list'</span><span class="p">:</span>
<span class="k">return</span> <span class="n">queryset</span>
<span class="k">return</span> <span class="nb">super</span><span class="p">().</span><span class="n">filter_queryset</span><span class="p">(</span><span class="n">queryset</span><span class="p">)</span>
</code></pre>
<blockquote>
<p>All filter_queryset does is invoke the filter backends<br>
Just make sure that your permissions are not implemented as filters<br>
or something like that<br>
which would be weird<br>
and probably bad</p>
</blockquote>
<p>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.</p> Pulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=273792018-04-11T12:34:24Zdaviddavis
<ul></ul><p>Is opening an issue or PR against DRF an option?</p> Pulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=273802018-04-11T12:48:31Zdalleydalley@redhat.com
<ul></ul><p>Already been done - the issue was closed</p>
<p><a href="https://github.com/encode/django-rest-framework/issues/5412" class="external">https://github.com/encode/django-rest-framework/issues/5412</a></p>
<pre><code>
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.
</code></pre> Pulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=273812018-04-11T13:00:46Zdaviddavis
<ul></ul><p>I see.</p>
<p>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.</p>
<p>I don't like the idea of overriding <code>filter_queryset</code> either though. Maybe we can just close this out for now as WONTFIX and see if other users complain?</p> Pulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=273822018-04-11T13:27:22ZCodeHeeler
<ul></ul><p>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?</p> Pulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=273842018-04-11T13:32:03Zdalleydalley@redhat.com
<ul></ul><p>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.</p>
<p>They "work" on detail endpoints but (whatever thing is responsible for the schema) is smart enough to know that you shouldn't do it.</p> Pulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=273952018-04-11T14:37:37Zdaviddavis
<ul></ul><p><a class="user active" href="https://pulp.plan.io/users/349">dalley</a>, I am seeing something different. Here is the repo versions read endpoint:</p>
<pre><code> "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}/"
},
</code></pre> Pulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=273962018-04-11T14:39:03Zdalleydalley@redhat.com
<ul></ul><p>@daviddavis</p>
<p>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.</p> Pulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=273972018-04-11T14:40:24Zdaviddavis
<ul></ul><p><a class="user active" href="https://pulp.plan.io/users/349">dalley</a> ah maybe drf_yasg knows somehow to hide the filters. If that's the case, then +1 to WONTFIX on this from me.</p> Pulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=273982018-04-11T14:43:33Zdalleydalley@redhat.com
<ul></ul><p>Yeah, this is what yasg outputs (yaml format)</p>
<pre><code class="yaml syntaxhl" data-language="yaml"><span class="s">/repositories/{repository_pk}/versions/{number}/</span><span class="err">:</span>
<span class="na">get</span><span class="pi">:</span>
<span class="na">operationId</span><span class="pi">:</span> <span class="s">repositories_versions_read</span>
<span class="na">description</span><span class="pi">:</span> <span class="s1">'</span><span class="s">'</span>
<span class="na">parameters</span><span class="pi">:</span> <span class="pi">[]</span>
<span class="na">responses</span><span class="pi">:</span>
<span class="s1">'</span><span class="s">200'</span><span class="err">:</span>
<span class="na">description</span><span class="pi">:</span> <span class="s1">'</span><span class="s">'</span>
<span class="na">schema</span><span class="pi">:</span>
<span class="na">$ref</span><span class="pi">:</span> <span class="s1">'</span><span class="s">#/definitions/RepositoryVersion'</span>
<span class="na">tags</span><span class="pi">:</span>
<span class="pi">-</span> <span class="s">repositories</span>
</code></pre> Pulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=273992018-04-11T14:59:14Zdalleydalley@redhat.com
<ul></ul><p>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.</p> Pulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=274132018-04-11T16:28:19Zamacdona@redhat.comaustin@redhat.com
<ul></ul><p>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.</p>
<p>Would override of filter_queryset remove those filters from the docs?</p>
<ul>
<li>My instinct is no, which would mean that we still document those filters, but that users couldn't use them.</li>
<li>If no, -1 to override</li>
<li>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.</li>
</ul>
<p>If these filters are "legal" in the code, but not mentioned in the docs, I am fine with that situation and we can close.</p>
<p>If these filters are mentioned in the docs, +0 leave open as a documentation bug. (Remove Pulp 3 MVP tag)</p> Pulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=274202018-04-11T19:17:28Zdalleydalley@redhat.com
<ul></ul><p>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.</p>
<p>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.</p> Pulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=274212018-04-11T19:20:15Zdalleydalley@redhat.com
<ul><li><strong>Status</strong> changed from <i>ASSIGNED</i> to <i>CLOSED - WONTFIX</i></li><li><strong>Assignee</strong> deleted (<del><i>dalley</i></del>)</li><li><strong>Tags</strong> deleted (<del><i>Pulp 3 MVP</i></del>)</li></ul> Pulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=274222018-04-11T19:22:56Zdalleydalley@redhat.com
<ul><li><strong>Sprint</strong> deleted (<del><i>Sprint 35</i></del>)</li></ul> Pulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=274712018-04-13T17:47:34Zdalleydalley@redhat.com
<ul><li><strong>Subject</strong> changed from <i>Filters should not exist for /api/v3/repositories/<uuid>/versions/<version>/</i> to <i>Filters are applied to detail endpoints</i></li></ul> Pulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=414012019-04-25T16:45:25Zdaviddavis
<ul><li><strong>Sprint/Milestone</strong> set to <i>3.0.0</i></li></ul> Pulp - Issue #3537: Filters are applied to detail endpointshttps://pulp.plan.io/issues/3537?journal_id=425762019-04-26T20:35:57Zbmbouterbmbouter@redhat.com
<ul><li><strong>Tags</strong> deleted (<del><i>Pulp 3</i></del>)</li></ul>