-
Notifications
You must be signed in to change notification settings - Fork 107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable user to filter tasks by created_resources #229
Conversation
Codecov Report
@@ Coverage Diff @@
## master #229 +/- ##
==========================================
+ Coverage 67.66% 67.74% +0.07%
==========================================
Files 69 69
Lines 3272 3289 +17
==========================================
+ Hits 2214 2228 +14
- Misses 1058 1061 +3
Continue to review full report at Codecov.
|
70b6cf1
to
5f2ed8f
Compare
|
||
|
||
class FilterTaskCreatedResourcesTestCase(unittest.TestCase): | ||
"""Perform filtering over reserved resources.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"""Perform filtering over reserved resources.""" | |
"""Perform filtering over created resources.""" |
5f2ed8f
to
7d75eb7
Compare
|
||
|
||
class FilterTaskCreatedResourcesContentTestCase(unittest.TestCase): | ||
"""Perform filtering for contents of created resources.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this work has a related pulp.plan.io issue it would be nice to add as part of the docstring. It is very helpful later on.
"""Perform filtering for contents of created resources.""" | |
"""Perform filtering for contents of created resources. | |
This test targets the following issue: | |
* `Pulp #xxx <https://pulp.plan.io/issues/xxxx>` | |
""" |
cls.client = api.Client(config.get_config(), api.page_handler) | ||
|
||
cls.repository = cls.client.post(REPO_PATH, gen_repo()) | ||
response = cls.client.post(cls.repository['_versions_href']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is interesting. Goal here is to create a task. But I was expecting to be necessary to pass a few parameters to this endpoint: add_content_units, remove_content_units, or base_path. It seems that they are not mandatory.
https://docs.pulpproject.org/en/3.0/nightly/restapi.html#operation/repositories_versions_list
'created_resources': self.task['created_resources'][0] | ||
} | ||
results = self.client.get(TASKS_PATH, params=filter_params) | ||
self.assertEqual(len(results), 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.assertEqual(len(results), 1) | |
self.assertEqual(len(results), 1, results) |
pulpcore/app/viewsets/task.py
Outdated
@@ -28,6 +32,7 @@ class TaskFilter(BaseFilterSet): | |||
started_at = IsoDateTimeFilter(field_name='started_at') | |||
finished_at = IsoDateTimeFilter(field_name='finished_at') | |||
parent = HyperlinkRelatedFilter() | |||
created_resources = RepositoryGenericRelationFilter() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
created resources can be not only repository versions but many other things like distributions, content, etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$ http GET :24817/pulp/api/v3/tasks/?created_resources=/pulp/api/v3/distributions/docker/docker/ac1cbc69-1b1b-4108-90cd-45952dc2d60e/
HTTP/1.1 400 Bad Request
Allow: GET, HEAD, OPTIONS
Connection: close
Content-Length: 97
Content-Type: application/json
Date: Tue, 13 Aug 2019 14:51:16 GMT
Server: gunicorn/19.9.0
Vary: Accept, Cookie
X-Frame-Options: SAMEORIGIN
[
"URI not valid: /pulp/api/v3/distributions/docker/docker/ac1cbc69-1b1b-4108-90cd-45952dc2d60e/"
]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the filter needs to be extended to other resources that repo version
bc1489f
to
a169fe9
Compare
f89b220
to
f5d6416
Compare
Loop through the QuerySet and retrieve objects from a database which | ||
are matching a type and a URI of a corresponding created resource. | ||
""" | ||
for task in qs: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heres another pattern we can try:
from django.urls import resolve
match = resolve("/pulp/api/v3/repositories/38f857e4-92c5-4550-a087-7ce1a37f58a9/versions/1/")
match.func.cls.get_resource(
"/pulp/api/v3/repositories/38f857e4-92c5-4550-a087-7ce1a37f58a9/versions/1/",
match.func.cls.queryset.model
)
Out: <Repository: 3e361f06-b81a-4c15-92af-3715d72d91a4; Version: 1>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which can be trimmed:
match = resolve(value)
return NamedModelViewSet.get_resource(value, match.func.cls.queryset.model)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks great! Will it work in every scenario, like filtering distributions, contents, repository versions, and so on?
I will definitely give it a try tomorrow. Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should, but I only tried it with repo versions because I had some lying around.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tested it manually and it seems to be working fine.
a10bbb8
to
b96864d
Compare
b96864d
to
2ed3ed0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it works! thank you 🍺
closes #4931
https://pulp.plan.io/issues/4931